fix(v-on): refactor DOM event options modifer handling

fix #1567

Previously multiple `v-on` handlers with different event attach option
modifers (`.once`, `.capture` and `.passive`) are generated as an array
of objects in the form of `[{ handler, options }]` - however, this
makes it pretty complex for `runtime-dom` to properly handle all
possible value permutations, as each handler may need to be attached
with different options.

With this commit, they are now generated as event props with different
keys - e.g. `v-on:click.capture` is now generated as a prop named
`onClick.capture`. This allows them to be patched as separate props
which makes the runtime handling much simpler.
This commit is contained in:
Evan You 2020-07-14 11:48:05 -04:00
parent 9152a89016
commit 380c6792d8
8 changed files with 200 additions and 189 deletions

View File

@ -6,7 +6,9 @@ import {
CompilerOptions, CompilerOptions,
ErrorCodes, ErrorCodes,
NodeTypes, NodeTypes,
VNodeCall VNodeCall,
helperNameMap,
CAPITALIZE
} from '../../src' } from '../../src'
import { transformOn } from '../../src/transforms/vOn' import { transformOn } from '../../src/transforms/vOn'
import { transformElement } from '../../src/transforms/transformElement' import { transformElement } from '../../src/transforms/transformElement'
@ -73,7 +75,11 @@ describe('compiler: transform v-on', () => {
{ {
key: { key: {
type: NodeTypes.COMPOUND_EXPRESSION, type: NodeTypes.COMPOUND_EXPRESSION,
children: [`"on" + (`, { content: `event` }, `)`] children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `event` },
`)`
]
}, },
value: { value: {
type: NodeTypes.SIMPLE_EXPRESSION, type: NodeTypes.SIMPLE_EXPRESSION,
@ -94,7 +100,11 @@ describe('compiler: transform v-on', () => {
{ {
key: { key: {
type: NodeTypes.COMPOUND_EXPRESSION, type: NodeTypes.COMPOUND_EXPRESSION,
children: [`"on" + (`, { content: `_ctx.event` }, `)`] children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `_ctx.event` },
`)`
]
}, },
value: { value: {
type: NodeTypes.SIMPLE_EXPRESSION, type: NodeTypes.SIMPLE_EXPRESSION,
@ -116,7 +126,7 @@ describe('compiler: transform v-on', () => {
key: { key: {
type: NodeTypes.COMPOUND_EXPRESSION, type: NodeTypes.COMPOUND_EXPRESSION,
children: [ children: [
`"on" + (`, `"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `_ctx.event` }, { content: `_ctx.event` },
`(`, `(`,
{ content: `_ctx.foo` }, { content: `_ctx.foo` },

View File

@ -5,16 +5,15 @@ import {
ElementNode, ElementNode,
ObjectExpression, ObjectExpression,
NodeTypes, NodeTypes,
VNodeCall VNodeCall,
helperNameMap,
CAPITALIZE
} from '@vue/compiler-core' } from '@vue/compiler-core'
import { transformOn } from '../../src/transforms/vOn' import { transformOn } from '../../src/transforms/vOn'
import { V_ON_WITH_MODIFIERS, V_ON_WITH_KEYS } from '../../src/runtimeHelpers' import { V_ON_WITH_MODIFIERS, V_ON_WITH_KEYS } from '../../src/runtimeHelpers'
import { transformElement } from '../../../compiler-core/src/transforms/transformElement' import { transformElement } from '../../../compiler-core/src/transforms/transformElement'
import { transformExpression } from '../../../compiler-core/src/transforms/transformExpression' import { transformExpression } from '../../../compiler-core/src/transforms/transformExpression'
import { import { genFlagText } from '../../../compiler-core/__tests__/testUtils'
createObjectMatcher,
genFlagText
} from '../../../compiler-core/__tests__/testUtils'
import { PatchFlags } from '@vue/shared' import { PatchFlags } from '@vue/shared'
function parseWithVOn(template: string, options: CompilerOptions = {}) { function parseWithVOn(template: string, options: CompilerOptions = {}) {
@ -83,29 +82,28 @@ describe('compiler-dom: transform v-on', () => {
}) })
expect(prop).toMatchObject({ expect(prop).toMatchObject({
type: NodeTypes.JS_PROPERTY, type: NodeTypes.JS_PROPERTY,
value: createObjectMatcher({ key: {
handler: { content: `onClick.capture.passive`
},
value: {
callee: V_ON_WITH_MODIFIERS, callee: V_ON_WITH_MODIFIERS,
arguments: [{ content: '_ctx.test' }, '["stop"]'] arguments: [{ content: '_ctx.test' }, '["stop"]']
}, }
options: createObjectMatcher({
capture: { content: 'true', isStatic: false },
passive: { content: 'true', isStatic: false }
})
})
}) })
}) })
it('should wrap keys guard for keyboard events or dynamic events', () => { it('should wrap keys guard for keyboard events or dynamic events', () => {
const { const {
props: [prop] props: [prop]
} = parseWithVOn(`<div @keyDown.stop.capture.ctrl.a="test"/>`, { } = parseWithVOn(`<div @keydown.stop.capture.ctrl.a="test"/>`, {
prefixIdentifiers: true prefixIdentifiers: true
}) })
expect(prop).toMatchObject({ expect(prop).toMatchObject({
type: NodeTypes.JS_PROPERTY, type: NodeTypes.JS_PROPERTY,
value: createObjectMatcher({ key: {
handler: { content: `onKeydown.capture`
},
value: {
callee: V_ON_WITH_KEYS, callee: V_ON_WITH_KEYS,
arguments: [ arguments: [
{ {
@ -114,11 +112,7 @@ describe('compiler-dom: transform v-on', () => {
}, },
'["a"]' '["a"]'
] ]
}, }
options: createObjectMatcher({
capture: { content: 'true', isStatic: false }
})
})
}) })
}) })
@ -206,9 +200,21 @@ describe('compiler-dom: transform v-on', () => {
type: NodeTypes.COMPOUND_EXPRESSION, type: NodeTypes.COMPOUND_EXPRESSION,
children: [ children: [
`(`, `(`,
{ children: [`"on" + (`, { content: 'event' }, `)`] }, {
`).toLowerCase() === "onclick" ? "onContextmenu" : (`, children: [
{ children: [`"on" + (`, { content: 'event' }, `)`] }, `"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`) === "onClick" ? "onContextmenu" : (`,
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`)` `)`
] ]
}) })
@ -232,9 +238,21 @@ describe('compiler-dom: transform v-on', () => {
type: NodeTypes.COMPOUND_EXPRESSION, type: NodeTypes.COMPOUND_EXPRESSION,
children: [ children: [
`(`, `(`,
{ children: [`"on" + (`, { content: 'event' }, `)`] }, {
`).toLowerCase() === "onclick" ? "onMouseup" : (`, children: [
{ children: [`"on" + (`, { content: 'event' }, `)`] }, `"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`) === "onClick" ? "onMouseup" : (`,
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`)` `)`
] ]
}) })
@ -254,24 +272,17 @@ describe('compiler-dom: transform v-on', () => {
expect((root as any).children[0].codegenNode.patchFlag).toBe( expect((root as any).children[0].codegenNode.patchFlag).toBe(
genFlagText(PatchFlags.HYDRATE_EVENTS) genFlagText(PatchFlags.HYDRATE_EVENTS)
) )
expect(prop.value).toMatchObject({ expect(prop).toMatchObject({
key: {
content: `onKeyup.capture`
},
value: {
type: NodeTypes.JS_CACHE_EXPRESSION, type: NodeTypes.JS_CACHE_EXPRESSION,
index: 1, index: 1,
value: {
type: NodeTypes.JS_OBJECT_EXPRESSION,
properties: [
{
key: { content: 'handler' },
value: { value: {
type: NodeTypes.JS_CALL_EXPRESSION, type: NodeTypes.JS_CALL_EXPRESSION,
callee: V_ON_WITH_KEYS callee: V_ON_WITH_KEYS
} }
},
{
key: { content: 'options' },
value: { type: NodeTypes.JS_OBJECT_EXPRESSION }
}
]
} }
}) })
}) })

View File

@ -3,7 +3,6 @@ import {
DirectiveTransform, DirectiveTransform,
createObjectProperty, createObjectProperty,
createCallExpression, createCallExpression,
createObjectExpression,
createSimpleExpression, createSimpleExpression,
NodeTypes, NodeTypes,
createCompoundExpression, createCompoundExpression,
@ -80,7 +79,7 @@ const transformClick = (key: ExpressionNode, event: string) => {
? createCompoundExpression([ ? createCompoundExpression([
`(`, `(`,
key, key,
`).toLowerCase() === "onclick" ? "${event}" : (`, `) === "onClick" ? "${event}" : (`,
key, key,
`)` `)`
]) ])
@ -126,19 +125,15 @@ export const transformOn: DirectiveTransform = (dir, node, context) => {
} }
if (eventOptionModifiers.length) { if (eventOptionModifiers.length) {
handlerExp = createObjectExpression([ key = isStaticExp(key)
createObjectProperty('handler', handlerExp), ? createSimpleExpression(
createObjectProperty( `${key.content}.${eventOptionModifiers.join(`.`)}`,
'options', true
createObjectExpression(
eventOptionModifiers.map(modifier =>
createObjectProperty(
modifier,
createSimpleExpression('true', false)
)
)
)
) )
: createCompoundExpression([
`(`,
key,
`) + ".${eventOptionModifiers.join(`.`)}"`
]) ])
} }

View File

@ -160,30 +160,61 @@ describe('component: emit', () => {
expect(`event validation failed for event "foo"`).toHaveBeenWarned() expect(`event validation failed for event "foo"`).toHaveBeenWarned()
}) })
test('isEmitListener', () => { test('.once', () => {
const Foo = defineComponent({
render() {},
emits: {
foo: null
},
created() {
this.$emit('foo')
this.$emit('foo')
}
})
const fn = jest.fn()
render(
h(Foo, {
'onFoo.once': fn
}),
nodeOps.createElement('div')
)
expect(fn).toHaveBeenCalledTimes(1)
})
describe('isEmitListener', () => {
test('array option', () => {
const def1 = { emits: ['click'] } const def1 = { emits: ['click'] }
expect(isEmitListener(def1, 'onClick')).toBe(true) expect(isEmitListener(def1, 'onClick')).toBe(true)
expect(isEmitListener(def1, 'onclick')).toBe(false) expect(isEmitListener(def1, 'onclick')).toBe(false)
expect(isEmitListener(def1, 'onBlick')).toBe(false) expect(isEmitListener(def1, 'onBlick')).toBe(false)
})
test('object option', () => {
const def2 = { emits: { click: null } } const def2 = { emits: { click: null } }
expect(isEmitListener(def2, 'onClick')).toBe(true) expect(isEmitListener(def2, 'onClick')).toBe(true)
expect(isEmitListener(def2, 'onclick')).toBe(false) expect(isEmitListener(def2, 'onclick')).toBe(false)
expect(isEmitListener(def2, 'onBlick')).toBe(false) expect(isEmitListener(def2, 'onBlick')).toBe(false)
})
test('with mixins and extends', () => {
const mixin1 = { emits: ['foo'] } const mixin1 = { emits: ['foo'] }
const mixin2 = { emits: ['bar'] } const mixin2 = { emits: ['bar'] }
const extend = { emits: ['baz'] } const extend = { emits: ['baz'] }
const def3 = { const def3 = {
emits: { click: null },
mixins: [mixin1, mixin2], mixins: [mixin1, mixin2],
extends: extend extends: extend
} }
expect(isEmitListener(def3, 'onClick')).toBe(true)
expect(isEmitListener(def3, 'onFoo')).toBe(true) expect(isEmitListener(def3, 'onFoo')).toBe(true)
expect(isEmitListener(def3, 'onBar')).toBe(true) expect(isEmitListener(def3, 'onBar')).toBe(true)
expect(isEmitListener(def3, 'onBaz')).toBe(true) expect(isEmitListener(def3, 'onBaz')).toBe(true)
expect(isEmitListener(def3, 'onclick')).toBe(false) expect(isEmitListener(def3, 'onclick')).toBe(false)
expect(isEmitListener(def3, 'onBlick')).toBe(false) expect(isEmitListener(def3, 'onBlick')).toBe(false)
}) })
test('.once listeners', () => {
const def2 = { emits: { click: null } }
expect(isEmitListener(def2, 'onClick.once')).toBe(true)
expect(isEmitListener(def2, 'onclick.once')).toBe(false)
})
})
}) })

View File

@ -246,6 +246,8 @@ export interface ComponentInternalInstance {
slots: InternalSlots slots: InternalSlots
refs: Data refs: Data
emit: EmitFn emit: EmitFn
// used for keeping track of .once event handlers on components
emitted: Record<string, boolean> | null
/** /**
* setup related * setup related
@ -396,7 +398,8 @@ export function createComponentInstance(
rtg: null, rtg: null,
rtc: null, rtc: null,
ec: null, ec: null,
emit: null as any // to be set immediately emit: null as any, // to be set immediately
emitted: null
} }
if (__DEV__) { if (__DEV__) {
instance.ctx = createRenderContext(instance) instance.ctx = createRenderContext(instance)

View File

@ -67,12 +67,21 @@ export function emit(
} }
} }
let handler = props[`on${capitalize(event)}`] let handlerName = `on${capitalize(event)}`
let handler = props[handlerName]
// for v-model update:xxx events, also trigger kebab-case equivalent // for v-model update:xxx events, also trigger kebab-case equivalent
// for props passed via kebab-case // for props passed via kebab-case
if (!handler && event.startsWith('update:')) { if (!handler && event.startsWith('update:')) {
event = hyphenate(event) handlerName = `on${capitalize(hyphenate(event))}`
handler = props[`on${capitalize(event)}`] handler = props[handlerName]
}
if (!handler) {
handler = props[handlerName + `.once`]
if (!instance.emitted) {
;(instance.emitted = {} as Record<string, boolean>)[handlerName] = true
} else if (instance.emitted[handlerName]) {
return
}
} }
if (handler) { if (handler) {
callWithAsyncErrorHandling( callWithAsyncErrorHandling(
@ -123,13 +132,13 @@ function normalizeEmitsOptions(
// e.g. With `emits: { click: null }`, props named `onClick` and `onclick` are // e.g. With `emits: { click: null }`, props named `onClick` and `onclick` are
// both considered matched listeners. // both considered matched listeners.
export function isEmitListener(comp: Component, key: string): boolean { export function isEmitListener(comp: Component, key: string): boolean {
if (!isOn(key)) { let emits: ObjectEmitsOptions | undefined
if (!isOn(key) || !(emits = normalizeEmitsOptions(comp))) {
return false return false
} }
const emits = normalizeEmitsOptions(comp) key = key.replace(/\.once$/, '')
return ( return (
!!emits && hasOwn(emits, key[2].toLowerCase() + key.slice(3)) ||
(hasOwn(emits, key[2].toLowerCase() + key.slice(3)) || hasOwn(emits, key.slice(2))
hasOwn(emits, key.slice(2)))
) )
} }

View File

@ -57,17 +57,11 @@ describe(`runtime-dom: events patching`, () => {
expect(fn).not.toHaveBeenCalled() expect(fn).not.toHaveBeenCalled()
}) })
it('should support event options', async () => { it('should support event option modifiers', async () => {
const el = document.createElement('div') const el = document.createElement('div')
const event = new Event('click') const event = new Event('click')
const fn = jest.fn() const fn = jest.fn()
const nextValue = { patchProp(el, 'onClick.once.capture', null, fn)
handler: fn,
options: {
once: true
}
}
patchProp(el, 'onClick', null, nextValue)
el.dispatchEvent(event) el.dispatchEvent(event)
await timeout() await timeout()
el.dispatchEvent(event) el.dispatchEvent(event)
@ -75,39 +69,12 @@ describe(`runtime-dom: events patching`, () => {
expect(fn).toHaveBeenCalledTimes(1) expect(fn).toHaveBeenCalledTimes(1)
}) })
it('should support varying event options', async () => {
const el = document.createElement('div')
const event = new Event('click')
const prevFn = jest.fn()
const nextFn = jest.fn()
const nextValue = {
handler: nextFn,
options: {
once: true
}
}
patchProp(el, 'onClick', null, prevFn)
patchProp(el, 'onClick', prevFn, nextValue)
el.dispatchEvent(event)
await timeout()
el.dispatchEvent(event)
await timeout()
expect(prevFn).not.toHaveBeenCalled()
expect(nextFn).toHaveBeenCalledTimes(1)
})
it('should unassign event handler with options', async () => { it('should unassign event handler with options', async () => {
const el = document.createElement('div') const el = document.createElement('div')
const event = new Event('click') const event = new Event('click')
const fn = jest.fn() const fn = jest.fn()
const nextValue = { patchProp(el, 'onClick.capture', null, fn)
handler: fn, patchProp(el, 'onClick.capture', fn, null)
options: {
once: true
}
}
patchProp(el, 'onClick', null, nextValue)
patchProp(el, 'onClick', nextValue, null)
el.dispatchEvent(event) el.dispatchEvent(event)
await timeout() await timeout()
el.dispatchEvent(event) el.dispatchEvent(event)

View File

@ -1,4 +1,4 @@
import { EMPTY_OBJ, isArray } from '@vue/shared' import { isArray } from '@vue/shared'
import { import {
ComponentInternalInstance, ComponentInternalInstance,
callWithAsyncErrorHandling callWithAsyncErrorHandling
@ -14,12 +14,6 @@ type EventValue = (Function | Function[]) & {
invoker?: Invoker | null invoker?: Invoker | null
} }
type EventValueWithOptions = {
handler: EventValue
options: AddEventListenerOptions
invoker?: Invoker | null
}
// Async edge case fix requires storing an event listener's attach timestamp. // Async edge case fix requires storing an event listener's attach timestamp.
let _getNow: () => number = Date.now let _getNow: () => number = Date.now
@ -67,52 +61,43 @@ export function removeEventListener(
export function patchEvent( export function patchEvent(
el: Element, el: Element,
rawName: string, rawName: string,
prevValue: EventValueWithOptions | EventValue | null, prevValue: EventValue | null,
nextValue: EventValueWithOptions | EventValue | null, nextValue: EventValue | null,
instance: ComponentInternalInstance | null = null instance: ComponentInternalInstance | null = null
) { ) {
const name = rawName.slice(2).toLowerCase()
const prevOptions = prevValue && 'options' in prevValue && prevValue.options
const nextOptions = nextValue && 'options' in nextValue && nextValue.options
const invoker = prevValue && prevValue.invoker const invoker = prevValue && prevValue.invoker
const value = if (nextValue && invoker) {
nextValue && 'handler' in nextValue ? nextValue.handler : nextValue // patch
if (prevOptions || nextOptions) {
const prev = prevOptions || EMPTY_OBJ
const next = nextOptions || EMPTY_OBJ
if (
prev.capture !== next.capture ||
prev.passive !== next.passive ||
prev.once !== next.once
) {
if (invoker) {
removeEventListener(el, name, invoker, prev)
}
if (nextValue && value) {
const invoker = createInvoker(value, instance)
nextValue.invoker = invoker
addEventListener(el, name, invoker, next)
}
return
}
}
if (nextValue && value) {
if (invoker) {
;(prevValue as EventValue).invoker = null ;(prevValue as EventValue).invoker = null
invoker.value = value invoker.value = nextValue
nextValue.invoker = invoker nextValue.invoker = invoker
} else { } else {
addEventListener( const [name, options] = parseName(rawName)
el, if (nextValue) {
name, addEventListener(el, name, createInvoker(nextValue, instance), options)
createInvoker(value, instance),
nextOptions || void 0
)
}
} else if (invoker) { } else if (invoker) {
removeEventListener(el, name, invoker, prevOptions || void 0) // remove
removeEventListener(el, name, invoker, options)
}
}
}
const optionsModifierRE = /\.(once|passive|capture)\b/g
function parseName(name: string): [string, EventListenerOptions | undefined] {
name = name.slice(2).toLowerCase()
if (optionsModifierRE.test(name)) {
const options: EventListenerOptions = {}
name = name.replace(
optionsModifierRE,
(_, key: keyof EventListenerOptions) => {
options[key] = true
return ''
}
)
return [name, options]
} else {
return [name, undefined]
} }
} }