Skip to content

Commit 6edff52

Browse files
dummdidummdummdidumm
committed
fix: warn against accidental global event referenced
closes #10393
1 parent 456cf84 commit 6edff52

File tree

8 files changed

+198
-9
lines changed

8 files changed

+198
-9
lines changed

‎.changeset/chilly-snakes-scream.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: warn against accidental global event referenced

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

+30-7
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
import { error } from '../../errors.js';
2-
import { extract_identifiers, get_parent, is_text_attribute, object } from '../../utils/ast.js';
2+
import {
3+
extract_identifiers,
4+
get_parent,
5+
is_expression_attribute,
6+
is_text_attribute,
7+
object
8+
} from '../../utils/ast.js';
39
import { warn } from '../../warnings.js';
410
import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
511
import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js';
612
import { binding_properties } from '../bindings.js';
7-
import { ContentEditableBindings, EventModifiers, SVGElements } from '../constants.js';
13+
import {
14+
ContentEditableBindings,
15+
EventModifiers,
16+
SVGElements,
17+
global_events
18+
} from '../constants.js';
819
import { is_custom_element_node } from '../nodes.js';
920
import {
1021
regex_illegal_attribute_character,
@@ -66,12 +77,24 @@ function validate_element(node, context) {
6677
}
6778

6879
if (attribute.name.startsWith('on') && attribute.name.length > 2) {
69-
if (
70-
attribute.value === true ||
71-
is_text_attribute(attribute) ||
72-
attribute.value.length > 1
73-
) {
80+
if (!is_expression_attribute(attribute)) {
7481
error(attribute, 'invalid-event-attribute-value');
82+
} else {
83+
const value = attribute.value[0].expression;
84+
if (
85+
value.type === 'Identifier' &&
86+
value.name === attribute.name &&
87+
!context.state.scope.get(value.name) &&
88+
global_events.has(attribute.name)
89+
) {
90+
warn(
91+
context.state.analysis.warnings,
92+
attribute,
93+
context.path,
94+
'global-event-reference',
95+
attribute.name
96+
);
97+
}
7598
}
7699
}
77100

‎packages/svelte/src/compiler/phases/constants.js

+117
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,120 @@ export const JsKeywords = [
197197
'return',
198198
'this'
199199
];
200+
201+
/**
202+
* Event variables that are available globally through the window object,
203+
* but it's likely a user mistake if they're used
204+
*/
205+
export const global_events = new Set([
206+
'onabort',
207+
'onafterprint',
208+
'onanimationcancel',
209+
'onanimationend',
210+
'onanimationiteration',
211+
'onanimationstart',
212+
'onauxclick',
213+
'onbeforeprint',
214+
'onbeforeunload',
215+
'onblur',
216+
'oncanplay',
217+
'oncanplaythrough',
218+
'onchange',
219+
'onclick',
220+
'onclose',
221+
'oncontextmenu',
222+
'oncuechange',
223+
'ondblclick',
224+
'ondevicemotion',
225+
'ondeviceorientation',
226+
'ondrag',
227+
'ondragend',
228+
'ondragenter',
229+
'ondragleave',
230+
'ondragover',
231+
'ondragstart',
232+
'ondrop',
233+
'ondurationchange',
234+
'onemptied',
235+
'onended',
236+
'onerror',
237+
'onfocus',
238+
'onformdata',
239+
'ongamepadconnected',
240+
'ongamepaddisconnected',
241+
'ongotpointercapture',
242+
'onhashchange',
243+
'oninput',
244+
'oninvalid',
245+
'onkeydown',
246+
'onkeypress',
247+
'onkeyup',
248+
'onlanguagechange',
249+
'onload',
250+
'onloadeddata',
251+
'onloadedmetadata',
252+
'onloadstart',
253+
'onlostpointercapture',
254+
'onmessage',
255+
'onmessageerror',
256+
'onmousedown',
257+
'onmouseenter',
258+
'onmouseleave',
259+
'onmousemove',
260+
'onmouseout',
261+
'onmouseover',
262+
'onmouseup',
263+
'onoffline',
264+
'ononline',
265+
'onorientationchange',
266+
'onpagehide',
267+
'onpageshow',
268+
'onpause',
269+
'onplay',
270+
'onplaying',
271+
'onpointercancel',
272+
'onpointerdown',
273+
'onpointerenter',
274+
'onpointerleave',
275+
'onpointermove',
276+
'onpointerout',
277+
'onpointerover',
278+
'onpointerup',
279+
'onpopstate',
280+
'onprogress',
281+
'onratechange',
282+
'onrejectionhandled',
283+
'onreset',
284+
'onresize',
285+
'onscroll',
286+
'onsecuritypolicyviolation',
287+
'onseeked',
288+
'onseeking',
289+
'onselect',
290+
'onselectionchange',
291+
'onselectstart',
292+
'onslotchange',
293+
'onstalled',
294+
'onstorage',
295+
'onsubmit',
296+
'onsuspend',
297+
'ontimeupdate',
298+
'ontoggle',
299+
'ontouchcancel',
300+
'ontouchend',
301+
'ontouchmove',
302+
'ontouchstart',
303+
'ontransitioncancel',
304+
'ontransitionend',
305+
'ontransitionrun',
306+
'ontransitionstart',
307+
'onunhandledrejection',
308+
'onunload',
309+
'onvolumechange',
310+
'onwaiting',
311+
'onwebkitanimationend',
312+
'onwebkitanimationiteration',
313+
'onwebkitanimationstart',
314+
'onwebkittransitionend',
315+
'onwheel'
316+
]);

‎packages/svelte/src/compiler/utils/ast.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function is_text_attribute(attribute) {
3636
* @param {import('#compiler').Attribute} attribute
3737
* @returns {attribute is import('#compiler').Attribute & { value: [import('#compiler').ExpressionTag] }}
3838
*/
39-
function is_expression_attribute(attribute) {
39+
export function is_expression_attribute(attribute) {
4040
return (
4141
attribute.value !== true &&
4242
attribute.value.length === 1 &&

‎packages/svelte/src/compiler/warnings.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ const css = {
1212

1313
/** @satisfies {Warnings} */
1414
const attributes = {
15-
'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided'
15+
'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided',
16+
/** @param {string} name */
17+
'global-event-reference': (name) =>
18+
`You are referencing the global property window.${name}. Did you forget to declare a variable with that name?`
1619
};
1720

1821
/** @satisfies {Warnings} */

‎packages/svelte/tests/validator/samples/global-event-reference/_config.js

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { test } from '../../test';
2+
3+
export default test({});

‎packages/svelte/tests/validator/samples/global-event-reference/input.svelte

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<script>
2+
let onclick;
3+
</script>
4+
5+
<button {onclick}></button>
6+
<button onclick={onclick}></button>
7+
8+
<button {onTypeScriptWillCatchThis}></button>
9+
<button onTypeScriptWillCatchThis={onTypeScriptWillCatchThis}></button>
10+
11+
<button {onkeydown}></button>
12+
<button onkeydown={onkeydown}></button>

‎packages/svelte/tests/validator/samples/global-event-reference/warnings.json

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{
3+
"code": "global-event-reference",
4+
"message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?",
5+
"start": {
6+
"column": 8,
7+
"line": 11
8+
},
9+
"end": {
10+
"column": 19,
11+
"line": 11
12+
}
13+
},
14+
{
15+
"code": "global-event-reference",
16+
"message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?",
17+
"start": {
18+
"column": 8,
19+
"line": 12
20+
},
21+
"end": {
22+
"column": 29,
23+
"line": 12
24+
}
25+
}
26+
]

0 commit comments

Comments
 (0)