]> git.ipfire.org Git - thirdparty/vuejs/core.git/commitdiff
refactor(runtime-core): adjust attr fallthrough behavior
authorEvan You <yyx990803@gmail.com>
Fri, 3 Apr 2020 01:51:01 +0000 (21:51 -0400)
committerEvan You <yyx990803@gmail.com>
Fri, 3 Apr 2020 13:20:37 +0000 (09:20 -0400)
BREAKING CHANGE: attribute fallthrough behavior has been adjusted
according to https://github.com/vuejs/rfcs/pull/154

packages/runtime-core/__tests__/apiSetupContext.spec.ts
packages/runtime-core/__tests__/components/Suspense.spec.ts
packages/runtime-core/__tests__/hydration.spec.ts
packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts
packages/runtime-core/src/componentRenderUtils.ts

index 8f7ab078bbeeacef3543fdf2c1cc68bb119fbedd..08f0235b94b6b3b57e6ef4aec075a4949fac0dba 100644 (file)
@@ -54,7 +54,8 @@ describe('api: setup context', () => {
     }
 
     const Child = defineComponent({
-      setup(props: { count: number }) {
+      props: { count: Number },
+      setup(props) {
         watchEffect(() => {
           dummy = props.count
         })
index 4738c80ef7840f2ba9bf4a1bb59b7dda012eb6a5..5b71689a4356722bb334824683700a3b1974c15c 100644 (file)
@@ -222,7 +222,8 @@ describe('Suspense', () => {
 
   test('content update before suspense resolve', async () => {
     const Async = defineAsyncComponent({
-      setup(props: { msg: string }) {
+      props: { msg: String },
+      setup(props: any) {
         return () => h('div', props.msg)
       }
     })
@@ -569,7 +570,8 @@ describe('Suspense', () => {
     const calls: number[] = []
 
     const AsyncChildWithSuspense = defineAsyncComponent({
-      setup(props: { msg: string }) {
+      props: { msg: String },
+      setup(props: any) {
         onMounted(() => {
           calls.push(0)
         })
@@ -583,7 +585,8 @@ describe('Suspense', () => {
 
     const AsyncInsideNestedSuspense = defineAsyncComponent(
       {
-        setup(props: { msg: string }) {
+        props: { msg: String },
+        setup(props: any) {
           onMounted(() => {
             calls.push(2)
           })
@@ -594,7 +597,8 @@ describe('Suspense', () => {
     )
 
     const AsyncChildParent = defineAsyncComponent({
-      setup(props: { msg: string }) {
+      props: { msg: String },
+      setup(props: any) {
         onMounted(() => {
           calls.push(1)
         })
@@ -604,7 +608,8 @@ describe('Suspense', () => {
 
     const NestedAsyncChild = defineAsyncComponent(
       {
-        setup(props: { msg: string }) {
+        props: { msg: String },
+        setup(props: any) {
           onMounted(() => {
             calls.push(3)
           })
index 837be3ee541084d4af79386a922444716bd80bd7..ea9a8a27774b105196625f97d08c71272f1fe6b3 100644 (file)
@@ -8,7 +8,8 @@ import {
   createStaticVNode,
   Suspense,
   onMounted,
-  defineAsyncComponent
+  defineAsyncComponent,
+  defineComponent
 } from '@vue/runtime-dom'
 import { renderToString } from '@vue/server-renderer'
 import { mockWarn } from '@vue/shared'
@@ -448,8 +449,9 @@ describe('SSR hydration', () => {
     const mountedCalls: number[] = []
     const asyncDeps: Promise<any>[] = []
 
-    const AsyncChild = {
-      async setup(props: { n: number }) {
+    const AsyncChild = defineComponent({
+      props: ['n'],
+      async setup(props) {
         const count = ref(props.n)
         onMounted(() => {
           mountedCalls.push(props.n)
@@ -468,7 +470,7 @@ describe('SSR hydration', () => {
             count.value
           )
       }
-    }
+    })
 
     const done = jest.fn()
     const App = {
index 0f5f54dcdf25c2b31da590f90237dbeb495bf865..d7eb0ac967df8429577ea3f1eb4bda6ff283586b 100644 (file)
@@ -15,7 +15,7 @@ import { mockWarn } from '@vue/shared'
 describe('attribute fallthrough', () => {
   mockWarn()
 
-  it('should allow whitelisted attrs to fallthrough', async () => {
+  it('should allow attrs to fallthrough', async () => {
     const click = jest.fn()
     const childUpdated = jest.fn()
 
@@ -30,12 +30,12 @@ describe('attribute fallthrough', () => {
 
         return () =>
           h(Child, {
-            foo: 1,
+            foo: count.value + 1,
             id: 'test',
             class: 'c' + count.value,
             style: { color: count.value ? 'red' : 'green' },
             onClick: inc,
-            'data-id': 1
+            'data-id': count.value + 1
           })
       }
     }
@@ -47,7 +47,6 @@ describe('attribute fallthrough', () => {
           h(
             'div',
             {
-              id: props.id, // id is not whitelisted
               class: 'c2',
               style: { fontWeight: 'bold' }
             },
@@ -62,8 +61,8 @@ describe('attribute fallthrough', () => {
 
     const node = root.children[0] as HTMLElement
 
-    expect(node.getAttribute('id')).toBe('test') // id is not whitelisted, but explicitly bound
-    expect(node.getAttribute('foo')).toBe(null) // foo is not whitelisted
+    expect(node.getAttribute('id')).toBe('test')
+    expect(node.getAttribute('foo')).toBe('1')
     expect(node.getAttribute('class')).toBe('c2 c0')
     expect(node.style.color).toBe('green')
     expect(node.style.fontWeight).toBe('bold')
@@ -71,6 +70,121 @@ describe('attribute fallthrough', () => {
     node.dispatchEvent(new CustomEvent('click'))
     expect(click).toHaveBeenCalled()
 
+    await nextTick()
+    expect(childUpdated).toHaveBeenCalled()
+    expect(node.getAttribute('id')).toBe('test')
+    expect(node.getAttribute('foo')).toBe('2')
+    expect(node.getAttribute('class')).toBe('c2 c1')
+    expect(node.style.color).toBe('red')
+    expect(node.style.fontWeight).toBe('bold')
+    expect(node.dataset.id).toBe('2')
+  })
+
+  it('should only allow whitelisted fallthrough on functional component with optional props', async () => {
+    const click = jest.fn()
+    const childUpdated = jest.fn()
+
+    const count = ref(0)
+
+    function inc() {
+      count.value++
+      click()
+    }
+
+    const Hello = () =>
+      h(Child, {
+        foo: count.value + 1,
+        id: 'test',
+        class: 'c' + count.value,
+        style: { color: count.value ? 'red' : 'green' },
+        onClick: inc
+      })
+
+    const Child = (props: any) => {
+      childUpdated()
+      return h(
+        'div',
+        {
+          class: 'c2',
+          style: { fontWeight: 'bold' }
+        },
+        props.foo
+      )
+    }
+
+    const root = document.createElement('div')
+    document.body.appendChild(root)
+    render(h(Hello), root)
+
+    const node = root.children[0] as HTMLElement
+
+    // not whitelisted
+    expect(node.getAttribute('id')).toBe(null)
+    expect(node.getAttribute('foo')).toBe(null)
+
+    // whitelisted: style, class, event listeners
+    expect(node.getAttribute('class')).toBe('c2 c0')
+    expect(node.style.color).toBe('green')
+    expect(node.style.fontWeight).toBe('bold')
+    node.dispatchEvent(new CustomEvent('click'))
+    expect(click).toHaveBeenCalled()
+
+    await nextTick()
+    expect(childUpdated).toHaveBeenCalled()
+    expect(node.getAttribute('id')).toBe(null)
+    expect(node.getAttribute('foo')).toBe(null)
+    expect(node.getAttribute('class')).toBe('c2 c1')
+    expect(node.style.color).toBe('red')
+    expect(node.style.fontWeight).toBe('bold')
+  })
+
+  it('should allow all attrs on functional component with declared props', async () => {
+    const click = jest.fn()
+    const childUpdated = jest.fn()
+
+    const count = ref(0)
+
+    function inc() {
+      count.value++
+      click()
+    }
+
+    const Hello = () =>
+      h(Child, {
+        foo: count.value + 1,
+        id: 'test',
+        class: 'c' + count.value,
+        style: { color: count.value ? 'red' : 'green' },
+        onClick: inc
+      })
+
+    const Child = (props: { foo: number }) => {
+      childUpdated()
+      return h(
+        'div',
+        {
+          class: 'c2',
+          style: { fontWeight: 'bold' }
+        },
+        props.foo
+      )
+    }
+    Child.props = ['foo']
+
+    const root = document.createElement('div')
+    document.body.appendChild(root)
+    render(h(Hello), root)
+
+    const node = root.children[0] as HTMLElement
+
+    expect(node.getAttribute('id')).toBe('test')
+    expect(node.getAttribute('foo')).toBe(null) // declared as prop
+    expect(node.getAttribute('class')).toBe('c2 c0')
+    expect(node.style.color).toBe('green')
+    expect(node.style.fontWeight).toBe('bold')
+    node.dispatchEvent(new CustomEvent('click'))
+    expect(click).toHaveBeenCalled()
+
     await nextTick()
     expect(childUpdated).toHaveBeenCalled()
     expect(node.getAttribute('id')).toBe('test')
index d20ba2b7468af975172e0dafccaf07b0b6f47fcb..6ba6728004e6c3fc2c89076702d2bfd5153f76c8 100644 (file)
@@ -55,6 +55,7 @@ export function renderComponentRoot(
     accessedAttrs = false
   }
   try {
+    let fallthroughAttrs
     if (vnode.shapeFlag & ShapeFlags.STATEFUL_COMPONENT) {
       // withProxy is a proxy with a different `has` trap only for
       // runtime-compiled render functions using `with` block.
@@ -62,6 +63,7 @@ export function renderComponentRoot(
       result = normalizeVNode(
         instance.render!.call(proxyToUse, proxyToUse!, renderCache)
       )
+      fallthroughAttrs = attrs
     } else {
       // functional
       const render = Component as FunctionalComponent
@@ -74,14 +76,14 @@ export function renderComponentRoot(
             })
           : render(props, null as any /* we know it doesn't need it */)
       )
+      fallthroughAttrs = Component.props ? attrs : getFallthroughAttrs(attrs)
     }
 
     // attr merging
-    let fallthroughAttrs
     if (
       Component.inheritAttrs !== false &&
-      attrs !== EMPTY_OBJ &&
-      (fallthroughAttrs = getFallthroughAttrs(attrs))
+      fallthroughAttrs &&
+      fallthroughAttrs !== EMPTY_OBJ
     ) {
       if (
         result.shapeFlag & ShapeFlags.ELEMENT ||
@@ -140,14 +142,7 @@ export function renderComponentRoot(
 const getFallthroughAttrs = (attrs: Data): Data | undefined => {
   let res: Data | undefined
   for (const key in attrs) {
-    if (
-      key === 'class' ||
-      key === 'style' ||
-      key === 'role' ||
-      isOn(key) ||
-      key.indexOf('aria-') === 0 ||
-      key.indexOf('data-') === 0
-    ) {
+    if (key === 'class' || key === 'style' || isOn(key)) {
       ;(res || (res = {}))[key] = attrs[key]
     }
   }