From 6edff5251821ef9e116bbdbc4de3aecd39a6a68d Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 9 Feb 2024 14:41:48 +0100 Subject: [PATCH 1/5] fix: warn against accidental global event referenced closes #10393 --- .changeset/chilly-snakes-scream.md | 5 + .../compiler/phases/2-analyze/validation.js | 37 ++++-- .../svelte/src/compiler/phases/constants.js | 117 ++++++++++++++++++ packages/svelte/src/compiler/utils/ast.js | 2 +- packages/svelte/src/compiler/warnings.js | 5 +- .../samples/global-event-reference/_config.js | 3 + .../global-event-reference/input.svelte | 12 ++ .../global-event-reference/warnings.json | 26 ++++ 8 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 .changeset/chilly-snakes-scream.md create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/_config.js create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/input.svelte create mode 100644 packages/svelte/tests/validator/samples/global-event-reference/warnings.json diff --git a/.changeset/chilly-snakes-scream.md b/.changeset/chilly-snakes-scream.md new file mode 100644 index 000000000000..dfdc19458e97 --- /dev/null +++ b/.changeset/chilly-snakes-scream.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: warn against accidental global event referenced diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 8af0a2940240..13a22b194217 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -1,10 +1,21 @@ import { error } from '../../errors.js'; -import { extract_identifiers, get_parent, is_text_attribute, object } from '../../utils/ast.js'; +import { + extract_identifiers, + get_parent, + is_expression_attribute, + is_text_attribute, + object +} from '../../utils/ast.js'; import { warn } from '../../warnings.js'; import fuzzymatch from '../1-parse/utils/fuzzymatch.js'; import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js'; import { binding_properties } from '../bindings.js'; -import { ContentEditableBindings, EventModifiers, SVGElements } from '../constants.js'; +import { + ContentEditableBindings, + EventModifiers, + SVGElements, + global_events +} from '../constants.js'; import { is_custom_element_node } from '../nodes.js'; import { regex_illegal_attribute_character, @@ -66,12 +77,24 @@ function validate_element(node, context) { } if (attribute.name.startsWith('on') && attribute.name.length > 2) { - if ( - attribute.value === true || - is_text_attribute(attribute) || - attribute.value.length > 1 - ) { + if (!is_expression_attribute(attribute)) { error(attribute, 'invalid-event-attribute-value'); + } else { + const value = attribute.value[0].expression; + if ( + value.type === 'Identifier' && + value.name === attribute.name && + !context.state.scope.get(value.name) && + global_events.has(attribute.name) + ) { + warn( + context.state.analysis.warnings, + attribute, + context.path, + 'global-event-reference', + attribute.name + ); + } } } diff --git a/packages/svelte/src/compiler/phases/constants.js b/packages/svelte/src/compiler/phases/constants.js index 33580703698b..d3cb2db1d6f5 100644 --- a/packages/svelte/src/compiler/phases/constants.js +++ b/packages/svelte/src/compiler/phases/constants.js @@ -197,3 +197,120 @@ export const JsKeywords = [ 'return', 'this' ]; + +/** + * Event variables that are available globally through the window object, + * but it's likely a user mistake if they're used + */ +export const global_events = new Set([ + 'onabort', + 'onafterprint', + 'onanimationcancel', + 'onanimationend', + 'onanimationiteration', + 'onanimationstart', + 'onauxclick', + 'onbeforeprint', + 'onbeforeunload', + 'onblur', + 'oncanplay', + 'oncanplaythrough', + 'onchange', + 'onclick', + 'onclose', + 'oncontextmenu', + 'oncuechange', + 'ondblclick', + 'ondevicemotion', + 'ondeviceorientation', + 'ondrag', + 'ondragend', + 'ondragenter', + 'ondragleave', + 'ondragover', + 'ondragstart', + 'ondrop', + 'ondurationchange', + 'onemptied', + 'onended', + 'onerror', + 'onfocus', + 'onformdata', + 'ongamepadconnected', + 'ongamepaddisconnected', + 'ongotpointercapture', + 'onhashchange', + 'oninput', + 'oninvalid', + 'onkeydown', + 'onkeypress', + 'onkeyup', + 'onlanguagechange', + 'onload', + 'onloadeddata', + 'onloadedmetadata', + 'onloadstart', + 'onlostpointercapture', + 'onmessage', + 'onmessageerror', + 'onmousedown', + 'onmouseenter', + 'onmouseleave', + 'onmousemove', + 'onmouseout', + 'onmouseover', + 'onmouseup', + 'onoffline', + 'ononline', + 'onorientationchange', + 'onpagehide', + 'onpageshow', + 'onpause', + 'onplay', + 'onplaying', + 'onpointercancel', + 'onpointerdown', + 'onpointerenter', + 'onpointerleave', + 'onpointermove', + 'onpointerout', + 'onpointerover', + 'onpointerup', + 'onpopstate', + 'onprogress', + 'onratechange', + 'onrejectionhandled', + 'onreset', + 'onresize', + 'onscroll', + 'onsecuritypolicyviolation', + 'onseeked', + 'onseeking', + 'onselect', + 'onselectionchange', + 'onselectstart', + 'onslotchange', + 'onstalled', + 'onstorage', + 'onsubmit', + 'onsuspend', + 'ontimeupdate', + 'ontoggle', + 'ontouchcancel', + 'ontouchend', + 'ontouchmove', + 'ontouchstart', + 'ontransitioncancel', + 'ontransitionend', + 'ontransitionrun', + 'ontransitionstart', + 'onunhandledrejection', + 'onunload', + 'onvolumechange', + 'onwaiting', + 'onwebkitanimationend', + 'onwebkitanimationiteration', + 'onwebkitanimationstart', + 'onwebkittransitionend', + 'onwheel' +]); diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index 33bdc451c869..464c2081f3e2 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/packages/svelte/src/compiler/utils/ast.js @@ -36,7 +36,7 @@ export function is_text_attribute(attribute) { * @param {import('#compiler').Attribute} attribute * @returns {attribute is import('#compiler').Attribute & { value: [import('#compiler').ExpressionTag] }} */ -function is_expression_attribute(attribute) { +export function is_expression_attribute(attribute) { return ( attribute.value !== true && attribute.value.length === 1 && diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 5c810664ba85..147905a60902 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -12,7 +12,10 @@ const css = { /** @satisfies {Warnings} */ const attributes = { - 'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided' + 'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided', + /** @param {string} name */ + 'global-event-reference': (name) => + `You are referencing the global property window.${name}. Did you forget to declare a variable with that name?` }; /** @satisfies {Warnings} */ diff --git a/packages/svelte/tests/validator/samples/global-event-reference/_config.js b/packages/svelte/tests/validator/samples/global-event-reference/_config.js new file mode 100644 index 000000000000..f47bee71df87 --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/validator/samples/global-event-reference/input.svelte b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte new file mode 100644 index 000000000000..ef713e6dc76f --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/packages/svelte/tests/validator/samples/global-event-reference/warnings.json b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json new file mode 100644 index 000000000000..8ac8519e1c5e --- /dev/null +++ b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json @@ -0,0 +1,26 @@ +[ + { + "code": "global-event-reference", + "message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?", + "start": { + "column": 8, + "line": 11 + }, + "end": { + "column": 19, + "line": 11 + } + }, + { + "code": "global-event-reference", + "message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?", + "start": { + "column": 8, + "line": 12 + }, + "end": { + "column": 29, + "line": 12 + } + } +] From 892351fb54ef0798c979b5f012bb1395fa277b2a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 17 Feb 2024 11:49:15 -0500 Subject: [PATCH 2/5] remove list --- .../compiler/phases/2-analyze/validation.js | 10 +- .../svelte/src/compiler/phases/constants.js | 117 ------------------ 2 files changed, 2 insertions(+), 125 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index 8423a190f5a5..a3257bad335c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -10,12 +10,7 @@ import { warn } from '../../warnings.js'; import fuzzymatch from '../1-parse/utils/fuzzymatch.js'; import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js'; import { binding_properties } from '../bindings.js'; -import { - ContentEditableBindings, - EventModifiers, - SVGElements, - global_events -} from '../constants.js'; +import { ContentEditableBindings, EventModifiers, SVGElements } from '../constants.js'; import { is_custom_element_node } from '../nodes.js'; import { regex_illegal_attribute_character, @@ -84,8 +79,7 @@ function validate_element(node, context) { if ( value.type === 'Identifier' && value.name === attribute.name && - !context.state.scope.get(value.name) && - global_events.has(attribute.name) + !context.state.scope.get(value.name) ) { warn( context.state.analysis.warnings, diff --git a/packages/svelte/src/compiler/phases/constants.js b/packages/svelte/src/compiler/phases/constants.js index e344c4097823..eaf01b7f341a 100644 --- a/packages/svelte/src/compiler/phases/constants.js +++ b/packages/svelte/src/compiler/phases/constants.js @@ -197,120 +197,3 @@ export const JsKeywords = [ 'return', 'this' ]; - -/** - * Event variables that are available globally through the window object, - * but it's likely a user mistake if they're used - */ -export const global_events = new Set([ - 'onabort', - 'onafterprint', - 'onanimationcancel', - 'onanimationend', - 'onanimationiteration', - 'onanimationstart', - 'onauxclick', - 'onbeforeprint', - 'onbeforeunload', - 'onblur', - 'oncanplay', - 'oncanplaythrough', - 'onchange', - 'onclick', - 'onclose', - 'oncontextmenu', - 'oncuechange', - 'ondblclick', - 'ondevicemotion', - 'ondeviceorientation', - 'ondrag', - 'ondragend', - 'ondragenter', - 'ondragleave', - 'ondragover', - 'ondragstart', - 'ondrop', - 'ondurationchange', - 'onemptied', - 'onended', - 'onerror', - 'onfocus', - 'onformdata', - 'ongamepadconnected', - 'ongamepaddisconnected', - 'ongotpointercapture', - 'onhashchange', - 'oninput', - 'oninvalid', - 'onkeydown', - 'onkeypress', - 'onkeyup', - 'onlanguagechange', - 'onload', - 'onloadeddata', - 'onloadedmetadata', - 'onloadstart', - 'onlostpointercapture', - 'onmessage', - 'onmessageerror', - 'onmousedown', - 'onmouseenter', - 'onmouseleave', - 'onmousemove', - 'onmouseout', - 'onmouseover', - 'onmouseup', - 'onoffline', - 'ononline', - 'onorientationchange', - 'onpagehide', - 'onpageshow', - 'onpause', - 'onplay', - 'onplaying', - 'onpointercancel', - 'onpointerdown', - 'onpointerenter', - 'onpointerleave', - 'onpointermove', - 'onpointerout', - 'onpointerover', - 'onpointerup', - 'onpopstate', - 'onprogress', - 'onratechange', - 'onrejectionhandled', - 'onreset', - 'onresize', - 'onscroll', - 'onsecuritypolicyviolation', - 'onseeked', - 'onseeking', - 'onselect', - 'onselectionchange', - 'onselectstart', - 'onslotchange', - 'onstalled', - 'onstorage', - 'onsubmit', - 'onsuspend', - 'ontimeupdate', - 'ontoggle', - 'ontouchcancel', - 'ontouchend', - 'ontouchmove', - 'ontouchstart', - 'ontransitioncancel', - 'ontransitionend', - 'ontransitionrun', - 'ontransitionstart', - 'onunhandledrejection', - 'onunload', - 'onvolumechange', - 'onwaiting', - 'onwebkitanimationend', - 'onwebkitanimationiteration', - 'onwebkitanimationstart', - 'onwebkittransitionend', - 'onwheel' -]); From e5ed5e0351c3c38cfb37503bb62793b97a139ce1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 17 Feb 2024 11:50:02 -0500 Subject: [PATCH 3/5] remove else --- .../compiler/phases/2-analyze/validation.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js index a3257bad335c..959f50874299 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/validation.js +++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js @@ -74,21 +74,21 @@ function validate_element(node, context) { if (attribute.name.startsWith('on') && attribute.name.length > 2) { if (!is_expression_attribute(attribute)) { error(attribute, 'invalid-event-attribute-value'); - } else { - const value = attribute.value[0].expression; - if ( - value.type === 'Identifier' && - value.name === attribute.name && - !context.state.scope.get(value.name) - ) { - warn( - context.state.analysis.warnings, - attribute, - context.path, - 'global-event-reference', - attribute.name - ); - } + } + + const value = attribute.value[0].expression; + if ( + value.type === 'Identifier' && + value.name === attribute.name && + !context.state.scope.get(value.name) + ) { + warn( + context.state.analysis.warnings, + attribute, + context.path, + 'global-event-reference', + attribute.name + ); } } From c28f98ba9264074ab0498d90e56447b824e53254 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 17 Feb 2024 11:52:29 -0500 Subject: [PATCH 4/5] tweak --- packages/svelte/src/compiler/warnings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 4d8e658dc11e..2f8308e27fcf 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -15,7 +15,7 @@ const attributes = { 'avoid-is': () => 'The "is" attribute is not supported cross-browser and should be avoided', /** @param {string} name */ 'global-event-reference': (name) => - `You are referencing the global property window.${name}. Did you forget to declare a variable with that name?` + `You are referencing globalThis.${name}. Did you forget to declare a variable with that name?` }; /** @satisfies {Warnings} */ From 23d9d12047ab8eab3a02a70696255abcd869481f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 17 Feb 2024 11:55:35 -0500 Subject: [PATCH 5/5] update test --- .../samples/global-event-reference/input.svelte | 3 --- .../samples/global-event-reference/warnings.json | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/svelte/tests/validator/samples/global-event-reference/input.svelte b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte index ef713e6dc76f..9a34449b42b1 100644 --- a/packages/svelte/tests/validator/samples/global-event-reference/input.svelte +++ b/packages/svelte/tests/validator/samples/global-event-reference/input.svelte @@ -5,8 +5,5 @@ - - - diff --git a/packages/svelte/tests/validator/samples/global-event-reference/warnings.json b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json index 8ac8519e1c5e..ccc1e482b52a 100644 --- a/packages/svelte/tests/validator/samples/global-event-reference/warnings.json +++ b/packages/svelte/tests/validator/samples/global-event-reference/warnings.json @@ -1,26 +1,26 @@ [ { "code": "global-event-reference", - "message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?", + "message": "You are referencing globalThis.onkeydown. Did you forget to declare a variable with that name?", "start": { "column": 8, - "line": 11 + "line": 8 }, "end": { "column": 19, - "line": 11 + "line": 8 } }, { "code": "global-event-reference", - "message": "You are referencing the global property window.onkeydown. Did you forget to declare a variable with that name?", + "message": "You are referencing globalThis.onkeydown. Did you forget to declare a variable with that name?", "start": { "column": 8, - "line": 12 + "line": 9 }, "end": { "column": 29, - "line": 12 + "line": 9 } } ]