From 83605ef26c37ee2c56a189d5f10a378052fbbc93 Mon Sep 17 00:00:00 2001 From: Evan You Date: Tue, 9 Oct 2018 21:10:30 -0400 Subject: [PATCH] refactor: remove inheritAttrs / nativeOn, enforce explicit fallthrough --- .../core/__tests__/attrsFallthrough.spec.ts | 123 +++++++++--------- packages/core/src/component.ts | 1 - packages/core/src/componentOptions.ts | 1 - packages/core/src/componentProps.ts | 25 +--- packages/core/src/componentUtils.ts | 39 ++---- packages/core/src/createRenderer.ts | 8 +- packages/core/src/h.ts | 6 +- packages/core/src/index.ts | 5 +- packages/core/src/utils.ts | 3 +- packages/core/src/vdom.ts | 2 +- packages/renderer-dom/src/patchData.ts | 2 +- packages/vue/src/index.ts | 9 +- 12 files changed, 97 insertions(+), 127 deletions(-) diff --git a/packages/core/__tests__/attrsFallthrough.spec.ts b/packages/core/__tests__/attrsFallthrough.spec.ts index 7d59e19e..41bf51bd 100644 --- a/packages/core/__tests__/attrsFallthrough.spec.ts +++ b/packages/core/__tests__/attrsFallthrough.spec.ts @@ -1,9 +1,9 @@ // 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', () => { - it('should not fallthrough on components with no declared props', async () => { - const nativeClick = jest.fn() + it('everything should be in props when component has no declared props', async () => { + const click = jest.fn() const childUpdated = jest.fn() class Hello extends Component<{}, { count: number }> { @@ -14,7 +14,7 @@ describe('attribute fallthrough', () => { } inc() { this.count++ - nativeClick() + click() } render() { return h(Child, { @@ -22,23 +22,26 @@ describe('attribute fallthrough', () => { id: 'test', class: 'c' + this.count, style: { color: this.count ? 'red' : 'green' }, - nativeOnClick: this.inc + onClick: this.inc }) } } - class Child extends Component<{ foo: number }> { + class Child extends Component { updated() { childUpdated() } - render() { - return h( - 'div', - { - class: 'c2', - style: { fontWeight: 'bold' } - }, - this.$props.foo + render(props: any) { + return cloneVNode( + h( + 'div', + { + class: 'c2', + style: { fontWeight: 'bold' } + }, + props.foo + ), + props ) } } @@ -49,28 +52,25 @@ describe('attribute fallthrough', () => { const node = root.children[0] as HTMLElement - // attrs do not fallthrough because no props are declared - expect(node.hasAttribute('id')).toBe(false) - expect(node.hasAttribute('foo')).toBe(false) - - // class, style and nativeOn* always fallthrough + 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') node.dispatchEvent(new CustomEvent('click')) - expect(nativeClick).toHaveBeenCalled() + expect(click).toHaveBeenCalled() await nextTick() expect(childUpdated).toHaveBeenCalled() - expect(node.hasAttribute('id')).toBe(false) - expect(node.hasAttribute('foo')).toBe(false) + expect(node.getAttribute('id')).toBe('test') + expect(node.getAttribute('foo')).toBe('1') expect(node.getAttribute('class')).toBe('c2 c1') expect(node.style.color).toBe('red') expect(node.style.fontWeight).toBe('bold') }) - it('should fallthrough on components with declared props', async () => { - const nativeClick = jest.fn() + it('should separate in attrs when component has declared props', async () => { + const click = jest.fn() const childUpdated = jest.fn() class Hello extends Component<{}, { count: number }> { @@ -81,7 +81,7 @@ describe('attribute fallthrough', () => { } inc() { this.count++ - nativeClick() + click() } render() { return h(Child, { @@ -89,7 +89,7 @@ describe('attribute fallthrough', () => { id: 'test', class: 'c' + this.count, style: { color: this.count ? 'red' : 'green' }, - nativeOnClick: this.inc + onClick: this.inc }) } } @@ -102,13 +102,16 @@ describe('attribute fallthrough', () => { childUpdated() } render() { - return h( - 'div', - { - class: 'c2', - style: { fontWeight: 'bold' } - }, - this.$props.foo + return cloneVNode( + h( + 'div', + { + class: 'c2', + style: { fontWeight: 'bold' } + }, + 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 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.style.color).toBe('green') expect(node.style.fontWeight).toBe('bold') node.dispatchEvent(new CustomEvent('click')) - expect(nativeClick).toHaveBeenCalled() + expect(click).toHaveBeenCalled() + + // ...while declared ones remain props + expect(node.hasAttribute('foo')).toBe(false) await nextTick() expect(childUpdated).toHaveBeenCalled() expect(node.getAttribute('id')).toBe('test') - expect(node.hasAttribute('foo')).toBe(false) expect(node.getAttribute('class')).toBe('c2 c1') expect(node.style.color).toBe('red') expect(node.style.fontWeight).toBe('bold') + + expect(node.hasAttribute('foo')).toBe(false) }) it('should fallthrough on multi-nested components', async () => { - const nativeClick = jest.fn() + const click = jest.fn() const childUpdated = jest.fn() const grandChildUpdated = jest.fn() @@ -153,7 +156,7 @@ describe('attribute fallthrough', () => { } inc() { this.count++ - nativeClick() + click() } render() { return h(Child, { @@ -161,20 +164,17 @@ describe('attribute fallthrough', () => { id: 'test', class: 'c' + this.count, style: { color: this.count ? 'red' : 'green' }, - nativeOnClick: this.inc + onClick: this.inc }) } } class Child extends Component { - static props = { - foo: Number - } updated() { childUpdated() } - render(props: any) { - return h(GrandChild, props) + render() { + return h(GrandChild, this.$props) } } @@ -186,13 +186,16 @@ describe('attribute fallthrough', () => { grandChildUpdated() } render(props: any) { - return h( - 'div', - { - class: 'c2', - style: { fontWeight: 'bold' } - }, - props.foo + return cloneVNode( + h( + 'div', + { + class: 'c2', + style: { fontWeight: 'bold' } + }, + props.foo + ), + this.$attrs ) } } @@ -205,23 +208,23 @@ describe('attribute fallthrough', () => { // with declared props, any parent attr that isn't a prop falls through 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.style.color).toBe('green') expect(node.style.fontWeight).toBe('bold') node.dispatchEvent(new CustomEvent('click')) - expect(nativeClick).toHaveBeenCalled() + expect(click).toHaveBeenCalled() + + // ...while declared ones remain props + expect(node.hasAttribute('foo')).toBe(false) await nextTick() expect(childUpdated).toHaveBeenCalled() expect(grandChildUpdated).toHaveBeenCalled() expect(node.getAttribute('id')).toBe('test') - expect(node.hasAttribute('foo')).toBe(false) expect(node.getAttribute('class')).toBe('c2 c1') expect(node.style.color).toBe('red') expect(node.style.fontWeight).toBe('bold') + + expect(node.hasAttribute('foo')).toBe(false) }) }) diff --git a/packages/core/src/component.ts b/packages/core/src/component.ts index 9f01c284..ec5254d4 100644 --- a/packages/core/src/component.ts +++ b/packages/core/src/component.ts @@ -24,7 +24,6 @@ export interface FunctionalComponent

{ (props: Readonly

, slots: Slots, attrs: Data): any pure?: boolean props?: ComponentPropsOptions

- inheritAttrs?: boolean displayName?: string } diff --git a/packages/core/src/componentOptions.ts b/packages/core/src/componentOptions.ts index a75148ba..690daab1 100644 --- a/packages/core/src/componentOptions.ts +++ b/packages/core/src/componentOptions.ts @@ -8,7 +8,6 @@ export interface ComponentClassOptions { computed?: ComponentComputedOptions watch?: ComponentWatchOptions displayName?: string - inheritAttrs?: boolean } export interface ComponentOptions diff --git a/packages/core/src/componentProps.ts b/packages/core/src/componentProps.ts index fe95aa81..e10d6a95 100644 --- a/packages/core/src/componentProps.ts +++ b/packages/core/src/componentProps.ts @@ -7,14 +7,7 @@ import { Prop, PropType } from './componentOptions' -import { - EMPTY_OBJ, - nativeOnRE, - vnodeHookRE, - camelize, - hyphenate, - capitalize -} from './utils' +import { EMPTY_OBJ, camelize, hyphenate, capitalize } from './utils' export function initializeProps( instance: ComponentInstance, @@ -67,7 +60,7 @@ const EMPTY_PROPS = { props: EMPTY_OBJ } // resolve raw VNode data. // - 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) // - for the rest: // - 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') { continue } - // class, style, nativeOn & directive hooks are always extracted into a - // separate `attrs` object, which can then be merged onto child component - // root. in addition, if the component has explicitly declared props, then - // 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)) - ) { + // any non-declared data are put into a separate `attrs` object + // for spreading + if (hasDeclaredProps && !options.hasOwnProperty(key)) { ;(attrs || (attrs = {}))[key] = rawData[key] } else { props[key] = rawData[key] diff --git a/packages/core/src/componentUtils.ts b/packages/core/src/componentUtils.ts index 04d8705c..cd58957c 100644 --- a/packages/core/src/componentUtils.ts +++ b/packages/core/src/componentUtils.ts @@ -55,11 +55,12 @@ export function createComponentInstance( export function renderInstanceRoot(instance: ComponentInstance): VNode { let vnode try { - vnode = instance.render.call(instance.$proxy, h, { - props: instance.$props, - slots: instance.$slots, - attrs: instance.$attrs - }) + vnode = instance.render.call( + instance.$proxy, + instance.$props, + instance.$slots, + instance.$attrs + ) } catch (e1) { handleError(e1, instance, ErrorTypes.RENDER) if (__DEV__ && instance.renderError) { @@ -70,12 +71,7 @@ export function renderInstanceRoot(instance: ComponentInstance): VNode { } } } - return normalizeComponentRoot( - vnode, - instance.$parentVNode, - instance.$attrs, - instance.constructor.inheritAttrs - ) + return normalizeComponentRoot(vnode, instance.$parentVNode) } export function teardownComponentInstance(instance: ComponentInstance) { @@ -95,9 +91,7 @@ export function teardownComponentInstance(instance: ComponentInstance) { export function normalizeComponentRoot( vnode: any, - componentVNode: VNode | null, - attrs: Record | void, - inheritAttrs: boolean | void + componentVNode: VNode | null ): VNode { if (vnode == null) { vnode = createTextVNode('') @@ -105,12 +99,7 @@ export function normalizeComponentRoot( vnode = createTextVNode(vnode + '') } else if (Array.isArray(vnode)) { if (vnode.length === 1) { - vnode = normalizeComponentRoot( - vnode[0], - componentVNode, - attrs, - inheritAttrs - ) + vnode = normalizeComponentRoot(vnode[0], componentVNode) } else { vnode = createFragment(vnode) } @@ -120,13 +109,7 @@ export function normalizeComponentRoot( componentVNode && (flags & VNodeFlags.COMPONENT || flags & VNodeFlags.ELEMENT) ) { - if ( - inheritAttrs !== false && - attrs !== void 0 && - Object.keys(attrs).length > 0 - ) { - vnode = cloneVNode(vnode, attrs) - } else if (el) { + if (el) { vnode = cloneVNode(vnode) } if (flags & VNodeFlags.COMPONENT) { @@ -188,6 +171,8 @@ export function createComponentClassFromOptions( proto.beforeUnmount = value } else if (key === 'destroyed') { proto.unmounted = value + } else { + proto[key] = value } } else { proto[key] = value diff --git a/packages/core/src/createRenderer.ts b/packages/core/src/createRenderer.ts index f03acf09..cf55e002 100644 --- a/packages/core/src/createRenderer.ts +++ b/packages/core/src/createRenderer.ts @@ -238,9 +238,7 @@ export function createRenderer(options: RendererOptions) { const { props, attrs } = resolveProps(data, render.props) const subTree = (vnode.children = normalizeComponentRoot( render(props, slots || EMPTY_OBJ, attrs || EMPTY_OBJ), - vnode, - attrs, - render.inheritAttrs + vnode )) mount(subTree, container, parentComponent, isSVG, endNode) vnode.el = subTree.el as RenderNode @@ -526,9 +524,7 @@ export function createRenderer(options: RendererOptions) { const { props, attrs } = resolveProps(nextData, render.props) const nextTree = (nextVNode.children = normalizeComponentRoot( render(props, nextSlots || EMPTY_OBJ, attrs || EMPTY_OBJ), - nextVNode, - attrs, - render.inheritAttrs + nextVNode )) patch(prevTree, nextTree, container, parentComponent, isSVG) nextVNode.el = nextTree.el diff --git a/packages/core/src/h.ts b/packages/core/src/h.ts index f423ddeb..d56950ef 100644 --- a/packages/core/src/h.ts +++ b/packages/core/src/h.ts @@ -9,7 +9,7 @@ import { createFragment, createPortal } from './vdom' -import { isObservable, unwrap } from '@vue/observer' +import { isObservable } from '@vue/observer' export const Fragment = 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 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)) { - data = Object.assign({}, unwrap(data)) + data = Object.assign({}, data) } let key = null diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d4e203b1..6c2d323c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -11,7 +11,10 @@ export * from '@vue/observer' export { nextTick } from '@vue/scheduler' // Internal API -export { createComponentInstance } from './componentUtils' +export { + createComponentInstance, + createComponentClassFromOptions +} from './componentUtils' // Optional APIs // these are imported on-demand and can be tree-shaken diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 25cd87f0..701aa69c 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -3,9 +3,8 @@ export const EMPTY_OBJ: { readonly [key: string]: any } = Object.freeze({}) export const NOOP = () => {} export const onRE = /^on/ -export const nativeOnRE = /^nativeOn/ export const vnodeHookRE = /^vnode/ -export const handlersRE = /^on|^nativeOn|^vnode/ +export const handlersRE = /^on|^vnode/ export const reservedPropRE = /^(?:key|ref|slots)$|^vnode/ export function normalizeStyle( diff --git a/packages/core/src/vdom.ts b/packages/core/src/vdom.ts index d5dfd2ae..0719bdfb 100644 --- a/packages/core/src/vdom.ts +++ b/packages/core/src/vdom.ts @@ -277,7 +277,7 @@ export function cloneVNode(vnode: VNode, extraData?: VNodeData): VNode { extraData.style ]) } else if (handlersRE.test(key)) { - // on*, nativeOn*, vnode* + // on*, vnode* const existing = clonedData[key] clonedData[key] = existing ? [].concat(existing, extraData[key]) diff --git a/packages/renderer-dom/src/patchData.ts b/packages/renderer-dom/src/patchData.ts index fef6ce56..be805a54 100644 --- a/packages/renderer-dom/src/patchData.ts +++ b/packages/renderer-dom/src/patchData.ts @@ -5,7 +5,7 @@ import { patchAttr } from './modules/attrs' import { patchDOMProp } from './modules/props' import { patchEvent } from './modules/events' -export const onRE = /^on|^nativeOn/ +export const onRE = /^on/ const domPropsRE = /^domProps/ export function patchData( diff --git a/packages/vue/src/index.ts b/packages/vue/src/index.ts index 72675acf..7fed1d2f 100644 --- a/packages/vue/src/index.ts +++ b/packages/vue/src/index.ts @@ -3,7 +3,8 @@ import { render, nextTick, Component, - createComponentInstance + createComponentInstance, + createComponentClassFromOptions } from '@vue/renderer-dom' // 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 // 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 - const instance = createComponentInstance(vnode, options._normalized, null) + const instance = createComponentInstance(vnode, Component, null) // set the instance on the vnode before mounting. // the mount function will skip creating a new instance if it finds an // existing one. @@ -48,4 +50,3 @@ interface Vue { } export default Vue -export * from '@vue/renderer-dom'