Skip to content

Commit

Permalink
refactor(runtime-core): adjust attr fallthrough behavior
Browse files Browse the repository at this point in the history
BREAKING CHANGE: attribute fallthrough behavior has been adjusted
according to vuejs/rfcs#154
  • Loading branch information
yyx990803 committed Apr 3, 2020
1 parent 2103a48 commit 21bcdec
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 27 deletions.
3 changes: 2 additions & 1 deletion packages/runtime-core/__tests__/apiSetupContext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ describe('api: setup context', () => {
}

const Child = defineComponent({
setup(props: { count: number }) {
props: { count: Number },
setup(props) {
watchEffect(() => {
dummy = props.count
})
Expand Down
15 changes: 10 additions & 5 deletions packages/runtime-core/__tests__/components/Suspense.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down Expand Up @@ -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)
})
Expand All @@ -583,7 +585,8 @@ describe('Suspense', () => {

const AsyncInsideNestedSuspense = defineAsyncComponent(
{
setup(props: { msg: string }) {
props: { msg: String },
setup(props: any) {
onMounted(() => {
calls.push(2)
})
Expand All @@ -594,7 +597,8 @@ describe('Suspense', () => {
)

const AsyncChildParent = defineAsyncComponent({
setup(props: { msg: string }) {
props: { msg: String },
setup(props: any) {
onMounted(() => {
calls.push(1)
})
Expand All @@ -604,7 +608,8 @@ describe('Suspense', () => {

const NestedAsyncChild = defineAsyncComponent(
{
setup(props: { msg: string }) {
props: { msg: String },
setup(props: any) {
onMounted(() => {
calls.push(3)
})
Expand Down
10 changes: 6 additions & 4 deletions packages/runtime-core/__tests__/hydration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand All @@ -468,7 +470,7 @@ describe('SSR hydration', () => {
count.value
)
}
}
})

const done = jest.fn()
const App = {
Expand Down
126 changes: 120 additions & 6 deletions packages/runtime-core/__tests__/rendererAttrsFallthrough.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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
})
}
}
Expand All @@ -47,7 +47,6 @@ describe('attribute fallthrough', () => {
h(
'div',
{
id: props.id, // id is not whitelisted
class: 'c2',
style: { fontWeight: 'bold' }
},
Expand All @@ -62,15 +61,130 @@ 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')
expect(node.dataset.id).toBe('1')
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')
Expand Down
17 changes: 6 additions & 11 deletions packages/runtime-core/src/componentRenderUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@ 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.
const proxyToUse = withProxy || proxy
result = normalizeVNode(
instance.render!.call(proxyToUse, proxyToUse!, renderCache)
)
fallthroughAttrs = attrs
} else {
// functional
const render = Component as FunctionalComponent
Expand All @@ -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 ||
Expand Down Expand Up @@ -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]
}
}
Expand Down

0 comments on commit 21bcdec

Please sign in to comment.