From 9f7e650acb9bc14354ad3801c435f76e9734ce2c Mon Sep 17 00:00:00 2001 From: Admin Date: Thu, 9 Aug 2018 00:31:30 -0500 Subject: [PATCH 01/11] Adds event modifiers using | character --- src/compile/nodes/Element.ts | 56 ++++++++++++++++++++++++++++++---- src/shared/dom.js | 8 ++--- src/utils/getEventModifiers.ts | 36 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 src/utils/getEventModifiers.ts diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index 403b9cce8b..7c42ab71e2 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -6,6 +6,7 @@ import validCalleeObjects from '../../utils/validCalleeObjects'; import reservedNames from '../../utils/reservedNames'; import fixAttributeCasing from '../../utils/fixAttributeCasing'; import { quoteNameIfNecessary, quotePropIfNecessary } from '../../utils/quoteIfNecessary'; +import getEventModifiers from '../../utils/getEventModifiers'; import Compiler from '../Compiler'; import Node from './shared/Node'; import Block from '../dom/Block'; @@ -21,7 +22,45 @@ import mapChildren from './shared/mapChildren'; import { dimensions } from '../../utils/patterns'; // source: https://gist.github.com/ArjanSchouten/0b8574a6ad7f5065a5e7 -const booleanAttributes = new Set('async autocomplete autofocus autoplay border challenge checked compact contenteditable controls default defer disabled formnovalidate frameborder hidden indeterminate ismap loop multiple muted nohref noresize noshade novalidate nowrap open readonly required reversed scoped scrolling seamless selected sortable spellcheck translate'.split(' ')); +const booleanAttributes = new Set([ + 'async', + 'autocomplete', + 'autofocus', + 'autoplay', + 'border', + 'challenge', + 'checked', + 'compact', + 'contenteditable', + 'controls', + 'default', + 'defer', + 'disabled', + 'formnovalidate', + 'frameborder', + 'hidden', + 'indeterminate', + 'ismap', + 'loop', + 'multiple', + 'muted', + 'nohref', + 'noresize', + 'noshade', + 'novalidate', + 'nowrap', + 'open', + 'readonly', + 'required', + 'reversed', + 'scoped', + 'scrolling', + 'seamless', + 'selected', + 'sortable', + 'spellcheck', + 'translate' +]); export default class Element extends Node { type: 'Element'; @@ -612,14 +651,19 @@ export default class Element extends Node { const target = handler.shouldHoist ? 'this' : this.var; + // split by | to remove stop, prevent, pass, etc. + const eventName = handler.name.split('|')[0]; + // get a name for the event handler that is globally unique - // if hoisted, locally unique otherwise + // if hoisted, locally unique otherwise. const handlerName = (handler.shouldHoist ? compiler : block).getUniqueName( - `${handler.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_handler` + `${eventName.replace(/[^a-zA-Z0-9_$]/g, '_')}_handler` ); const component = block.alias('component'); // can't use #component, might be hoisted + const { bodyModifiers, optionModifiers } = getEventModifiers(handler.name); + // create the handler body const handlerBody = deindent` ${handler.shouldHoist && ( @@ -627,7 +671,7 @@ export default class Element extends Node { ? `const { ${[handler.usesComponent && 'component', handler.usesContext && 'ctx'].filter(Boolean).join(', ')} } = ${target}._svelte;` : null )} - + ${bodyModifiers} ${handler.snippet ? handler.snippet : `${component}.fire("${handler.name}", event);`} @@ -659,11 +703,11 @@ export default class Element extends Node { } block.builders.hydrate.addLine( - `@addListener(${this.var}, "${handler.name}", ${handlerName});` + `@addListener(${this.var}, "${eventName}", ${handlerName}, ${JSON.stringify(optionModifiers)});` ); block.builders.destroy.addLine( - `@removeListener(${this.var}, "${handler.name}", ${handlerName});` + `@removeListener(${this.var}, "${eventName}", ${handlerName}, ${JSON.stringify(optionModifiers)});` ); } }); diff --git a/src/shared/dom.js b/src/shared/dom.js index 03aaf8aaeb..0fb8f78f95 100644 --- a/src/shared/dom.js +++ b/src/shared/dom.js @@ -73,12 +73,12 @@ export function createComment() { return document.createComment(''); } -export function addListener(node, event, handler) { - node.addEventListener(event, handler, false); +export function addListener(node, event, handler, options) { + node.addEventListener(event, handler, options); } -export function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); +export function removeListener(node, event, handler, options) { + node.removeEventListener(event, handler, options); } export function setAttribute(node, attribute, value) { diff --git a/src/utils/getEventModifiers.ts b/src/utils/getEventModifiers.ts new file mode 100644 index 0000000000..f194e987e2 --- /dev/null +++ b/src/utils/getEventModifiers.ts @@ -0,0 +1,36 @@ +import EventHandler from '../compile/nodes/EventHandler'; +import deindent from '../utils/deindent'; + +export default function getEventModifiers(handlerName: String) { + // Ignore first element because it's the event name, i.e. click + let modifiers = handlerName.split('|').slice(1); + + let eventModifiers = modifiers.reduce((acc, m) => { + if (m === 'stop') + acc.bodyModifiers += 'event.stopPropagation();\n'; + else if (m === 'prevent') + acc.bodyModifiers += 'event.preventDefault();\n'; + else if (m === 'capture') + acc.optionModifiers[m] = true; + else if (m === 'once') + acc.optionModifiers[m] = true; + else if (m === 'passive') + acc.optionModifiers[m] = true; + + return acc; + }, { + bodyModifiers: '', + optionModifiers: { + capture: false, + once: false, + passive: false, + } + }); + + if (eventModifiers.bodyModifiers !== '') + eventModifiers.bodyModifiers = deindent` + ${eventModifiers.bodyModifiers} + `; + + return eventModifiers; +} \ No newline at end of file From 448d7335d447b50205c6020e4653e5c74c4fc85c Mon Sep 17 00:00:00 2001 From: Admin Date: Thu, 9 Aug 2018 01:06:09 -0500 Subject: [PATCH 02/11] Fixes tests that use events --- test/js/samples/input-files/expected-bundle.js | 8 ++++---- test/js/samples/input-range/expected-bundle.js | 8 ++++---- .../input-without-blowback-guard/expected-bundle.js | 8 ++++---- test/js/samples/media-bindings/expected-bundle.js | 8 ++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/test/js/samples/input-files/expected-bundle.js b/test/js/samples/input-files/expected-bundle.js index 097dc9e1b5..fa515f700c 100644 --- a/test/js/samples/input-files/expected-bundle.js +++ b/test/js/samples/input-files/expected-bundle.js @@ -17,12 +17,12 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { - node.addEventListener(event, handler, false); +function addListener(node, event, handler, options) { + node.addEventListener(event, handler, options); } -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); +function removeListener(node, event, handler, options) { + node.removeEventListener(event, handler, options); } function setAttribute(node, attribute, value) { diff --git a/test/js/samples/input-range/expected-bundle.js b/test/js/samples/input-range/expected-bundle.js index 50ba725fa9..b159277bf6 100644 --- a/test/js/samples/input-range/expected-bundle.js +++ b/test/js/samples/input-range/expected-bundle.js @@ -17,12 +17,12 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { - node.addEventListener(event, handler, false); +function addListener(node, event, handler, options) { + node.addEventListener(event, handler, options); } -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); +function removeListener(node, event, handler, options) { + node.removeEventListener(event, handler, options); } function setAttribute(node, attribute, value) { diff --git a/test/js/samples/input-without-blowback-guard/expected-bundle.js b/test/js/samples/input-without-blowback-guard/expected-bundle.js index 5bf43ec519..b50393ef21 100644 --- a/test/js/samples/input-without-blowback-guard/expected-bundle.js +++ b/test/js/samples/input-without-blowback-guard/expected-bundle.js @@ -17,12 +17,12 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { - node.addEventListener(event, handler, false); +function addListener(node, event, handler, options) { + node.addEventListener(event, handler, options); } -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); +function removeListener(node, event, handler, options) { + node.removeEventListener(event, handler, options); } function setAttribute(node, attribute, value) { diff --git a/test/js/samples/media-bindings/expected-bundle.js b/test/js/samples/media-bindings/expected-bundle.js index ec9116f544..17e820a067 100644 --- a/test/js/samples/media-bindings/expected-bundle.js +++ b/test/js/samples/media-bindings/expected-bundle.js @@ -17,12 +17,12 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { - node.addEventListener(event, handler, false); +function addListener(node, event, handler, options) { + node.addEventListener(event, handler, options); } -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); +function removeListener(node, event, handler, options) { + node.removeEventListener(event, handler, options); } function timeRangesToArray(ranges) { From 8d133bf3af7342abebd2feb192729cf16886b494 Mon Sep 17 00:00:00 2001 From: Admin Date: Thu, 9 Aug 2018 22:05:31 -0500 Subject: [PATCH 03/11] Adds invalid test for event-modifiers. --- src/validate/html/validateEventHandler.ts | 13 +++++++++++++ .../samples/event-modifiers-invalid/errors.json | 15 +++++++++++++++ .../samples/event-modifiers-invalid/input.html | 1 + 3 files changed, 29 insertions(+) create mode 100644 test/validator/samples/event-modifiers-invalid/errors.json create mode 100644 test/validator/samples/event-modifiers-invalid/input.html diff --git a/src/validate/html/validateEventHandler.ts b/src/validate/html/validateEventHandler.ts index df499a9fd5..b114070993 100644 --- a/src/validate/html/validateEventHandler.ts +++ b/src/validate/html/validateEventHandler.ts @@ -6,6 +6,8 @@ import { Node } from '../../interfaces'; const validBuiltins = new Set(['set', 'fire', 'destroy']); +const validModifiers = new Set(['stop', 'prevent', 'capture', 'once', 'passive']); + export default function validateEventHandlerCallee( validator: Validator, attribute: Node, @@ -22,6 +24,17 @@ export default function validateEventHandlerCallee( }); } + const modifiers = attribute.name.split('|').slice(1); + if ( + modifiers.length > 0 && + modifiers.filter(m => !validModifiers.has(m)).length > 0 + ) { + validator.error(attribute, { + code: 'invalid-event-modifiers', + message: `Valid event modifiers are ${[...validModifiers].join(', ')}.` + }); + } + const { name } = flattenReference(callee); if (validCalleeObjects.has(name) || name === 'options') return; diff --git a/test/validator/samples/event-modifiers-invalid/errors.json b/test/validator/samples/event-modifiers-invalid/errors.json new file mode 100644 index 0000000000..99737f50d2 --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid/errors.json @@ -0,0 +1,15 @@ +[{ + "message": "Valid event modifiers are stop, prevent, capture, once, passive.", + "code": "invalid-event-modifiers", + "start": { + "line": 1, + "column": 8, + "character": 8 + }, + "end": { + "line": 1, + "column": 36, + "character": 36 + }, + "pos": 8 +}] \ No newline at end of file diff --git a/test/validator/samples/event-modifiers-invalid/input.html b/test/validator/samples/event-modifiers-invalid/input.html new file mode 100644 index 0000000000..27dacdbc2d --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid/input.html @@ -0,0 +1 @@ + \ No newline at end of file From c65f06a610693db3f371dccae683a1ee897c7003 Mon Sep 17 00:00:00 2001 From: Admin Date: Sat, 11 Aug 2018 21:16:42 -0500 Subject: [PATCH 04/11] Changes stop and prevent to stopPropagation and preventDefault --- src/utils/getEventModifiers.ts | 4 ++-- src/validate/html/validateEventHandler.ts | 2 +- test/validator/samples/event-modifiers-invalid/errors.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/getEventModifiers.ts b/src/utils/getEventModifiers.ts index f194e987e2..554a2c79af 100644 --- a/src/utils/getEventModifiers.ts +++ b/src/utils/getEventModifiers.ts @@ -6,9 +6,9 @@ export default function getEventModifiers(handlerName: String) { let modifiers = handlerName.split('|').slice(1); let eventModifiers = modifiers.reduce((acc, m) => { - if (m === 'stop') + if (m === 'stopPropagation') acc.bodyModifiers += 'event.stopPropagation();\n'; - else if (m === 'prevent') + else if (m === 'preventDefault') acc.bodyModifiers += 'event.preventDefault();\n'; else if (m === 'capture') acc.optionModifiers[m] = true; diff --git a/src/validate/html/validateEventHandler.ts b/src/validate/html/validateEventHandler.ts index b114070993..f1df2f741c 100644 --- a/src/validate/html/validateEventHandler.ts +++ b/src/validate/html/validateEventHandler.ts @@ -6,7 +6,7 @@ import { Node } from '../../interfaces'; const validBuiltins = new Set(['set', 'fire', 'destroy']); -const validModifiers = new Set(['stop', 'prevent', 'capture', 'once', 'passive']); +const validModifiers = new Set(['stopPropagation', 'preventDefault', 'capture', 'once', 'passive']); export default function validateEventHandlerCallee( validator: Validator, diff --git a/test/validator/samples/event-modifiers-invalid/errors.json b/test/validator/samples/event-modifiers-invalid/errors.json index 99737f50d2..af1cca83e4 100644 --- a/test/validator/samples/event-modifiers-invalid/errors.json +++ b/test/validator/samples/event-modifiers-invalid/errors.json @@ -1,5 +1,5 @@ [{ - "message": "Valid event modifiers are stop, prevent, capture, once, passive.", + "message": "Valid event modifiers are stopPropagation, preventDefault, capture, once, passive.", "code": "invalid-event-modifiers", "start": { "line": 1, From 4784ff02d63ecc74778853b2c236d798d7a11392 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 12:33:21 -0400 Subject: [PATCH 05/11] add some more modifier validation tests --- src/compile/nodes/Element.ts | 54 +++++++++++++++++++ src/compile/nodes/EventHandler.ts | 3 ++ src/compile/nodes/shared/Expression.ts | 14 +++-- src/parse/read/directives.ts | 5 +- .../event-modifiers-invalid/errors.json | 4 +- .../event-modifiers-redundant/input.html | 16 ++++++ .../event-modifiers-redundant/warnings.json | 32 +++++++++++ 7 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 test/validator/samples/event-modifiers-redundant/input.html create mode 100644 test/validator/samples/event-modifiers-redundant/warnings.json diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index c060edc1f8..e05c587805 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -14,6 +14,7 @@ import mapChildren from './shared/mapChildren'; import { dimensions } from '../../utils/patterns'; import fuzzymatch from '../validate/utils/fuzzymatch'; import Ref from './Ref'; +import list from '../../utils/list'; const svg = /^(?:altGlyph|altGlyphDef|altGlyphItem|animate|animateColor|animateMotion|animateTransform|circle|clipPath|color-profile|cursor|defs|desc|discard|ellipse|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feDropShadow|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|filter|font|font-face|font-face-format|font-face-name|font-face-src|font-face-uri|foreignObject|g|glyph|glyphRef|hatch|hatchpath|hkern|image|line|linearGradient|marker|mask|mesh|meshgradient|meshpatch|meshrow|metadata|missing-glyph|mpath|path|pattern|polygon|polyline|radialGradient|rect|set|solidcolor|stop|switch|symbol|text|textPath|tref|tspan|unknown|use|view|vkern)$/; @@ -228,6 +229,7 @@ export default class Element extends Node { this.validateAttributes(); this.validateBindings(); this.validateContent(); + this.validateEventHandlers(); } validateAttributes() { @@ -563,6 +565,58 @@ export default class Element extends Node { } } + validateEventHandlers() { + const { component } = this; + + const validModifiers = new Set([ + 'preventDefault', + 'stopPropagation', + 'capture', + 'once', + 'passive' + ]); + + const passiveEvents = new Set([ + 'wheel', + 'touchstart', + 'touchmove', + 'touchend', + 'touchcancel' + ]); + + this.handlers.forEach(handler => { + handler.modifiers.forEach(modifier => { + if (!validModifiers.has(modifier)) { + component.error(handler, { + code: 'invalid-event-modifier', + message: `Valid event modifiers are ${list([...validModifiers])}` + }); + } + + if (modifier === 'passive') { + if (passiveEvents.has(handler.name)) { + const usesEvent = ( + handler.callee.name === 'event' || + handler.args.some(x => x.usesEvent) + ); + + if (!usesEvent) { + component.warn(handler, { + code: 'redundant-event-modifier', + message: `Touch event handlers that don't use the 'event' object are passive by default` + }); + } + } else { + component.warn(handler, { + code: 'redundant-event-modifier', + message: `The passive modifier only works with wheel and touch events` + }); + } + } + }); + }); + } + getStaticAttributeValue(name: string) { const attribute = this.attributes.find( (attr: Attribute) => attr.type === 'Attribute' && attr.name.toLowerCase() === name diff --git a/src/compile/nodes/EventHandler.ts b/src/compile/nodes/EventHandler.ts index 05aa51d1da..00b24f21f5 100644 --- a/src/compile/nodes/EventHandler.ts +++ b/src/compile/nodes/EventHandler.ts @@ -9,6 +9,7 @@ const validBuiltins = new Set(['set', 'fire', 'destroy']); export default class EventHandler extends Node { name: string; + modifiers: Set; dependencies: Set; expression: Node; callee: any; // TODO @@ -26,6 +27,8 @@ export default class EventHandler extends Node { super(component, parent, scope, info); this.name = info.name; + this.modifiers = new Set(info.modifiers); + component.used.events.add(this.name); this.dependencies = new Set(); diff --git a/src/compile/nodes/shared/Expression.ts b/src/compile/nodes/shared/Expression.ts index 6acb6282ec..dc6f77d82e 100644 --- a/src/compile/nodes/shared/Expression.ts +++ b/src/compile/nodes/shared/Expression.ts @@ -57,11 +57,12 @@ export default class Expression { component: Component; node: any; snippet: string; - - usesContext: boolean; references: Set; dependencies: Set; + usesContext = false; + usesEvent = false; + thisReferences: Array<{ start: number, end: number }>; constructor(component, parent, scope, info) { @@ -77,8 +78,6 @@ export default class Expression { this.snippet = `[✂${info.start}-${info.end}✂]`; - this.usesContext = false; - const dependencies = new Set(); const { code, helpers } = component; @@ -109,7 +108,12 @@ export default class Expression { if (isReference(node, parent)) { const { name, nodes } = flattenReference(node); - if (currentScope.has(name) || (name === 'event' && isEventHandler)) return; + if (name === 'event' && isEventHandler) { + expression.usesEvent = true; + return; + } + + if (currentScope.has(name)) return; if (component.helpers.has(name)) { let object = node; diff --git a/src/parse/read/directives.ts b/src/parse/read/directives.ts index f1955ffcbc..6993f3ebda 100644 --- a/src/parse/read/directives.ts +++ b/src/parse/read/directives.ts @@ -25,8 +25,9 @@ const DIRECTIVES: Record +
+ + \ No newline at end of file diff --git a/test/validator/samples/event-modifiers-redundant/warnings.json b/test/validator/samples/event-modifiers-redundant/warnings.json new file mode 100644 index 0000000000..7b6cae16d4 --- /dev/null +++ b/test/validator/samples/event-modifiers-redundant/warnings.json @@ -0,0 +1,32 @@ +[ + { + "message": "The passive modifier only works with wheel and touch events", + "code": "redundant-event-modifier", + "start": { + "line": 1, + "column": 8, + "character": 8 + }, + "end": { + "line": 1, + "column": 40, + "character": 40 + }, + "pos": 8 + }, + { + "message": "Touch event handlers that don't use the 'event' object are passive by default", + "code": "redundant-event-modifier", + "start": { + "line": 2, + "column": 5, + "character": 56 + }, + "end": { + "line": 2, + "column": 47, + "character": 98 + }, + "pos": 56 + } +] From 6c92d9955fafaf134a662fa7744b35e5098e4687 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 13:24:09 -0400 Subject: [PATCH 06/11] disallow once/passive in legacy mode, for now --- src/compile/nodes/Element.ts | 9 ++++++ test/validator/index.js | 1 + .../event-modifiers-invalid/errors.json | 28 +++++++++---------- .../samples/event-modifiers-legacy/_config.js | 3 ++ .../event-modifiers-legacy/errors.json | 15 ++++++++++ .../samples/event-modifiers-legacy/input.html | 1 + 6 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 test/validator/samples/event-modifiers-legacy/_config.js create mode 100644 test/validator/samples/event-modifiers-legacy/errors.json create mode 100644 test/validator/samples/event-modifiers-legacy/input.html diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index e05c587805..fad73bce93 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -613,6 +613,15 @@ export default class Element extends Node { }); } } + + if (component.options.legacy && (modifier === 'once' || modifier === 'passive')) { + // TODO this could be supported, but it would need a few changes to + // how event listeners work + component.error(handler, { + code: 'invalid-event-modifier', + message: `The '${modifier}' modifier cannot be used in legacy mode` + }); + } }); }); } diff --git a/test/validator/index.js b/test/validator/index.js index 852ac4191b..0f5ad18a6e 100644 --- a/test/validator/index.js +++ b/test/validator/index.js @@ -32,6 +32,7 @@ describe("validate", () => { warnings.push({ code, message, pos, start, end }); }, dev: config.dev, + legacy: config.legacy, generate: false }); diff --git a/test/validator/samples/event-modifiers-invalid/errors.json b/test/validator/samples/event-modifiers-invalid/errors.json index 858711708c..0de4e9aacf 100644 --- a/test/validator/samples/event-modifiers-invalid/errors.json +++ b/test/validator/samples/event-modifiers-invalid/errors.json @@ -1,15 +1,15 @@ [{ - "message": "Valid event modifiers are preventDefault, stopPropagation, capture, once or passive", - "code": "invalid-event-modifier", - "start": { - "line": 1, - "column": 8, - "character": 8 - }, - "end": { - "line": 1, - "column": 36, - "character": 36 - }, - "pos": 8 -}] \ No newline at end of file + "message": "Valid event modifiers are preventDefault, stopPropagation, capture, once or passive", + "code": "invalid-event-modifier", + "start": { + "line": 1, + "column": 8, + "character": 8 + }, + "end": { + "line": 1, + "column": 36, + "character": 36 + }, + "pos": 8 +}] diff --git a/test/validator/samples/event-modifiers-legacy/_config.js b/test/validator/samples/event-modifiers-legacy/_config.js new file mode 100644 index 0000000000..0179e05ec4 --- /dev/null +++ b/test/validator/samples/event-modifiers-legacy/_config.js @@ -0,0 +1,3 @@ +export default { + legacy: true +}; \ No newline at end of file diff --git a/test/validator/samples/event-modifiers-legacy/errors.json b/test/validator/samples/event-modifiers-legacy/errors.json new file mode 100644 index 0000000000..2e340b7b2f --- /dev/null +++ b/test/validator/samples/event-modifiers-legacy/errors.json @@ -0,0 +1,15 @@ +[{ + "message": "The 'once' modifier cannot be used in legacy mode", + "code": "invalid-event-modifier", + "start": { + "line": 1, + "column": 8, + "character": 8 + }, + "end": { + "line": 1, + "column": 37, + "character": 37 + }, + "pos": 8 +}] diff --git a/test/validator/samples/event-modifiers-legacy/input.html b/test/validator/samples/event-modifiers-legacy/input.html new file mode 100644 index 0000000000..3ef8edeee9 --- /dev/null +++ b/test/validator/samples/event-modifiers-legacy/input.html @@ -0,0 +1 @@ + \ No newline at end of file From 4082cdefe7726e9afb0d0e8c2a498824c3d4d839 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 13:46:17 -0400 Subject: [PATCH 07/11] implement event modifiers --- src/compile/nodes/Element.ts | 12 +-- src/compile/nodes/EventHandler.ts | 4 + .../render-dom/wrappers/Element/index.ts | 32 +++++-- test/js/samples/event-modifiers/expected.js | 91 +++++++++++++++++++ test/js/samples/event-modifiers/input.html | 19 ++++ test/parser/samples/event-handler/output.json | 1 + 6 files changed, 147 insertions(+), 12 deletions(-) create mode 100644 test/js/samples/event-modifiers/expected.js create mode 100644 test/js/samples/event-modifiers/input.html diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index fad73bce93..890b384f83 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -595,12 +595,7 @@ export default class Element extends Node { if (modifier === 'passive') { if (passiveEvents.has(handler.name)) { - const usesEvent = ( - handler.callee.name === 'event' || - handler.args.some(x => x.usesEvent) - ); - - if (!usesEvent) { + if (!handler.usesEventObject) { component.warn(handler, { code: 'redundant-event-modifier', message: `Touch event handlers that don't use the 'event' object are passive by default` @@ -623,6 +618,11 @@ export default class Element extends Node { }); } }); + + if (passiveEvents.has(handler.name) && !handler.usesEventObject) { + // touch/wheel events should be passive by default + handler.modifiers.add('passive'); + } }); } diff --git a/src/compile/nodes/EventHandler.ts b/src/compile/nodes/EventHandler.ts index 00b24f21f5..9676923d81 100644 --- a/src/compile/nodes/EventHandler.ts +++ b/src/compile/nodes/EventHandler.ts @@ -16,6 +16,7 @@ export default class EventHandler extends Node { usesComponent: boolean; usesContext: boolean; + usesEventObject: boolean; isCustomEvent: boolean; shouldHoist: boolean; @@ -42,11 +43,13 @@ export default class EventHandler extends Node { this.usesComponent = !validCalleeObjects.has(this.callee.name); this.usesContext = false; + this.usesEventObject = this.callee.name === 'event'; this.args = info.expression.arguments.map(param => { const expression = new Expression(component, this, scope, param); addToSet(this.dependencies, expression.dependencies); if (expression.usesContext) this.usesContext = true; + if (expression.usesEvent) this.usesEventObject = true; return expression; }); @@ -58,6 +61,7 @@ export default class EventHandler extends Node { this.args = null; this.usesComponent = true; this.usesContext = false; + this.usesEventObject = true; this.snippet = null; // TODO handle shorthand events here? } diff --git a/src/compile/render-dom/wrappers/Element/index.ts b/src/compile/render-dom/wrappers/Element/index.ts index 58b215889b..47c1bc57f3 100644 --- a/src/compile/render-dom/wrappers/Element/index.ts +++ b/src/compile/render-dom/wrappers/Element/index.ts @@ -649,8 +649,13 @@ export default class ElementWrapper extends Wrapper { ${handlerName}.destroy(); `); } else { + const modifiers = []; + if (handler.modifiers.has('preventDefault')) modifiers.push('event.preventDefault();'); + if (handler.modifiers.has('stopPropagation')) modifiers.push('event.stopPropagation();'); + const handlerFunction = deindent` function ${handlerName}(event) { + ${modifiers} ${handlerBody} } `; @@ -661,13 +666,28 @@ export default class ElementWrapper extends Wrapper { block.builders.init.addBlock(handlerFunction); } - block.builders.hydrate.addLine( - `@addListener(${this.var}, "${handler.name}", ${handlerName});` - ); + const opts = ['passive', 'once', 'capture'].filter(mod => handler.modifiers.has(mod)); + if (opts.length) { + const optString = (opts.length === 1 && opts[0] === 'capture') + ? 'true' + : `{ ${opts.map(opt => `${opt}: true`).join(', ')} }`; - block.builders.destroy.addLine( - `@removeListener(${this.var}, "${handler.name}", ${handlerName});` - ); + block.builders.hydrate.addLine( + `@addListener(${this.var}, "${handler.name}", ${handlerName}, ${optString});` + ); + + block.builders.destroy.addLine( + `@removeListener(${this.var}, "${handler.name}", ${handlerName}, ${optString});` + ); + } else { + block.builders.hydrate.addLine( + `@addListener(${this.var}, "${handler.name}", ${handlerName});` + ); + + block.builders.destroy.addLine( + `@removeListener(${this.var}, "${handler.name}", ${handlerName});` + ); + } } }); } diff --git a/test/js/samples/event-modifiers/expected.js b/test/js/samples/event-modifiers/expected.js new file mode 100644 index 0000000000..e0b355f216 --- /dev/null +++ b/test/js/samples/event-modifiers/expected.js @@ -0,0 +1,91 @@ +/* generated by Svelte vX.Y.Z */ +import { addListener, append, assign, createElement, createText, detachNode, init, insert, noop, proto, removeListener } from "svelte/shared.js"; + +var methods = { + handleTouchstart() { + // ... + }, + + handleClick() { + // ... + } +}; + +function create_main_fragment(component, ctx) { + var div, button0, text1, button1, text3, button2; + + function click_handler(event) { + event.preventDefault(); + event.stopPropagation(); + component.handleClick(); + } + + function click_handler_1(event) { + component.handleClick(); + } + + function click_handler_2(event) { + component.handleClick(); + } + + function touchstart_handler(event) { + component.handleTouchstart(); + } + + return { + c() { + div = createElement("div"); + button0 = createElement("button"); + button0.textContent = "click me"; + text1 = createText("\n\t"); + button1 = createElement("button"); + button1.textContent = "or me"; + text3 = createText("\n\t"); + button2 = createElement("button"); + button2.textContent = "or me!"; + addListener(button0, "click", click_handler); + addListener(button1, "click", click_handler_1, { once: true, capture: true }); + addListener(button2, "click", click_handler_2, true); + addListener(div, "touchstart", touchstart_handler, { passive: true }); + }, + + m(target, anchor) { + insert(target, div, anchor); + append(div, button0); + append(div, text1); + append(div, button1); + append(div, text3); + append(div, button2); + }, + + p: noop, + + d(detach) { + if (detach) { + detachNode(div); + } + + removeListener(button0, "click", click_handler); + removeListener(button1, "click", click_handler_1, { once: true, capture: true }); + removeListener(button2, "click", click_handler_2, true); + removeListener(div, "touchstart", touchstart_handler, { passive: true }); + } + }; +} + +function SvelteComponent(options) { + init(this, options); + this._state = assign({}, options.data); + this._intro = true; + + this._fragment = create_main_fragment(this, this._state); + + if (options.target) { + this._fragment.c(); + this._mount(options.target, options.anchor); + } +} + +assign(SvelteComponent.prototype, proto); +assign(SvelteComponent.prototype, methods); +export default SvelteComponent; \ No newline at end of file diff --git a/test/js/samples/event-modifiers/input.html b/test/js/samples/event-modifiers/input.html new file mode 100644 index 0000000000..4726d96c70 --- /dev/null +++ b/test/js/samples/event-modifiers/input.html @@ -0,0 +1,19 @@ +
+ + + +
+ + \ No newline at end of file diff --git a/test/parser/samples/event-handler/output.json b/test/parser/samples/event-handler/output.json index c297572b16..617c2fde71 100644 --- a/test/parser/samples/event-handler/output.json +++ b/test/parser/samples/event-handler/output.json @@ -15,6 +15,7 @@ "end": 45, "type": "EventHandler", "name": "click", + "modifiers": [], "expression": { "type": "CallExpression", "start": 18, From 3f3954102f1cb560e43cf1dac871c42c8530cdcc Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 13:58:43 -0400 Subject: [PATCH 08/11] disallow passive|preventDefault combo --- src/compile/nodes/Element.ts | 9 ++++++++- .../event-modifiers-invalid-passive/errors.json | 15 +++++++++++++++ .../event-modifiers-invalid-passive/input.html | 3 +++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 test/validator/samples/event-modifiers-invalid-passive/errors.json create mode 100644 test/validator/samples/event-modifiers-invalid-passive/input.html diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index 890b384f83..cf3a1323f2 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -585,6 +585,13 @@ export default class Element extends Node { ]); this.handlers.forEach(handler => { + if (handler.modifiers.has('passive') && handler.modifiers.has('preventDefault')) { + component.error(handler, { + code: 'invalid-event-modifier', + message: `The 'passive' and 'preventDefault' modifiers cannot be used together` + }); + } + handler.modifiers.forEach(modifier => { if (!validModifiers.has(modifier)) { component.error(handler, { @@ -619,7 +626,7 @@ export default class Element extends Node { } }); - if (passiveEvents.has(handler.name) && !handler.usesEventObject) { + if (passiveEvents.has(handler.name) && !handler.usesEventObject && !handler.modifiers.has('preventDefault')) { // touch/wheel events should be passive by default handler.modifiers.add('passive'); } diff --git a/test/validator/samples/event-modifiers-invalid-passive/errors.json b/test/validator/samples/event-modifiers-invalid-passive/errors.json new file mode 100644 index 0000000000..90fce9eb44 --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid-passive/errors.json @@ -0,0 +1,15 @@ +[{ + "message": "The 'passive' and 'preventDefault' modifiers cannot be used together", + "code": "invalid-event-modifier", + "start": { + "line": 1, + "column": 5, + "character": 5 + }, + "end": { + "line": 1, + "column": 52, + "character": 52 + }, + "pos": 5 +}] diff --git a/test/validator/samples/event-modifiers-invalid-passive/input.html b/test/validator/samples/event-modifiers-invalid-passive/input.html new file mode 100644 index 0000000000..e51cd57ae7 --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid-passive/input.html @@ -0,0 +1,3 @@ +
+ oops +
\ No newline at end of file From 28610747b97cd7e90618dd889b5d1bccf3c25987 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 14:09:13 -0400 Subject: [PATCH 09/11] remove unused code --- src/utils/getEventModifiers.ts | 36 ---------------------------------- 1 file changed, 36 deletions(-) delete mode 100644 src/utils/getEventModifiers.ts diff --git a/src/utils/getEventModifiers.ts b/src/utils/getEventModifiers.ts deleted file mode 100644 index 554a2c79af..0000000000 --- a/src/utils/getEventModifiers.ts +++ /dev/null @@ -1,36 +0,0 @@ -import EventHandler from '../compile/nodes/EventHandler'; -import deindent from '../utils/deindent'; - -export default function getEventModifiers(handlerName: String) { - // Ignore first element because it's the event name, i.e. click - let modifiers = handlerName.split('|').slice(1); - - let eventModifiers = modifiers.reduce((acc, m) => { - if (m === 'stopPropagation') - acc.bodyModifiers += 'event.stopPropagation();\n'; - else if (m === 'preventDefault') - acc.bodyModifiers += 'event.preventDefault();\n'; - else if (m === 'capture') - acc.optionModifiers[m] = true; - else if (m === 'once') - acc.optionModifiers[m] = true; - else if (m === 'passive') - acc.optionModifiers[m] = true; - - return acc; - }, { - bodyModifiers: '', - optionModifiers: { - capture: false, - once: false, - passive: false, - } - }); - - if (eventModifiers.bodyModifiers !== '') - eventModifiers.bodyModifiers = deindent` - ${eventModifiers.bodyModifiers} - `; - - return eventModifiers; -} \ No newline at end of file From c7ede66dc12638e059afbd8f54a9f880cbc3ebbd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 14:09:21 -0400 Subject: [PATCH 10/11] hoist these --- src/compile/nodes/Element.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index cf3a1323f2..9fa630124e 100644 --- a/src/compile/nodes/Element.ts +++ b/src/compile/nodes/Element.ts @@ -57,6 +57,22 @@ const a11yRequiredContent = new Set([ const invisibleElements = new Set(['meta', 'html', 'script', 'style']); +const validModifiers = new Set([ + 'preventDefault', + 'stopPropagation', + 'capture', + 'once', + 'passive' +]); + +const passiveEvents = new Set([ + 'wheel', + 'touchstart', + 'touchmove', + 'touchend', + 'touchcancel' +]); + export default class Element extends Node { type: 'Element'; name: string; @@ -568,22 +584,6 @@ export default class Element extends Node { validateEventHandlers() { const { component } = this; - const validModifiers = new Set([ - 'preventDefault', - 'stopPropagation', - 'capture', - 'once', - 'passive' - ]); - - const passiveEvents = new Set([ - 'wheel', - 'touchstart', - 'touchmove', - 'touchend', - 'touchcancel' - ]); - this.handlers.forEach(handler => { if (handler.modifiers.has('passive') && handler.modifiers.has('preventDefault')) { component.error(handler, { From 5b997d2f85fc171378e7d8a3cfa4912e930fd796 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 28 Oct 2018 14:12:29 -0400 Subject: [PATCH 11/11] remove unused code --- .../wrappers/shared/EventHandler.ts | 62 ------------------- 1 file changed, 62 deletions(-) delete mode 100644 src/compile/render-dom/wrappers/shared/EventHandler.ts diff --git a/src/compile/render-dom/wrappers/shared/EventHandler.ts b/src/compile/render-dom/wrappers/shared/EventHandler.ts deleted file mode 100644 index 08d3a0e14a..0000000000 --- a/src/compile/render-dom/wrappers/shared/EventHandler.ts +++ /dev/null @@ -1,62 +0,0 @@ -import Renderer from '../../Renderer'; -import Block from '../../Block'; -import Wrapper from './Wrapper'; -import EventHandler from '../../../nodes/EventHandler'; -import validCalleeObjects from '../../../../utils/validCalleeObjects'; - -export default class EventHandlerWrapper extends Wrapper { - node: EventHandler; - - constructor( - renderer: Renderer, - block: Block, - parent: Wrapper, - node: EventHandler, - stripWhitespace: boolean, - nextSibling: Wrapper - ) { - super(renderer, block, parent, node); - } - - render(block: Block, parentNode: string, parentNodes: string) { - const { renderer } = this; - const { component } = renderer; - - const hoisted = this.node.shouldHoist; - - if (this.node.insertionPoint === null) return; // TODO handle shorthand events here? - - if (!validCalleeObjects.has(this.node.callee.name)) { - const component_name = hoisted ? `component` : block.alias(`component`); - - // allow event.stopPropagation(), this.select() etc - // TODO verify that it's a valid callee (i.e. built-in or declared method) - if (this.node.callee.name[0] === '$' && !component.methods.has(this.node.callee.name)) { - component.code.overwrite( - this.node.insertionPoint, - this.node.insertionPoint + 1, - `${component_name}.store.` - ); - } else { - component.code.prependRight( - this.node.insertionPoint, - `${component_name}.` - ); - } - } - - if (this.node.isCustomEvent) { - this.node.args.forEach(arg => { - arg.overwriteThis(this.parent.var); - }); - - if (this.node.callee && this.node.callee.name === 'this') { - const node = this.node.callee.nodes[0]; - component.code.overwrite(node.start, node.end, this.parent.var, { - storeName: true, - contentOnly: true - }); - } - } - } -} \ No newline at end of file