Skip to content

Commit 21bcdec

Browse files
yyx990803yyx990803
committed
refactor(runtime-core): adjust attr fallthrough behavior
BREAKING CHANGE: attribute fallthrough behavior has been adjusted according to vuejs/rfcs#154
1 parent 2103a48 commit 21bcdec

File tree

5 files changed

+144
-27
lines changed

5 files changed

+144
-27
lines changed

‎packages/runtime-core/__tests__/apiSetupContext.spec.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ describe('api: setup context', () => {
5454
}
5555

5656
const Child = defineComponent({
57-
setup(props: { count: number }) {
57+
props: { count: Number },
58+
setup(props) {
5859
watchEffect(() => {
5960
dummy = props.count
6061
})

‎packages/runtime-core/__tests__/components/Suspense.spec.ts

+10-5
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,8 @@ describe('Suspense', () => {
222222

223223
test('content update before suspense resolve', async () => {
224224
const Async = defineAsyncComponent({
225-
setup(props: { msg: string }) {
225+
props: { msg: String },
226+
setup(props: any) {
226227
return () => h('div', props.msg)
227228
}
228229
})
@@ -569,7 +570,8 @@ describe('Suspense', () => {
569570
const calls: number[] = []
570571

571572
const AsyncChildWithSuspense = defineAsyncComponent({
572-
setup(props: { msg: string }) {
573+
props: { msg: String },
574+
setup(props: any) {
573575
onMounted(() => {
574576
calls.push(0)
575577
})
@@ -583,7 +585,8 @@ describe('Suspense', () => {
583585

584586
const AsyncInsideNestedSuspense = defineAsyncComponent(
585587
{
586-
setup(props: { msg: string }) {
588+
props: { msg: String },
589+
setup(props: any) {
587590
onMounted(() => {
588591
calls.push(2)
589592
})
@@ -594,7 +597,8 @@ describe('Suspense', () => {
594597
)
595598

596599
const AsyncChildParent = defineAsyncComponent({
597-
setup(props: { msg: string }) {
600+
props: { msg: String },
601+
setup(props: any) {
598602
onMounted(() => {
599603
calls.push(1)
600604
})
@@ -604,7 +608,8 @@ describe('Suspense', () => {
604608

605609
const NestedAsyncChild = defineAsyncComponent(
606610
{
607-
setup(props: { msg: string }) {
611+
props: { msg: String },
612+
setup(props: any) {
608613
onMounted(() => {
609614
calls.push(3)
610615
})

‎packages/runtime-core/__tests__/hydration.spec.ts

+6-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {
88
createStaticVNode,
99
Suspense,
1010
onMounted,
11-
defineAsyncComponent
11+
defineAsyncComponent,
12+
defineComponent
1213
} from '@vue/runtime-dom'
1314
import { renderToString } from '@vue/server-renderer'
1415
import { mockWarn } from '@vue/shared'
@@ -448,8 +449,9 @@ describe('SSR hydration', () => {
448449
const mountedCalls: number[] = []
449450
const asyncDeps: Promise<any>[] = []
450451

451-
const AsyncChild = {
452-
async setup(props: { n: number }) {
452+
const AsyncChild = defineComponent({
453+
props: ['n'],
454+
async setup(props) {
453455
const count = ref(props.n)
454456
onMounted(() => {
455457
mountedCalls.push(props.n)
@@ -468,7 +470,7 @@ describe('SSR hydration', () => {
468470
count.value
469471
)
470472
}
471-
}
473+
})
472474

473475
const done = jest.fn()
474476
const App = {

‎packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts

+120-6
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { mockWarn } from '@vue/shared'
1515
describe('attribute fallthrough', () => {
1616
mockWarn()
1717

18-
it('should allow whitelisted attrs to fallthrough', async () => {
18+
it('should allow attrs to fallthrough', async () => {
1919
const click = jest.fn()
2020
const childUpdated = jest.fn()
2121

@@ -30,12 +30,12 @@ describe('attribute fallthrough', () => {
3030

3131
return () =>
3232
h(Child, {
33-
foo: 1,
33+
foo: count.value + 1,
3434
id: 'test',
3535
class: 'c' + count.value,
3636
style: { color: count.value ? 'red' : 'green' },
3737
onClick: inc,
38-
'data-id': 1
38+
'data-id': count.value + 1
3939
})
4040
}
4141
}
@@ -47,7 +47,6 @@ describe('attribute fallthrough', () => {
4747
h(
4848
'div',
4949
{
50-
id: props.id, // id is not whitelisted
5150
class: 'c2',
5251
style: { fontWeight: 'bold' }
5352
},
@@ -62,15 +61,130 @@ describe('attribute fallthrough', () => {
6261

6362
const node = root.children[0] as HTMLElement
6463

65-
expect(node.getAttribute('id')).toBe('test') // id is not whitelisted, but explicitly bound
66-
expect(node.getAttribute('foo')).toBe(null) // foo is not whitelisted
64+
expect(node.getAttribute('id')).toBe('test')
65+
expect(node.getAttribute('foo')).toBe('1')
6766
expect(node.getAttribute('class')).toBe('c2 c0')
6867
expect(node.style.color).toBe('green')
6968
expect(node.style.fontWeight).toBe('bold')
7069
expect(node.dataset.id).toBe('1')
7170
node.dispatchEvent(new CustomEvent('click'))
7271
expect(click).toHaveBeenCalled()
7372

73+
await nextTick()
74+
expect(childUpdated).toHaveBeenCalled()
75+
expect(node.getAttribute('id')).toBe('test')
76+
expect(node.getAttribute('foo')).toBe('2')
77+
expect(node.getAttribute('class')).toBe('c2 c1')
78+
expect(node.style.color).toBe('red')
79+
expect(node.style.fontWeight).toBe('bold')
80+
expect(node.dataset.id).toBe('2')
81+
})
82+
83+
it('should only allow whitelisted fallthrough on functional component with optional props', async () => {
84+
const click = jest.fn()
85+
const childUpdated = jest.fn()
86+
87+
const count = ref(0)
88+
89+
function inc() {
90+
count.value++
91+
click()
92+
}
93+
94+
const Hello = () =>
95+
h(Child, {
96+
foo: count.value + 1,
97+
id: 'test',
98+
class: 'c' + count.value,
99+
style: { color: count.value ? 'red' : 'green' },
100+
onClick: inc
101+
})
102+
103+
const Child = (props: any) => {
104+
childUpdated()
105+
return h(
106+
'div',
107+
{
108+
class: 'c2',
109+
style: { fontWeight: 'bold' }
110+
},
111+
props.foo
112+
)
113+
}
114+
115+
const root = document.createElement('div')
116+
document.body.appendChild(root)
117+
render(h(Hello), root)
118+
119+
const node = root.children[0] as HTMLElement
120+
121+
// not whitelisted
122+
expect(node.getAttribute('id')).toBe(null)
123+
expect(node.getAttribute('foo')).toBe(null)
124+
125+
// whitelisted: style, class, event listeners
126+
expect(node.getAttribute('class')).toBe('c2 c0')
127+
expect(node.style.color).toBe('green')
128+
expect(node.style.fontWeight).toBe('bold')
129+
node.dispatchEvent(new CustomEvent('click'))
130+
expect(click).toHaveBeenCalled()
131+
132+
await nextTick()
133+
expect(childUpdated).toHaveBeenCalled()
134+
expect(node.getAttribute('id')).toBe(null)
135+
expect(node.getAttribute('foo')).toBe(null)
136+
expect(node.getAttribute('class')).toBe('c2 c1')
137+
expect(node.style.color).toBe('red')
138+
expect(node.style.fontWeight).toBe('bold')
139+
})
140+
141+
it('should allow all attrs on functional component with declared props', async () => {
142+
const click = jest.fn()
143+
const childUpdated = jest.fn()
144+
145+
const count = ref(0)
146+
147+
function inc() {
148+
count.value++
149+
click()
150+
}
151+
152+
const Hello = () =>
153+
h(Child, {
154+
foo: count.value + 1,
155+
id: 'test',
156+
class: 'c' + count.value,
157+
style: { color: count.value ? 'red' : 'green' },
158+
onClick: inc
159+
})
160+
161+
const Child = (props: { foo: number }) => {
162+
childUpdated()
163+
return h(
164+
'div',
165+
{
166+
class: 'c2',
167+
style: { fontWeight: 'bold' }
168+
},
169+
props.foo
170+
)
171+
}
172+
Child.props = ['foo']
173+
174+
const root = document.createElement('div')
175+
document.body.appendChild(root)
176+
render(h(Hello), root)
177+
178+
const node = root.children[0] as HTMLElement
179+
180+
expect(node.getAttribute('id')).toBe('test')
181+
expect(node.getAttribute('foo')).toBe(null) // declared as prop
182+
expect(node.getAttribute('class')).toBe('c2 c0')
183+
expect(node.style.color).toBe('green')
184+
expect(node.style.fontWeight).toBe('bold')
185+
node.dispatchEvent(new CustomEvent('click'))
186+
expect(click).toHaveBeenCalled()
187+
74188
await nextTick()
75189
expect(childUpdated).toHaveBeenCalled()
76190
expect(node.getAttribute('id')).toBe('test')

‎packages/runtime-core/src/componentRenderUtils.ts

+6-11
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,15 @@ export function renderComponentRoot(
5555
accessedAttrs = false
5656
}
5757
try {
58+
let fallthroughAttrs
5859
if (vnode.shapeFlag & ShapeFlags.STATEFUL_COMPONENT) {
5960
// withProxy is a proxy with a different `has` trap only for
6061
// runtime-compiled render functions using `with` block.
6162
const proxyToUse = withProxy || proxy
6263
result = normalizeVNode(
6364
instance.render!.call(proxyToUse, proxyToUse!, renderCache)
6465
)
66+
fallthroughAttrs = attrs
6567
} else {
6668
// functional
6769
const render = Component as FunctionalComponent
@@ -74,14 +76,14 @@ export function renderComponentRoot(
7476
})
7577
: render(props, null as any /* we know it doesn't need it */)
7678
)
79+
fallthroughAttrs = Component.props ? attrs : getFallthroughAttrs(attrs)
7780
}
7881

7982
// attr merging
80-
let fallthroughAttrs
8183
if (
8284
Component.inheritAttrs !== false &&
83-
attrs !== EMPTY_OBJ &&
84-
(fallthroughAttrs = getFallthroughAttrs(attrs))
85+
fallthroughAttrs &&
86+
fallthroughAttrs !== EMPTY_OBJ
8587
) {
8688
if (
8789
result.shapeFlag & ShapeFlags.ELEMENT ||
@@ -140,14 +142,7 @@ export function renderComponentRoot(
140142
const getFallthroughAttrs = (attrs: Data): Data | undefined => {
141143
let res: Data | undefined
142144
for (const key in attrs) {
143-
if (
144-
key === 'class' ||
145-
key === 'style' ||
146-
key === 'role' ||
147-
isOn(key) ||
148-
key.indexOf('aria-') === 0 ||
149-
key.indexOf('data-') === 0
150-
) {
145+
if (key === 'class' || key === 'style' || isOn(key)) {
151146
;(res || (res = {}))[key] = attrs[key]
152147
}
153148
}

0 commit comments

Comments
 (0)