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.

diff --git a/packages/compiler-core/__tests__/transforms/vOn.spec.ts b/packages/compiler-core/__tests__/transforms/vOn.spec.ts
index 191f4cb50189da79b024399fdc0a35a65f03ce86..873734226ab570649b6aefb39eaf62f3e6aabfe0 100644 (file)
--- a/packages/compiler-core/__tests__/transforms/vOn.spec.ts
+++ b/packages/compiler-core/__tests__/transforms/vOn.spec.ts
@@ -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'
@@ -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,
@@ -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,
@@ -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` },

diff --git a/packages/compiler-dom/__tests__/transforms/vOn.spec.ts b/packages/compiler-dom/__tests__/transforms/vOn.spec.ts
index 57e9300cd1f6b2307e7c82b94aa27691f00d28b8..1c7a60edf6af5da08b2f2b30f289c5e4afcad2ae 100644 (file)
--- a/packages/compiler-dom/__tests__/transforms/vOn.spec.ts
+++ b/packages/compiler-dom/__tests__/transforms/vOn.spec.ts
@@ -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 = {}) {
@@ -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"]'
+        ]
+      }
     })
   })
 
@@ -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' },
+            `)`
+          ]
+        },
         `)`
       ]
     })
@@ -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' },
+            `)`
+          ]
+        },
         `)`
       ]
     })
@@ -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
+        }
       }
     })
   })

diff --git a/packages/compiler-dom/src/transforms/vOn.ts b/packages/compiler-dom/src/transforms/vOn.ts
index 02fac64f7a616bf4d68593d5ef09bf2241655bc4..c08d5c804c27dec4e50c34380caa75c4856c00ab 100644 (file)
--- a/packages/compiler-dom/src/transforms/vOn.ts
+++ b/packages/compiler-dom/src/transforms/vOn.ts
@@ -3,7 +3,6 @@ import {
   DirectiveTransform,
   createObjectProperty,
   createCallExpression,
-  createObjectExpression,
   createSimpleExpression,
   NodeTypes,
   createCompoundExpression,
@@ -80,7 +79,7 @@ const transformClick = (key: ExpressionNode, event: string) => {
       ? createCompoundExpression([
           `(`,
           key,
-          `).toLowerCase() === "onclick" ? "${event}" : (`,
+          `) === "onClick" ? "${event}" : (`,
           key,
           `)`
         ])
@@ -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 {

diff --git a/packages/runtime-core/__tests__/componentEmits.spec.ts b/packages/runtime-core/__tests__/componentEmits.spec.ts
index 30b030eaa036f63d5933f5801e89cc98065efc3c..1e819b656eeebb44a642e25cb4ac5f66164783a1 100644 (file)
--- a/packages/runtime-core/__tests__/componentEmits.spec.ts
+++ b/packages/runtime-core/__tests__/componentEmits.spec.ts
@@ -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)
+    })
   })
 })

diff --git a/packages/runtime-core/src/component.ts b/packages/runtime-core/src/component.ts
index 5869c2b70c9410012f9f9610304eb0551eeb6cb3..21d4fe67bb418e502e95e9c07bed72e6da561ebe 100644 (file)
--- a/packages/runtime-core/src/component.ts
+++ b/packages/runtime-core/src/component.ts
@@ -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
@@ -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)

diff --git a/packages/runtime-core/src/componentEmits.ts b/packages/runtime-core/src/componentEmits.ts
index c44b3e1516fe9c663680e22ccad13a79f2bacf23..f8a419dd5338f61fd7e8d9b53174f4771566fb94 100644 (file)
--- a/packages/runtime-core/src/componentEmits.ts
+++ b/packages/runtime-core/src/componentEmits.ts
@@ -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 props passed via kebab-case
   if (!handler && event.startsWith('update:')) {
-    event = hyphenate(event)
-    handler = props[`on${capitalize(event)}`]
+    handlerName = `on${capitalize(hyphenate(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) {
     callWithAsyncErrorHandling(
@@ -123,13 +132,13 @@ function normalizeEmitsOptions(
 // e.g. With `emits: { click: null }`, props named `onClick` and `onclick` are
 // both considered matched listeners.
 export function isEmitListener(comp: Component, key: string): boolean {
-  if (!isOn(key)) {
+  let emits: ObjectEmitsOptions | undefined
+  if (!isOn(key) || !(emits = normalizeEmitsOptions(comp))) {
     return false
   }
-  const emits = normalizeEmitsOptions(comp)
+  key = key.replace(/\.once$/, '')
   return (
-    !!emits &&
-    (hasOwn(emits, key[2].toLowerCase() + key.slice(3)) ||
-      hasOwn(emits, key.slice(2)))
+    hasOwn(emits, key[2].toLowerCase() + key.slice(3)) ||
+    hasOwn(emits, key.slice(2))
   )
 }

diff --git a/packages/runtime-dom/__tests__/patchEvents.spec.ts b/packages/runtime-dom/__tests__/patchEvents.spec.ts
index 2bb8e2ac080875e7317f3db84decdb56a9c071aa..8962dbf5795520c530e6ef80f26f5911d1ee06ae 100644 (file)
--- a/packages/runtime-dom/__tests__/patchEvents.spec.ts
+++ b/packages/runtime-dom/__tests__/patchEvents.spec.ts
@@ -57,17 +57,11 @@ describe(`runtime-dom: events patching`, () => {
     expect(fn).not.toHaveBeenCalled()
   })
 
-  it('should support event options', async () => {
+  it('should support event option modifiers', async () => {
     const el = document.createElement('div')
     const event = new Event('click')
     const fn = jest.fn()
-    const nextValue = {
-      handler: fn,
-      options: {
-        once: true
-      }
-    }
-    patchProp(el, 'onClick', null, nextValue)
+    patchProp(el, 'onClick.once.capture', null, fn)
     el.dispatchEvent(event)
     await timeout()
     el.dispatchEvent(event)
@@ -75,39 +69,12 @@ describe(`runtime-dom: events patching`, () => {
     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 () => {
     const el = document.createElement('div')
     const event = new Event('click')
     const fn = jest.fn()
-    const nextValue = {
-      handler: fn,
-      options: {
-        once: true
-      }
-    }
-    patchProp(el, 'onClick', null, nextValue)
-    patchProp(el, 'onClick', nextValue, null)
+    patchProp(el, 'onClick.capture', null, fn)
+    patchProp(el, 'onClick.capture', fn, null)
     el.dispatchEvent(event)
     await timeout()
     el.dispatchEvent(event)

diff --git a/packages/runtime-dom/src/modules/events.ts b/packages/runtime-dom/src/modules/events.ts
index 553816c9f030d39bec33f55ff551625b2c587836..755d5841ab1d00c08a85769c365ef8289e2613ff 100644 (file)
--- a/packages/runtime-dom/src/modules/events.ts
+++ b/packages/runtime-dom/src/modules/events.ts
@@ -1,4 +1,4 @@
-import { EMPTY_OBJ, isArray } from '@vue/shared'
+import { isArray } from '@vue/shared'
 import {
   ComponentInternalInstance,
   callWithAsyncErrorHandling
@@ -14,12 +14,6 @@ type EventValue = (Function | Function[]) & {
   invoker?: Invoker | null
 }
 
-type EventValueWithOptions = {
-  handler: EventValue
-  options: AddEventListenerOptions
-  invoker?: Invoker | null
-}
-
 // Async edge case fix requires storing an event listener's attach timestamp.
 let _getNow: () => number = Date.now
 
@@ -67,52 +61,43 @@ export function removeEventListener(
 export function patchEvent(
   el: Element,
   rawName: string,
-  prevValue: EventValueWithOptions | EventValue | null,
-  nextValue: EventValueWithOptions | EventValue | null,
+  prevValue: EventValue | null,
+  nextValue: EventValue | 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 value =
-    nextValue && 'handler' in nextValue ? nextValue.handler : nextValue
-
-  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 && invoker) {
+    // patch
+    ;(prevValue as EventValue).invoker = null
+    invoker.value = nextValue
+    nextValue.invoker = invoker
+  } else {
+    const [name, options] = parseName(rawName)
+    if (nextValue) {
+      addEventListener(el, name, createInvoker(nextValue, instance), options)
+    } else if (invoker) {
+      // remove
+      removeEventListener(el, name, invoker, options)
     }
   }
+}
 
-  if (nextValue && value) {
-    if (invoker) {
-      ;(prevValue as EventValue).invoker = null
-      invoker.value = value
-      nextValue.invoker = invoker
-    } else {
-      addEventListener(
-        el,
-        name,
-        createInvoker(value, instance),
-        nextOptions || void 0
-      )
-    }
-  } else if (invoker) {
-    removeEventListener(el, name, invoker, prevOptions || void 0)
+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]
   }
 }