fix(runtime-dom): fix patching for attributes starting with on

fix #949

BREAKING CHANGE: Only props starting with `on` followed by an uppercase
letter or a non-letter character are considered event listeners.
This commit is contained in:
Evan You
2020-04-10 11:57:07 -04:00
parent 55566e8f52
commit 6eb3399311
8 changed files with 72 additions and 58 deletions

View File

@@ -1,27 +1,37 @@
import { patchAttr, xlinkNS } from '../../src/modules/attrs'
import { patchProp } from '../../src/patchProp'
import { xlinkNS } from '../../src/modules/attrs'
describe('attrs', () => {
test('xlink attributes', () => {
const el = document.createElementNS('http://www.w3.org/2000/svg', 'use')
patchAttr(el, 'xlink:href', 'a', true)
patchProp(el, 'xlink:href', null, 'a', true)
expect(el.getAttributeNS(xlinkNS, 'href')).toBe('a')
patchAttr(el, 'xlink:href', null, true)
patchProp(el, 'xlink:href', 'a', null, true)
expect(el.getAttributeNS(xlinkNS, 'href')).toBe(null)
})
test('boolean attributes', () => {
const el = document.createElement('input')
patchAttr(el, 'readonly', true, false)
patchProp(el, 'readonly', null, true)
expect(el.getAttribute('readonly')).toBe('')
patchAttr(el, 'readonly', false, false)
patchProp(el, 'readonly', true, false)
expect(el.getAttribute('readonly')).toBe(null)
})
test('attributes', () => {
const el = document.createElement('div')
patchAttr(el, 'id', 'a', false)
expect(el.getAttribute('id')).toBe('a')
patchAttr(el, 'id', null, false)
expect(el.getAttribute('id')).toBe(null)
patchProp(el, 'foo', null, 'a')
expect(el.getAttribute('foo')).toBe('a')
patchProp(el, 'foo', 'a', null)
expect(el.getAttribute('foo')).toBe(null)
})
// #949
test('onxxx but non-listener attributes', () => {
const el = document.createElement('div')
patchProp(el, 'onwards', null, 'a')
expect(el.getAttribute('onwards')).toBe('a')
patchProp(el, 'onwards', 'a', null)
expect(el.getAttribute('onwards')).toBe(null)
})
})

View File

@@ -1,4 +1,4 @@
import { patchEvent } from '../../src/modules/events'
import { patchProp } from '../../src/patchProp'
const timeout = () => new Promise(r => setTimeout(r))
@@ -7,7 +7,7 @@ describe(`events`, () => {
const el = document.createElement('div')
const event = new Event('click')
const fn = jest.fn()
patchEvent(el, 'onClick', null, fn, null)
patchProp(el, 'onClick', null, fn)
el.dispatchEvent(event)
await timeout()
el.dispatchEvent(event)
@@ -22,9 +22,9 @@ describe(`events`, () => {
const event = new Event('click')
const prevFn = jest.fn()
const nextFn = jest.fn()
patchEvent(el, 'onClick', null, prevFn, null)
patchProp(el, 'onClick', null, prevFn)
el.dispatchEvent(event)
patchEvent(el, 'onClick', prevFn, nextFn, null)
patchProp(el, 'onClick', prevFn, nextFn)
await timeout()
el.dispatchEvent(event)
await timeout()
@@ -39,7 +39,7 @@ describe(`events`, () => {
const event = new Event('click')
const fn1 = jest.fn()
const fn2 = jest.fn()
patchEvent(el, 'onClick', null, [fn1, fn2], null)
patchProp(el, 'onClick', null, [fn1, fn2])
el.dispatchEvent(event)
await timeout()
expect(fn1).toHaveBeenCalledTimes(1)
@@ -50,8 +50,8 @@ describe(`events`, () => {
const el = document.createElement('div')
const event = new Event('click')
const fn = jest.fn()
patchEvent(el, 'onClick', null, fn, null)
patchEvent(el, 'onClick', fn, null, null)
patchProp(el, 'onClick', null, fn)
patchProp(el, 'onClick', fn, null)
el.dispatchEvent(event)
await timeout()
expect(fn).not.toHaveBeenCalled()
@@ -67,7 +67,7 @@ describe(`events`, () => {
once: true
}
}
patchEvent(el, 'onClick', null, nextValue, null)
patchProp(el, 'onClick', null, nextValue)
el.dispatchEvent(event)
await timeout()
el.dispatchEvent(event)
@@ -86,8 +86,8 @@ describe(`events`, () => {
once: true
}
}
patchEvent(el, 'onClick', null, prevFn, null)
patchEvent(el, 'onClick', prevFn, nextValue, null)
patchProp(el, 'onClick', null, prevFn)
patchProp(el, 'onClick', prevFn, nextValue)
el.dispatchEvent(event)
await timeout()
el.dispatchEvent(event)
@@ -106,8 +106,8 @@ describe(`events`, () => {
once: true
}
}
patchEvent(el, 'onClick', null, nextValue, null)
patchEvent(el, 'onClick', nextValue, null, null)
patchProp(el, 'onClick', null, nextValue)
patchProp(el, 'onClick', nextValue, null)
el.dispatchEvent(event)
await timeout()
el.dispatchEvent(event)
@@ -115,21 +115,23 @@ describe(`events`, () => {
expect(fn).not.toHaveBeenCalled()
})
it('should assign native onclick attribute', async () => {
it('should support native onclick', async () => {
const el = document.createElement('div')
const event = new Event('click')
const fn = ((window as any)._nativeClickSpy = jest.fn())
patchEvent(el, 'onclick', null, '_nativeClickSpy()' as any)
// string should be set as attribute
const fn = ((window as any).__globalSpy = jest.fn())
patchProp(el, 'onclick', null, '__globalSpy(1)')
el.dispatchEvent(event)
await timeout()
expect(fn).toHaveBeenCalledTimes(1)
delete (window as any).__globalSpy
expect(fn).toHaveBeenCalledWith(1)
const fn2 = jest.fn()
patchEvent(el, 'onclick', null, fn2)
patchProp(el, 'onclick', '__globalSpy(1)', fn2)
el.dispatchEvent(event)
await timeout()
expect(fn).toHaveBeenCalledTimes(1)
expect(fn2).toHaveBeenCalledTimes(1)
expect(fn2).toHaveBeenCalledWith(event)
})
})

View File

@@ -1,39 +1,39 @@
import { patchStyle } from '../../src/modules/style'
import { patchProp } from '../../src/patchProp'
describe(`module style`, () => {
it('string', () => {
const el = document.createElement('div')
patchStyle(el, {}, 'color:red')
patchProp(el, 'style', {}, 'color:red')
expect(el.style.cssText.replace(/\s/g, '')).toBe('color:red;')
})
it('plain object', () => {
const el = document.createElement('div')
patchStyle(el, {}, { color: 'red' })
patchProp(el, 'style', {}, { color: 'red' })
expect(el.style.cssText.replace(/\s/g, '')).toBe('color:red;')
})
it('camelCase', () => {
const el = document.createElement('div')
patchStyle(el, {}, { marginRight: '10px' })
patchProp(el, 'style', {}, { marginRight: '10px' })
expect(el.style.cssText.replace(/\s/g, '')).toBe('margin-right:10px;')
})
it('remove if falsy value', () => {
const el = document.createElement('div')
patchStyle(el, { color: 'red' }, { color: undefined })
patchProp(el, 'style', { color: 'red' }, { color: undefined })
expect(el.style.cssText.replace(/\s/g, '')).toBe('')
})
it('!important', () => {
const el = document.createElement('div')
patchStyle(el, {}, { color: 'red !important' })
patchProp(el, 'style', {}, { color: 'red !important' })
expect(el.style.cssText.replace(/\s/g, '')).toBe('color:red!important;')
})
it('camelCase with !important', () => {
const el = document.createElement('div')
patchStyle(el, {}, { marginRight: '10px !important' })
patchProp(el, 'style', {}, { marginRight: '10px !important' })
expect(el.style.cssText.replace(/\s/g, '')).toBe(
'margin-right:10px!important;'
)
@@ -41,7 +41,7 @@ describe(`module style`, () => {
it('object with multiple entries', () => {
const el = document.createElement('div')
patchStyle(el, {}, { color: 'red', marginRight: '10px' })
patchProp(el, 'style', {}, { color: 'red', marginRight: '10px' })
expect(el.style.getPropertyValue('color')).toBe('red')
expect(el.style.getPropertyValue('margin-right')).toBe('10px')
})
@@ -65,13 +65,13 @@ describe(`module style`, () => {
it('CSS custom properties', () => {
const el = mockElementWithStyle()
patchStyle(el as any, {}, { '--theme': 'red' } as any)
patchProp(el as any, 'style', {}, { '--theme': 'red' } as any)
expect(el.style.getPropertyValue('--theme')).toBe('red')
})
it('auto vendor prefixing', () => {
const el = mockElementWithStyle()
patchStyle(el as any, {}, { transition: 'all 1s' })
patchProp(el as any, 'style', {}, { transition: 'all 1s' })
expect(el.style.WebkitTransition).toBe('all 1s')
})
})

View File

@@ -1,4 +1,4 @@
import { EMPTY_OBJ, isString } from '@vue/shared'
import { EMPTY_OBJ } from '@vue/shared'
import {
ComponentInternalInstance,
callWithAsyncErrorHandling
@@ -71,16 +71,6 @@ export function patchEvent(
nextValue: EventValueWithOptions | EventValue | null,
instance: ComponentInternalInstance | null = null
) {
// support native onxxx handlers
if (rawName in el) {
if (isString(nextValue)) {
el.setAttribute(rawName, nextValue)
} else {
;(el as any)[rawName] = nextValue
}
return
}
const name = rawName.slice(2).toLowerCase()
const prevOptions = prevValue && 'options' in prevValue && prevValue.options
const nextOptions = nextValue && 'options' in nextValue && nextValue.options

View File

@@ -3,9 +3,11 @@ import { patchStyle } from './modules/style'
import { patchAttr } from './modules/attrs'
import { patchDOMProp } from './modules/props'
import { patchEvent } from './modules/events'
import { isOn } from '@vue/shared'
import { isOn, isString } from '@vue/shared'
import { RendererOptions } from '@vue/runtime-core'
const nativeOnRE = /^on[a-z]/
export const patchProp: RendererOptions<Node, Element>['patchProp'] = (
el,
key,
@@ -31,7 +33,12 @@ export const patchProp: RendererOptions<Node, Element>['patchProp'] = (
if (key.indexOf('onUpdate:') < 0) {
patchEvent(el, key, prevValue, nextValue, parentComponent)
}
} else if (!isSVG && key in el) {
} else if (
!isSVG &&
key in el &&
// onclick="foo" needs to be set as an attribute to work
!(nativeOnRE.test(key) && isString(nextValue))
) {
patchDOMProp(
el,
key,