Skip to content

Commit 8eab284

Browse files
committed
fix: raising an error when mixing both old and new event-handling syntaxes
1 parent 8e17428 commit 8eab284

File tree

5 files changed

+49
-1
lines changed

5 files changed

+49
-1
lines changed

packages/svelte/messages/compile-errors/template.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@
172172

173173
> `let:` directive at invalid position
174174
175+
## mixed_old_and_new_event_handler_syntaxes
176+
177+
> Mixing old (on:%name%) and new syntaxes for event handling is not recommended. Use only the on%name% syntax.
178+
175179
## node_invalid_placement
176180

177181
> %thing% is invalid inside <%parent%>

packages/svelte/src/compiler/errors.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,16 @@ export function let_directive_invalid_placement(node) {
918918
e(node, "let_directive_invalid_placement", "`let:` directive at invalid position");
919919
}
920920

921+
/**
922+
* Mixing old (on:%name%) and new syntaxes for event handling is not recommended. Use only the on%name% syntax.
923+
* @param {null | number | NodeLike} node
924+
* @param {string} name
925+
* @returns {never}
926+
*/
927+
export function mixed_old_and_new_event_handler_syntaxes(node, name) {
928+
e(node, "mixed_old_and_new_event_handler_syntaxes", `Mixing old (on:${name}) and new syntaxes for event handling is not recommended. Use only the on${name} syntax.`);
929+
}
930+
921931
/**
922932
* %thing% is invalid inside <%parent%>
923933
* @param {null | number | NodeLike} node

packages/svelte/src/compiler/phases/2-analyze/index.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ function get_delegated_event(event_name, handler, context) {
103103
}
104104

105105
if (binding != null) {
106+
if (binding.references.some(({ path }) => path.some(({ type }) => type === 'OnDirective'))) {
107+
return non_hoistable;
108+
}
109+
106110
for (const { path } of binding.references) {
107111
const parent = path.at(-1);
108112
if (parent == null) {
@@ -370,6 +374,8 @@ export function analyze_component(root, source, options) {
370374
uses_slots: false,
371375
uses_component_bindings: false,
372376
uses_render_tags: false,
377+
event_directive_node: null,
378+
uses_event_attributes: false,
373379
custom_element: options.customElementOptions ?? options.customElement,
374380
inject_styles: options.css === 'injected' || options.customElement,
375381
accessors: options.customElement
@@ -1120,6 +1126,8 @@ const common_visitors = {
11201126
});
11211127

11221128
if (is_event_attribute(node)) {
1129+
context.state.analysis.uses_event_attributes = true;
1130+
11231131
const expression = node.value[0].expression;
11241132

11251133
const delegated_event = get_delegated_event(node.name.slice(2), expression, context);
@@ -1237,6 +1245,13 @@ const common_visitors = {
12371245

12381246
context.next();
12391247
},
1248+
OnDirective(node, { state, path, next }) {
1249+
const parent = path.at(-1);
1250+
if (parent?.type === 'SvelteElement' || parent?.type === 'RegularElement') {
1251+
state.analysis.event_directive_node ??= node;
1252+
}
1253+
next();
1254+
},
12401255
BindDirective(node, context) {
12411256
let i = context.path.length;
12421257
while (i--) {

packages/svelte/src/compiler/phases/2-analyze/validation.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as e from '../../errors.js';
88
import {
99
extract_identifiers,
1010
get_parent,
11+
is_event_attribute,
1112
is_expression_attribute,
1213
is_text_attribute,
1314
object,
@@ -104,6 +105,18 @@ function validate_element(node, context) {
104105

105106
for (const attribute of node.attributes) {
106107
if (attribute.type === 'Attribute') {
108+
const parent_type = node.type;
109+
110+
// Don't warn on component events; these might not be under the author's control so the warning would be unactionable
111+
if (
112+
(parent_type === 'RegularElement' || parent_type === 'SvelteElement') &&
113+
is_event_attribute(attribute) &&
114+
context.state.analysis.event_directive_node
115+
) {
116+
const event_directive_node = context.state.analysis.event_directive_node;
117+
e.mixed_old_and_new_event_handler_syntaxes(event_directive_node, event_directive_node.name);
118+
}
119+
107120
const is_expression = is_expression_attribute(attribute);
108121

109122
if (context.state.analysis.runes && is_expression) {
@@ -1204,10 +1217,13 @@ export const validation_runes = merge(validation, a11y_validators, {
12041217
w.slot_element_deprecated(node);
12051218
}
12061219
},
1207-
OnDirective(node, { path }) {
1220+
OnDirective(node, { state, path }) {
12081221
const parent_type = path.at(-1)?.type;
12091222
// Don't warn on component events; these might not be under the author's control so the warning would be unactionable
12101223
if (parent_type === 'RegularElement' || parent_type === 'SvelteElement') {
1224+
if (state.analysis.uses_event_attributes) {
1225+
e.mixed_old_and_new_event_handler_syntaxes(node, node.name);
1226+
}
12111227
w.event_directive_deprecated(node, node.name);
12121228
}
12131229
},

packages/svelte/src/compiler/phases/types.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type {
22
Binding,
33
Css,
44
Fragment,
5+
OnDirective,
56
RegularElement,
67
SlotElement,
78
SvelteElement,
@@ -57,6 +58,8 @@ export interface ComponentAnalysis extends Analysis {
5758
uses_slots: boolean;
5859
uses_component_bindings: boolean;
5960
uses_render_tags: boolean;
61+
event_directive_node: OnDirective | null;
62+
uses_event_attributes: boolean;
6063
custom_element: boolean | SvelteOptions['customElement'];
6164
/** If `true`, should append styles through JavaScript */
6265
inject_styles: boolean;

0 commit comments

Comments
 (0)