From a6c1a12e905193ce0628fa26391b6685a918807c 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 403b9cce8bee..7c42ab71e2f4 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 03aaf8aaeb39..0fb8f78f95fe 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 000000000000..f194e987e2bf --- /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 26360d4ced306bf103f8df4ff71b9afdca0e36a7 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 097dc9e1b589..fa515f700cad 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 50ba725fa96a..b159277bf6c7 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 5bf43ec519c4..b50393ef21fc 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 ec9116f544ca..17e820a067cf 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 adfc0e3e458f05293c42f1098b5b10a96640f73f 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 df499a9fd595..b1140709931b 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 000000000000..99737f50d23b --- /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 000000000000..27dacdbc2d3a --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid/input.html @@ -0,0 +1 @@ + \ No newline at end of file From 7c4b9a5a41dbbcc8e7dcba07c2b9ad78867d89f4 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 f194e987e2bf..554a2c79af21 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 b1140709931b..f1df2f741c39 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 99737f50d23b..af1cca83e46e 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 aa203973e09bcc789b62fc4721aea7ec9a369895 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 c060edc1f868..e05c5878054f 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 05aa51d1da6f..00b24f21f5c6 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 6acb6282ec29..dc6f77d82e34 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 f1955ffcbc2d..6993f3ebdadd 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 000000000000..7b6cae16d487 --- /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 8ec02b336d3d4c9d3eedc2d4ced11268e83e2ab3 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 e05c5878054f..fad73bce9394 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 852ac4191be8..0f5ad18a6e44 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 858711708cbd..0de4e9aacf38 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 000000000000..0179e05ec474 --- /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 000000000000..2e340b7b2f6c --- /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 000000000000..3ef8edeee9bb --- /dev/null +++ b/test/validator/samples/event-modifiers-legacy/input.html @@ -0,0 +1 @@ + \ No newline at end of file From 82b1b75afe3eb0ef3922abe707c6acc3e2a6d188 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 fad73bce9394..890b384f8330 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 00b24f21f5c6..9676923d8105 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 58b215889b78..47c1bc57f32e 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 000000000000..e0b355f21677 --- /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 000000000000..4726d96c70b3 --- /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 c297572b1645..617c2fde71b7 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 b3b95d4ee612c92ac53292c1594adf1c0d1f9c98 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 890b384f8330..cf3a1323f28d 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 000000000000..90fce9eb4458 --- /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 000000000000..e51cd57ae79e --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid-passive/input.html @@ -0,0 +1,3 @@ +
+ oops +
\ No newline at end of file From da3a45c526e124e025f71cb3de386fc1d303e64e 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 554a2c79af21..000000000000 --- 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 10011a5142cd4c07786cb9abc3092720b306b5b3 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 cf3a1323f28d..9fa630124e1a 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 f1d704493bf91cf999ae248cfbd39ea6024b0192 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 08d3a0e14afb..000000000000 --- 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