From 3f5616c141e451cd0736aa6ff339fb71c47b02e9 Mon Sep 17 00:00:00 2001 From: Caique Torres Date: Thu, 25 Apr 2024 11:55:46 -0300 Subject: [PATCH] fix: raising an error when mixing both old and new event-handling syntaxes --- .../svelte/messages/compile-errors/template.md | 4 ++++ packages/svelte/src/compiler/errors.js | 10 ++++++++++ .../src/compiler/phases/2-analyze/index.js | 11 +++++++++++ .../compiler/phases/2-analyze/validation.js | 18 +++++++++++++++++- packages/svelte/src/compiler/phases/types.d.ts | 3 +++ .../runes-legacy-syntax-warnings-2/errors.json | 14 ++++++++++++++ .../input.svelte | 11 +++++++++++ 7 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/errors.json create mode 100644 packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/input.svelte diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index c1b8c5f3c3..293d4845cd 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -172,6 +172,10 @@ > `let:` directive at invalid position +## mixed_event_handler_syntaxes + +> Mixing old (on:%name%) and new syntaxes for event handling is not allowed. Use only the on%name% syntax. + ## node_invalid_placement > %thing% is invalid inside <%parent%> diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index a00a2cd593..d2834d03d4 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -918,6 +918,16 @@ export function let_directive_invalid_placement(node) { e(node, "let_directive_invalid_placement", "`let:` directive at invalid position"); } +/** + * Mixing old (on:%name%) and new syntaxes for event handling is not allowed. Use only the on%name% syntax. + * @param {null | number | NodeLike} node + * @param {string} name + * @returns {never} + */ +export function mixed_event_handler_syntaxes(node, name) { + e(node, "mixed_event_handler_syntaxes", `Mixing old (on:${name}) and new syntaxes for event handling is not allowed. Use only the on${name} syntax.`); +} + /** * %thing% is invalid inside <%parent%> * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 2ed2288a8e..a57d494301 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -372,6 +372,8 @@ export function analyze_component(root, source, options) { uses_render_tags: false, needs_context: false, needs_props: false, + event_directive_node: null, + uses_event_attributes: false, custom_element: options.customElementOptions ?? options.customElement, inject_styles: options.css === 'injected' || options.customElement, accessors: options.customElement @@ -1153,6 +1155,8 @@ const common_visitors = { }); if (is_event_attribute(node)) { + context.state.analysis.uses_event_attributes = true; + const expression = node.value[0].expression; const delegated_event = get_delegated_event(node.name.slice(2), expression, context); @@ -1286,6 +1290,13 @@ const common_visitors = { context.next(); }, + OnDirective(node, { state, path, next }) { + const parent = path.at(-1); + if (parent?.type === 'SvelteElement' || parent?.type === 'RegularElement') { + state.analysis.event_directive_node ??= node; + } + next(); + }, BindDirective(node, context) { let i = context.path.length; while (i--) { diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 4f68b5f9ed..2a2c7e5e8f 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -8,6 +8,7 @@ import * as e from '../../errors.js'; import { extract_identifiers, get_parent, + is_event_attribute, is_expression_attribute, is_text_attribute, object, @@ -104,6 +105,18 @@ function validate_element(node, context) { for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { + const parent_type = node.type; + + // Don't warn on component events; these might not be under the author's control so the warning would be unactionable + if ( + (parent_type === 'RegularElement' || parent_type === 'SvelteElement') && + is_event_attribute(attribute) && + context.state.analysis.event_directive_node + ) { + const { event_directive_node } = context.state.analysis; + e.mixed_event_handler_syntaxes(event_directive_node, event_directive_node.name); + } + const is_expression = is_expression_attribute(attribute); if (context.state.analysis.runes && is_expression) { @@ -1204,10 +1217,13 @@ export const validation_runes = merge(validation, a11y_validators, { w.slot_element_deprecated(node); } }, - OnDirective(node, { path }) { + OnDirective(node, { state, path }) { const parent_type = path.at(-1)?.type; // Don't warn on component events; these might not be under the author's control so the warning would be unactionable if (parent_type === 'RegularElement' || parent_type === 'SvelteElement') { + if (state.analysis.uses_event_attributes) { + e.mixed_event_handler_syntaxes(node, node.name); + } w.event_directive_deprecated(node, node.name); } }, diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index 3e892fefef..ccfa8cb4cb 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -2,6 +2,7 @@ import type { Binding, Css, Fragment, + OnDirective, RegularElement, SlotElement, SvelteElement, @@ -59,6 +60,8 @@ export interface ComponentAnalysis extends Analysis { uses_render_tags: boolean; needs_context: boolean; needs_props: boolean; + event_directive_node: OnDirective | null; + uses_event_attributes: boolean; custom_element: boolean | SvelteOptions['customElement']; /** If `true`, should append styles through JavaScript */ inject_styles: boolean; diff --git a/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/errors.json b/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/errors.json new file mode 100644 index 0000000000..6a60c09003 --- /dev/null +++ b/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "mixed_event_handler_syntaxes", + "message": "Mixing old (on:click) and new syntaxes for event handling is not allowed. Use only the onclick syntax.", + "start": { + "line": 11, + "column": 8 + }, + "end": { + "line": 11, + "column": 22 + } + } +] diff --git a/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/input.svelte b/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/input.svelte new file mode 100644 index 0000000000..c6df239137 --- /dev/null +++ b/packages/svelte/tests/validator/samples/runes-legacy-syntax-warnings-2/input.svelte @@ -0,0 +1,11 @@ + + + + + + + + +