Skip to content

Commit

Permalink
fix(v-on): refactor DOM event options modifer handling
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@yyx990803
yyx990803 committed Jul 14, 2020
1 parent 9152a89 commit 380c679
Show file tree
Hide file tree
Showing 8 changed files with 202 additions and 191 deletions.
18 changes: 14 additions & 4 deletions packages/compiler-core/__tests__/transforms/vOn.spec.ts
View file
Expand Up @@ -6,7 +6,9 @@ import {
CompilerOptions,
ErrorCodes,
NodeTypes,
VNodeCall
VNodeCall,
helperNameMap,
CAPITALIZE
} from '../../src'
import { transformOn } from '../../src/transforms/vOn'
import { transformElement } from '../../src/transforms/transformElement'
Expand Down Expand Up @@ -73,7 +75,11 @@ describe('compiler: transform v-on', () => {
{
key: {
type: NodeTypes.COMPOUND_EXPRESSION,
children: [`"on" + (`, { content: `event` }, `)`]
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `event` },
`)`
]
},
value: {
type: NodeTypes.SIMPLE_EXPRESSION,
Expand All @@ -94,7 +100,11 @@ describe('compiler: transform v-on', () => {
{
key: {
type: NodeTypes.COMPOUND_EXPRESSION,
children: [`"on" + (`, { content: `_ctx.event` }, `)`]
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `_ctx.event` },
`)`
]
},
value: {
type: NodeTypes.SIMPLE_EXPRESSION,
Expand All @@ -116,7 +126,7 @@ describe('compiler: transform v-on', () => {
key: {
type: NodeTypes.COMPOUND_EXPRESSION,
children: [
`"on" + (`,
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: `_ctx.event` },
`(`,
{ content: `_ctx.foo` },
Expand Down
119 changes: 65 additions & 54 deletions packages/compiler-dom/__tests__/transforms/vOn.spec.ts
View file
Expand Up @@ -5,16 +5,15 @@ import {
ElementNode,
ObjectExpression,
NodeTypes,
VNodeCall
VNodeCall,
helperNameMap,
CAPITALIZE
} from '@vue/compiler-core'
import { transformOn } from '../../src/transforms/vOn'
import { V_ON_WITH_MODIFIERS, V_ON_WITH_KEYS } from '../../src/runtimeHelpers'
import { transformElement } from '../../../compiler-core/src/transforms/transformElement'
import { transformExpression } from '../../../compiler-core/src/transforms/transformExpression'
import {
createObjectMatcher,
genFlagText
} from '../../../compiler-core/__tests__/testUtils'
import { genFlagText } from '../../../compiler-core/__tests__/testUtils'
import { PatchFlags } from '@vue/shared'

function parseWithVOn(template: string, options: CompilerOptions = {}) {
Expand Down Expand Up @@ -83,42 +82,37 @@ describe('compiler-dom: transform v-on', () => {
})
expect(prop).toMatchObject({
type: NodeTypes.JS_PROPERTY,
value: createObjectMatcher({
handler: {
callee: V_ON_WITH_MODIFIERS,
arguments: [{ content: '_ctx.test' }, '["stop"]']
},
options: createObjectMatcher({
capture: { content: 'true', isStatic: false },
passive: { content: 'true', isStatic: false }
})
})
key: {
content: `onClick.capture.passive`
},
value: {
callee: V_ON_WITH_MODIFIERS,
arguments: [{ content: '_ctx.test' }, '["stop"]']
}
})
})

it('should wrap keys guard for keyboard events or dynamic events', () => {
const {
props: [prop]
} = parseWithVOn(`<div @keyDown.stop.capture.ctrl.a="test"/>`, {
} = parseWithVOn(`<div @keydown.stop.capture.ctrl.a="test"/>`, {
prefixIdentifiers: true
})
expect(prop).toMatchObject({
type: NodeTypes.JS_PROPERTY,
value: createObjectMatcher({
handler: {
callee: V_ON_WITH_KEYS,
arguments: [
{
callee: V_ON_WITH_MODIFIERS,
arguments: [{ content: '_ctx.test' }, '["stop","ctrl"]']
},
'["a"]'
]
},
options: createObjectMatcher({
capture: { content: 'true', isStatic: false }
})
})
key: {
content: `onKeydown.capture`
},
value: {
callee: V_ON_WITH_KEYS,
arguments: [
{
callee: V_ON_WITH_MODIFIERS,
arguments: [{ content: '_ctx.test' }, '["stop","ctrl"]']
},
'["a"]'
]
}
})
})

Expand Down Expand Up @@ -206,9 +200,21 @@ describe('compiler-dom: transform v-on', () => {
type: NodeTypes.COMPOUND_EXPRESSION,
children: [
`(`,
{ children: [`"on" + (`, { content: 'event' }, `)`] },
`).toLowerCase() === "onclick" ? "onContextmenu" : (`,
{ children: [`"on" + (`, { content: 'event' }, `)`] },
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`) === "onClick" ? "onContextmenu" : (`,
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`)`
]
})
Expand All @@ -232,9 +238,21 @@ describe('compiler-dom: transform v-on', () => {
type: NodeTypes.COMPOUND_EXPRESSION,
children: [
`(`,
{ children: [`"on" + (`, { content: 'event' }, `)`] },
`).toLowerCase() === "onclick" ? "onMouseup" : (`,
{ children: [`"on" + (`, { content: 'event' }, `)`] },
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`) === "onClick" ? "onMouseup" : (`,
{
children: [
`"on" + _${helperNameMap[CAPITALIZE]}(`,
{ content: 'event' },
`)`
]
},
`)`
]
})
Expand All @@ -254,24 +272,17 @@ describe('compiler-dom: transform v-on', () => {
expect((root as any).children[0].codegenNode.patchFlag).toBe(
genFlagText(PatchFlags.HYDRATE_EVENTS)
)
expect(prop.value).toMatchObject({
type: NodeTypes.JS_CACHE_EXPRESSION,
index: 1,
expect(prop).toMatchObject({
key: {
content: `onKeyup.capture`
},
value: {
type: NodeTypes.JS_OBJECT_EXPRESSION,
properties: [
{
key: { content: 'handler' },
value: {
type: NodeTypes.JS_CALL_EXPRESSION,
callee: V_ON_WITH_KEYS
}
},
{
key: { content: 'options' },
value: { type: NodeTypes.JS_OBJECT_EXPRESSION }
}
]
type: NodeTypes.JS_CACHE_EXPRESSION,
index: 1,
value: {
type: NodeTypes.JS_CALL_EXPRESSION,
callee: V_ON_WITH_KEYS
}
}
})
})
Expand Down
25 changes: 10 additions & 15 deletions packages/compiler-dom/src/transforms/vOn.ts
View file
Expand Up @@ -3,7 +3,6 @@ import {
DirectiveTransform,
createObjectProperty,
createCallExpression,
createObjectExpression,
createSimpleExpression,
NodeTypes,
createCompoundExpression,
Expand Down Expand Up @@ -80,7 +79,7 @@ const transformClick = (key: ExpressionNode, event: string) => {
? createCompoundExpression([
`(`,
key,
`).toLowerCase() === "onclick" ? "${event}" : (`,
`) === "onClick" ? "${event}" : (`,
key,
`)`
])
Expand Down Expand Up @@ -126,20 +125,16 @@ export const transformOn: DirectiveTransform = (dir, node, context) => {
}

if (eventOptionModifiers.length) {
handlerExp = createObjectExpression([
createObjectProperty('handler', handlerExp),
createObjectProperty(
'options',
createObjectExpression(
eventOptionModifiers.map(modifier =>
createObjectProperty(
modifier,
createSimpleExpression('true', false)
)
)
key = isStaticExp(key)
? createSimpleExpression(
`${key.content}.${eventOptionModifiers.join(`.`)}`,
true
)
)
])
: createCompoundExpression([
`(`,
key,
`) + ".${eventOptionModifiers.join(`.`)}"`
])
}

return {
Expand Down
81 changes: 56 additions & 25 deletions packages/runtime-core/__tests__/componentEmits.spec.ts
View file
Expand Up @@ -160,30 +160,61 @@ describe('component: emit', () => {
expect(`event validation failed for event "foo"`).toHaveBeenWarned()
})

test('isEmitListener', () => {
const def1 = { emits: ['click'] }
expect(isEmitListener(def1, 'onClick')).toBe(true)
expect(isEmitListener(def1, 'onclick')).toBe(false)
expect(isEmitListener(def1, 'onBlick')).toBe(false)

const def2 = { emits: { click: null } }
expect(isEmitListener(def2, 'onClick')).toBe(true)
expect(isEmitListener(def2, 'onclick')).toBe(false)
expect(isEmitListener(def2, 'onBlick')).toBe(false)

const mixin1 = { emits: ['foo'] }
const mixin2 = { emits: ['bar'] }
const extend = { emits: ['baz'] }
const def3 = {
emits: { click: null },
mixins: [mixin1, mixin2],
extends: extend
}
expect(isEmitListener(def3, 'onClick')).toBe(true)
expect(isEmitListener(def3, 'onFoo')).toBe(true)
expect(isEmitListener(def3, 'onBar')).toBe(true)
expect(isEmitListener(def3, 'onBaz')).toBe(true)
expect(isEmitListener(def3, 'onclick')).toBe(false)
expect(isEmitListener(def3, 'onBlick')).toBe(false)
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'] }
expect(isEmitListener(def1, 'onClick')).toBe(true)
expect(isEmitListener(def1, 'onclick')).toBe(false)
expect(isEmitListener(def1, 'onBlick')).toBe(false)
})

test('object option', () => {
const def2 = { emits: { click: null } }
expect(isEmitListener(def2, 'onClick')).toBe(true)
expect(isEmitListener(def2, 'onclick')).toBe(false)
expect(isEmitListener(def2, 'onBlick')).toBe(false)
})

test('with mixins and extends', () => {
const mixin1 = { emits: ['foo'] }
const mixin2 = { emits: ['bar'] }
const extend = { emits: ['baz'] }
const def3 = {
mixins: [mixin1, mixin2],
extends: extend
}
expect(isEmitListener(def3, 'onFoo')).toBe(true)
expect(isEmitListener(def3, 'onBar')).toBe(true)
expect(isEmitListener(def3, 'onBaz')).toBe(true)
expect(isEmitListener(def3, 'onclick')).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)
})
})
})
5 changes: 4 additions & 1 deletion packages/runtime-core/src/component.ts
View file
Expand Up @@ -246,6 +246,8 @@ export interface ComponentInternalInstance {
slots: InternalSlots
refs: Data
emit: EmitFn
// used for keeping track of .once event handlers on components
emitted: Record<string, boolean> | null

/**
* setup related
Expand Down Expand Up @@ -396,7 +398,8 @@ export function createComponentInstance(
rtg: null,
rtc: null,
ec: null,
emit: null as any // to be set immediately
emit: null as any, // to be set immediately
emitted: null
}
if (__DEV__) {
instance.ctx = createRenderContext(instance)
Expand Down

0 comments on commit 380c679

Please sign in to comment.