refactor: remove inheritAttrs / nativeOn, enforce explicit fallthrough

This commit is contained in:
Evan You 2018-10-09 21:10:30 -04:00
parent 699dfd95be
commit 83605ef26c
12 changed files with 97 additions and 127 deletions

View File

@ -1,9 +1,9 @@
// using DOM renderer because this case is mostly DOM-specific // using DOM renderer because this case is mostly DOM-specific
import { h, render, Component, nextTick } from '@vue/renderer-dom' import { h, render, Component, nextTick, cloneVNode } from '@vue/renderer-dom'
describe('attribute fallthrough', () => { describe('attribute fallthrough', () => {
it('should not fallthrough on components with no declared props', async () => { it('everything should be in props when component has no declared props', async () => {
const nativeClick = jest.fn() const click = jest.fn()
const childUpdated = jest.fn() const childUpdated = jest.fn()
class Hello extends Component<{}, { count: number }> { class Hello extends Component<{}, { count: number }> {
@ -14,7 +14,7 @@ describe('attribute fallthrough', () => {
} }
inc() { inc() {
this.count++ this.count++
nativeClick() click()
} }
render() { render() {
return h(Child, { return h(Child, {
@ -22,23 +22,26 @@ describe('attribute fallthrough', () => {
id: 'test', id: 'test',
class: 'c' + this.count, class: 'c' + this.count,
style: { color: this.count ? 'red' : 'green' }, style: { color: this.count ? 'red' : 'green' },
nativeOnClick: this.inc onClick: this.inc
}) })
} }
} }
class Child extends Component<{ foo: number }> { class Child extends Component {
updated() { updated() {
childUpdated() childUpdated()
} }
render() { render(props: any) {
return h( return cloneVNode(
'div', h(
{ 'div',
class: 'c2', {
style: { fontWeight: 'bold' } class: 'c2',
}, style: { fontWeight: 'bold' }
this.$props.foo },
props.foo
),
props
) )
} }
} }
@ -49,28 +52,25 @@ describe('attribute fallthrough', () => {
const node = root.children[0] as HTMLElement const node = root.children[0] as HTMLElement
// attrs do not fallthrough because no props are declared expect(node.getAttribute('id')).toBe('test')
expect(node.hasAttribute('id')).toBe(false) expect(node.getAttribute('foo')).toBe('1')
expect(node.hasAttribute('foo')).toBe(false)
// class, style and nativeOn* always fallthrough
expect(node.getAttribute('class')).toBe('c2 c0') expect(node.getAttribute('class')).toBe('c2 c0')
expect(node.style.color).toBe('green') expect(node.style.color).toBe('green')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
node.dispatchEvent(new CustomEvent('click')) node.dispatchEvent(new CustomEvent('click'))
expect(nativeClick).toHaveBeenCalled() expect(click).toHaveBeenCalled()
await nextTick() await nextTick()
expect(childUpdated).toHaveBeenCalled() expect(childUpdated).toHaveBeenCalled()
expect(node.hasAttribute('id')).toBe(false) expect(node.getAttribute('id')).toBe('test')
expect(node.hasAttribute('foo')).toBe(false) expect(node.getAttribute('foo')).toBe('1')
expect(node.getAttribute('class')).toBe('c2 c1') expect(node.getAttribute('class')).toBe('c2 c1')
expect(node.style.color).toBe('red') expect(node.style.color).toBe('red')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
}) })
it('should fallthrough on components with declared props', async () => { it('should separate in attrs when component has declared props', async () => {
const nativeClick = jest.fn() const click = jest.fn()
const childUpdated = jest.fn() const childUpdated = jest.fn()
class Hello extends Component<{}, { count: number }> { class Hello extends Component<{}, { count: number }> {
@ -81,7 +81,7 @@ describe('attribute fallthrough', () => {
} }
inc() { inc() {
this.count++ this.count++
nativeClick() click()
} }
render() { render() {
return h(Child, { return h(Child, {
@ -89,7 +89,7 @@ describe('attribute fallthrough', () => {
id: 'test', id: 'test',
class: 'c' + this.count, class: 'c' + this.count,
style: { color: this.count ? 'red' : 'green' }, style: { color: this.count ? 'red' : 'green' },
nativeOnClick: this.inc onClick: this.inc
}) })
} }
} }
@ -102,13 +102,16 @@ describe('attribute fallthrough', () => {
childUpdated() childUpdated()
} }
render() { render() {
return h( return cloneVNode(
'div', h(
{ 'div',
class: 'c2', {
style: { fontWeight: 'bold' } class: 'c2',
}, style: { fontWeight: 'bold' }
this.$props.foo },
this.$props.foo
),
this.$attrs
) )
} }
} }
@ -121,27 +124,27 @@ describe('attribute fallthrough', () => {
// with declared props, any parent attr that isn't a prop falls through // with declared props, any parent attr that isn't a prop falls through
expect(node.getAttribute('id')).toBe('test') expect(node.getAttribute('id')).toBe('test')
// ...while declared ones remain props
expect(node.hasAttribute('foo')).toBe(false)
// class, style and nativeOn* always fallthrough
expect(node.getAttribute('class')).toBe('c2 c0') expect(node.getAttribute('class')).toBe('c2 c0')
expect(node.style.color).toBe('green') expect(node.style.color).toBe('green')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
node.dispatchEvent(new CustomEvent('click')) node.dispatchEvent(new CustomEvent('click'))
expect(nativeClick).toHaveBeenCalled() expect(click).toHaveBeenCalled()
// ...while declared ones remain props
expect(node.hasAttribute('foo')).toBe(false)
await nextTick() await nextTick()
expect(childUpdated).toHaveBeenCalled() expect(childUpdated).toHaveBeenCalled()
expect(node.getAttribute('id')).toBe('test') expect(node.getAttribute('id')).toBe('test')
expect(node.hasAttribute('foo')).toBe(false)
expect(node.getAttribute('class')).toBe('c2 c1') expect(node.getAttribute('class')).toBe('c2 c1')
expect(node.style.color).toBe('red') expect(node.style.color).toBe('red')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
expect(node.hasAttribute('foo')).toBe(false)
}) })
it('should fallthrough on multi-nested components', async () => { it('should fallthrough on multi-nested components', async () => {
const nativeClick = jest.fn() const click = jest.fn()
const childUpdated = jest.fn() const childUpdated = jest.fn()
const grandChildUpdated = jest.fn() const grandChildUpdated = jest.fn()
@ -153,7 +156,7 @@ describe('attribute fallthrough', () => {
} }
inc() { inc() {
this.count++ this.count++
nativeClick() click()
} }
render() { render() {
return h(Child, { return h(Child, {
@ -161,20 +164,17 @@ describe('attribute fallthrough', () => {
id: 'test', id: 'test',
class: 'c' + this.count, class: 'c' + this.count,
style: { color: this.count ? 'red' : 'green' }, style: { color: this.count ? 'red' : 'green' },
nativeOnClick: this.inc onClick: this.inc
}) })
} }
} }
class Child extends Component { class Child extends Component {
static props = {
foo: Number
}
updated() { updated() {
childUpdated() childUpdated()
} }
render(props: any) { render() {
return h(GrandChild, props) return h(GrandChild, this.$props)
} }
} }
@ -186,13 +186,16 @@ describe('attribute fallthrough', () => {
grandChildUpdated() grandChildUpdated()
} }
render(props: any) { render(props: any) {
return h( return cloneVNode(
'div', h(
{ 'div',
class: 'c2', {
style: { fontWeight: 'bold' } class: 'c2',
}, style: { fontWeight: 'bold' }
props.foo },
props.foo
),
this.$attrs
) )
} }
} }
@ -205,23 +208,23 @@ describe('attribute fallthrough', () => {
// with declared props, any parent attr that isn't a prop falls through // with declared props, any parent attr that isn't a prop falls through
expect(node.getAttribute('id')).toBe('test') expect(node.getAttribute('id')).toBe('test')
// ...while declared ones remain props
expect(node.hasAttribute('foo')).toBe(false)
// class, style and nativeOn* always fallthrough
expect(node.getAttribute('class')).toBe('c2 c0') expect(node.getAttribute('class')).toBe('c2 c0')
expect(node.style.color).toBe('green') expect(node.style.color).toBe('green')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
node.dispatchEvent(new CustomEvent('click')) node.dispatchEvent(new CustomEvent('click'))
expect(nativeClick).toHaveBeenCalled() expect(click).toHaveBeenCalled()
// ...while declared ones remain props
expect(node.hasAttribute('foo')).toBe(false)
await nextTick() await nextTick()
expect(childUpdated).toHaveBeenCalled() expect(childUpdated).toHaveBeenCalled()
expect(grandChildUpdated).toHaveBeenCalled() expect(grandChildUpdated).toHaveBeenCalled()
expect(node.getAttribute('id')).toBe('test') expect(node.getAttribute('id')).toBe('test')
expect(node.hasAttribute('foo')).toBe(false)
expect(node.getAttribute('class')).toBe('c2 c1') expect(node.getAttribute('class')).toBe('c2 c1')
expect(node.style.color).toBe('red') expect(node.style.color).toBe('red')
expect(node.style.fontWeight).toBe('bold') expect(node.style.fontWeight).toBe('bold')
expect(node.hasAttribute('foo')).toBe(false)
}) })
}) })

View File

@ -24,7 +24,6 @@ export interface FunctionalComponent<P = {}> {
(props: Readonly<P>, slots: Slots, attrs: Data): any (props: Readonly<P>, slots: Slots, attrs: Data): any
pure?: boolean pure?: boolean
props?: ComponentPropsOptions<P> props?: ComponentPropsOptions<P>
inheritAttrs?: boolean
displayName?: string displayName?: string
} }

View File

@ -8,7 +8,6 @@ export interface ComponentClassOptions<This = ComponentInstance> {
computed?: ComponentComputedOptions<This> computed?: ComponentComputedOptions<This>
watch?: ComponentWatchOptions<This> watch?: ComponentWatchOptions<This>
displayName?: string displayName?: string
inheritAttrs?: boolean
} }
export interface ComponentOptions<This = ComponentInstance> export interface ComponentOptions<This = ComponentInstance>

View File

@ -7,14 +7,7 @@ import {
Prop, Prop,
PropType PropType
} from './componentOptions' } from './componentOptions'
import { import { EMPTY_OBJ, camelize, hyphenate, capitalize } from './utils'
EMPTY_OBJ,
nativeOnRE,
vnodeHookRE,
camelize,
hyphenate,
capitalize
} from './utils'
export function initializeProps( export function initializeProps(
instance: ComponentInstance, instance: ComponentInstance,
@ -67,7 +60,7 @@ const EMPTY_PROPS = { props: EMPTY_OBJ }
// resolve raw VNode data. // resolve raw VNode data.
// - filter out reserved keys (key, ref, slots) // - filter out reserved keys (key, ref, slots)
// - extract class, style and nativeOn* into $attrs (to be merged onto child // - extract class and style into $attrs (to be merged onto child
// component root) // component root)
// - for the rest: // - for the rest:
// - if has declared props: put declared ones in `props`, the rest in `attrs` // - if has declared props: put declared ones in `props`, the rest in `attrs`
@ -89,17 +82,9 @@ export function resolveProps(
if (key === 'key' || key === 'ref' || key === 'slots') { if (key === 'key' || key === 'ref' || key === 'slots') {
continue continue
} }
// class, style, nativeOn & directive hooks are always extracted into a // any non-declared data are put into a separate `attrs` object
// separate `attrs` object, which can then be merged onto child component // for spreading
// root. in addition, if the component has explicitly declared props, then if (hasDeclaredProps && !options.hasOwnProperty(key)) {
// any non-matching props are extracted into `attrs` as well.
if (
key === 'class' ||
key === 'style' ||
vnodeHookRE.test(key) ||
nativeOnRE.test(key) ||
(hasDeclaredProps && !options.hasOwnProperty(key))
) {
;(attrs || (attrs = {}))[key] = rawData[key] ;(attrs || (attrs = {}))[key] = rawData[key]
} else { } else {
props[key] = rawData[key] props[key] = rawData[key]

View File

@ -55,11 +55,12 @@ export function createComponentInstance(
export function renderInstanceRoot(instance: ComponentInstance): VNode { export function renderInstanceRoot(instance: ComponentInstance): VNode {
let vnode let vnode
try { try {
vnode = instance.render.call(instance.$proxy, h, { vnode = instance.render.call(
props: instance.$props, instance.$proxy,
slots: instance.$slots, instance.$props,
attrs: instance.$attrs instance.$slots,
}) instance.$attrs
)
} catch (e1) { } catch (e1) {
handleError(e1, instance, ErrorTypes.RENDER) handleError(e1, instance, ErrorTypes.RENDER)
if (__DEV__ && instance.renderError) { if (__DEV__ && instance.renderError) {
@ -70,12 +71,7 @@ export function renderInstanceRoot(instance: ComponentInstance): VNode {
} }
} }
} }
return normalizeComponentRoot( return normalizeComponentRoot(vnode, instance.$parentVNode)
vnode,
instance.$parentVNode,
instance.$attrs,
instance.constructor.inheritAttrs
)
} }
export function teardownComponentInstance(instance: ComponentInstance) { export function teardownComponentInstance(instance: ComponentInstance) {
@ -95,9 +91,7 @@ export function teardownComponentInstance(instance: ComponentInstance) {
export function normalizeComponentRoot( export function normalizeComponentRoot(
vnode: any, vnode: any,
componentVNode: VNode | null, componentVNode: VNode | null
attrs: Record<string, any> | void,
inheritAttrs: boolean | void
): VNode { ): VNode {
if (vnode == null) { if (vnode == null) {
vnode = createTextVNode('') vnode = createTextVNode('')
@ -105,12 +99,7 @@ export function normalizeComponentRoot(
vnode = createTextVNode(vnode + '') vnode = createTextVNode(vnode + '')
} else if (Array.isArray(vnode)) { } else if (Array.isArray(vnode)) {
if (vnode.length === 1) { if (vnode.length === 1) {
vnode = normalizeComponentRoot( vnode = normalizeComponentRoot(vnode[0], componentVNode)
vnode[0],
componentVNode,
attrs,
inheritAttrs
)
} else { } else {
vnode = createFragment(vnode) vnode = createFragment(vnode)
} }
@ -120,13 +109,7 @@ export function normalizeComponentRoot(
componentVNode && componentVNode &&
(flags & VNodeFlags.COMPONENT || flags & VNodeFlags.ELEMENT) (flags & VNodeFlags.COMPONENT || flags & VNodeFlags.ELEMENT)
) { ) {
if ( if (el) {
inheritAttrs !== false &&
attrs !== void 0 &&
Object.keys(attrs).length > 0
) {
vnode = cloneVNode(vnode, attrs)
} else if (el) {
vnode = cloneVNode(vnode) vnode = cloneVNode(vnode)
} }
if (flags & VNodeFlags.COMPONENT) { if (flags & VNodeFlags.COMPONENT) {
@ -188,6 +171,8 @@ export function createComponentClassFromOptions(
proto.beforeUnmount = value proto.beforeUnmount = value
} else if (key === 'destroyed') { } else if (key === 'destroyed') {
proto.unmounted = value proto.unmounted = value
} else {
proto[key] = value
} }
} else { } else {
proto[key] = value proto[key] = value

View File

@ -238,9 +238,7 @@ export function createRenderer(options: RendererOptions) {
const { props, attrs } = resolveProps(data, render.props) const { props, attrs } = resolveProps(data, render.props)
const subTree = (vnode.children = normalizeComponentRoot( const subTree = (vnode.children = normalizeComponentRoot(
render(props, slots || EMPTY_OBJ, attrs || EMPTY_OBJ), render(props, slots || EMPTY_OBJ, attrs || EMPTY_OBJ),
vnode, vnode
attrs,
render.inheritAttrs
)) ))
mount(subTree, container, parentComponent, isSVG, endNode) mount(subTree, container, parentComponent, isSVG, endNode)
vnode.el = subTree.el as RenderNode vnode.el = subTree.el as RenderNode
@ -526,9 +524,7 @@ export function createRenderer(options: RendererOptions) {
const { props, attrs } = resolveProps(nextData, render.props) const { props, attrs } = resolveProps(nextData, render.props)
const nextTree = (nextVNode.children = normalizeComponentRoot( const nextTree = (nextVNode.children = normalizeComponentRoot(
render(props, nextSlots || EMPTY_OBJ, attrs || EMPTY_OBJ), render(props, nextSlots || EMPTY_OBJ, attrs || EMPTY_OBJ),
nextVNode, nextVNode
attrs,
render.inheritAttrs
)) ))
patch(prevTree, nextTree, container, parentComponent, isSVG) patch(prevTree, nextTree, container, parentComponent, isSVG)
nextVNode.el = nextTree.el nextVNode.el = nextTree.el

View File

@ -9,7 +9,7 @@ import {
createFragment, createFragment,
createPortal createPortal
} from './vdom' } from './vdom'
import { isObservable, unwrap } from '@vue/observer' import { isObservable } from '@vue/observer'
export const Fragment = Symbol() export const Fragment = Symbol()
export const Portal = Symbol() export const Portal = Symbol()
@ -44,9 +44,9 @@ export const h = ((tag: ElementType, data?: any, children?: any): VNode => {
if (children === void 0) children = null if (children === void 0) children = null
// if value is observable, create a clone of original // if value is observable, create a clone of original
// so that we can mutate it later on. // so that we can normalize its class/style
if (isObservable(data)) { if (isObservable(data)) {
data = Object.assign({}, unwrap(data)) data = Object.assign({}, data)
} }
let key = null let key = null

View File

@ -11,7 +11,10 @@ export * from '@vue/observer'
export { nextTick } from '@vue/scheduler' export { nextTick } from '@vue/scheduler'
// Internal API // Internal API
export { createComponentInstance } from './componentUtils' export {
createComponentInstance,
createComponentClassFromOptions
} from './componentUtils'
// Optional APIs // Optional APIs
// these are imported on-demand and can be tree-shaken // these are imported on-demand and can be tree-shaken

View File

@ -3,9 +3,8 @@ export const EMPTY_OBJ: { readonly [key: string]: any } = Object.freeze({})
export const NOOP = () => {} export const NOOP = () => {}
export const onRE = /^on/ export const onRE = /^on/
export const nativeOnRE = /^nativeOn/
export const vnodeHookRE = /^vnode/ export const vnodeHookRE = /^vnode/
export const handlersRE = /^on|^nativeOn|^vnode/ export const handlersRE = /^on|^vnode/
export const reservedPropRE = /^(?:key|ref|slots)$|^vnode/ export const reservedPropRE = /^(?:key|ref|slots)$|^vnode/
export function normalizeStyle( export function normalizeStyle(

View File

@ -277,7 +277,7 @@ export function cloneVNode(vnode: VNode, extraData?: VNodeData): VNode {
extraData.style extraData.style
]) ])
} else if (handlersRE.test(key)) { } else if (handlersRE.test(key)) {
// on*, nativeOn*, vnode* // on*, vnode*
const existing = clonedData[key] const existing = clonedData[key]
clonedData[key] = existing clonedData[key] = existing
? [].concat(existing, extraData[key]) ? [].concat(existing, extraData[key])

View File

@ -5,7 +5,7 @@ import { patchAttr } from './modules/attrs'
import { patchDOMProp } from './modules/props' import { patchDOMProp } from './modules/props'
import { patchEvent } from './modules/events' import { patchEvent } from './modules/events'
export const onRE = /^on|^nativeOn/ export const onRE = /^on/
const domPropsRE = /^domProps/ const domPropsRE = /^domProps/
export function patchData( export function patchData(

View File

@ -3,7 +3,8 @@ import {
render, render,
nextTick, nextTick,
Component, Component,
createComponentInstance createComponentInstance,
createComponentClassFromOptions
} from '@vue/renderer-dom' } from '@vue/renderer-dom'
// Note: typing for this is intentionally loose, as it will be using 2.x types. // Note: typing for this is intentionally loose, as it will be using 2.x types.
@ -20,9 +21,10 @@ class Vue extends Component {
// in compat mode, h() can take an options object and will convert it // in compat mode, h() can take an options object and will convert it
// to a 3.x class-based component. // to a 3.x class-based component.
const vnode = h(options) const Component = createComponentClassFromOptions(options)
const vnode = h(Component)
// the component class is cached on the options object as ._normalized // the component class is cached on the options object as ._normalized
const instance = createComponentInstance(vnode, options._normalized, null) const instance = createComponentInstance(vnode, Component, null)
// set the instance on the vnode before mounting. // set the instance on the vnode before mounting.
// the mount function will skip creating a new instance if it finds an // the mount function will skip creating a new instance if it finds an
// existing one. // existing one.
@ -48,4 +50,3 @@ interface Vue {
} }
export default Vue export default Vue
export * from '@vue/renderer-dom'