diff --git a/src/compile/nodes/Element.ts b/src/compile/nodes/Element.ts index c060edc1f868..9fa630124e1a 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)$/; @@ -56,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; @@ -228,6 +245,7 @@ export default class Element extends Node { this.validateAttributes(); this.validateBindings(); this.validateContent(); + this.validateEventHandlers(); } validateAttributes() { @@ -563,6 +581,58 @@ export default class Element extends Node { } } + validateEventHandlers() { + const { component } = this; + + 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, { + code: 'invalid-event-modifier', + message: `Valid event modifiers are ${list([...validModifiers])}` + }); + } + + if (modifier === 'passive') { + if (passiveEvents.has(handler.name)) { + 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` + }); + } + } else { + component.warn(handler, { + code: 'redundant-event-modifier', + message: `The passive modifier only works with wheel and touch events` + }); + } + } + + 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` + }); + } + }); + + if (passiveEvents.has(handler.name) && !handler.usesEventObject && !handler.modifiers.has('preventDefault')) { + // touch/wheel events should be passive by default + handler.modifiers.add('passive'); + } + }); + } + 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..9676923d8105 100644 --- a/src/compile/nodes/EventHandler.ts +++ b/src/compile/nodes/EventHandler.ts @@ -9,12 +9,14 @@ const validBuiltins = new Set(['set', 'fire', 'destroy']); export default class EventHandler extends Node { name: string; + modifiers: Set; dependencies: Set; expression: Node; callee: any; // TODO usesComponent: boolean; usesContext: boolean; + usesEventObject: boolean; isCustomEvent: boolean; shouldHoist: boolean; @@ -26,6 +28,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(); @@ -39,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; }); @@ -55,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/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/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/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 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/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, 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-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 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..0de4e9aacf38 --- /dev/null +++ b/test/validator/samples/event-modifiers-invalid/errors.json @@ -0,0 +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 +}] 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 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 diff --git a/test/validator/samples/event-modifiers-redundant/input.html b/test/validator/samples/event-modifiers-redundant/input.html new file mode 100644 index 000000000000..2a2d390da421 --- /dev/null +++ b/test/validator/samples/event-modifiers-redundant/input.html @@ -0,0 +1,16 @@ + +
+ + \ 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 + } +]