From ff4d1fcd81d96f3ddb0e34f04e70e3539dc7a96f Mon Sep 17 00:00:00 2001 From: Evan You Date: Wed, 22 Apr 2020 16:36:18 -0400 Subject: [PATCH] fix(runtime-core): mixin options that rely on this context should be deferred Also ensure consistent option apply order with Vue 2, close #1016, close #1029 --- .../runtime-core/__tests__/apiOptions.spec.ts | 406 ++++++++++-------- packages/runtime-core/src/componentOptions.ts | 194 +++++---- 2 files changed, 340 insertions(+), 260 deletions(-) diff --git a/packages/runtime-core/__tests__/apiOptions.spec.ts b/packages/runtime-core/__tests__/apiOptions.spec.ts index 65115804..da99dd4a 100644 --- a/packages/runtime-core/__tests__/apiOptions.spec.ts +++ b/packages/runtime-core/__tests__/apiOptions.spec.ts @@ -562,6 +562,51 @@ describe('api: options', () => { expect(serializeInner(root)).toBe(`
1,1,3
`) }) + // #1016 + test('watcher initialization should be deferred in mixins', async () => { + const mixin1 = { + data() { + return { + mixin1Data: 'mixin1' + } + }, + methods: {} + } + + const watchSpy = jest.fn() + const mixin2 = { + watch: { + mixin3Data: watchSpy + } + } + + const mixin3 = { + data() { + return { + mixin3Data: 'mixin3' + } + }, + methods: {} + } + + let vm: any + const Comp = { + mixins: [mixin1, mixin2, mixin3], + render() {}, + created() { + vm = this + } + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + + // should have no warnings + vm.mixin3Data = 'hello' + await nextTick() + expect(watchSpy.mock.calls[0].slice(0, 2)).toEqual(['hello', 'mixin3']) + }) + describe('warnings', () => { mockWarn() @@ -631,164 +676,6 @@ describe('api: options', () => { ).toHaveBeenWarned() }) - test('data property is already declared in props', () => { - const Comp = { - props: { foo: Number }, - data: () => ({ - foo: 1 - }), - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Data property "foo" is already defined in Props.` - ).toHaveBeenWarned() - }) - - test('computed property is already declared in data', () => { - const Comp = { - data: () => ({ - foo: 1 - }), - computed: { - foo() {} - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Computed property "foo" is already defined in Data.` - ).toHaveBeenWarned() - }) - - test('computed property is already declared in props', () => { - const Comp = { - props: { foo: Number }, - computed: { - foo() {} - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Computed property "foo" is already defined in Props.` - ).toHaveBeenWarned() - }) - - test('methods property is not a function', () => { - const Comp = { - methods: { - foo: 1 - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Method "foo" has type "number" in the component definition. ` + - `Did you reference the function correctly?` - ).toHaveBeenWarned() - }) - - test('methods property is already declared in data', () => { - const Comp = { - data: () => ({ - foo: 2 - }), - methods: { - foo() {} - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Methods property "foo" is already defined in Data.` - ).toHaveBeenWarned() - }) - - test('methods property is already declared in props', () => { - const Comp = { - props: { - foo: Number - }, - methods: { - foo() {} - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Methods property "foo" is already defined in Props.` - ).toHaveBeenWarned() - }) - - test('methods property is already declared in computed', () => { - const Comp = { - computed: { - foo: { - get() {}, - set() {} - } - }, - methods: { - foo() {} - }, - render() {} - } - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Methods property "foo" is already defined in Computed.` - ).toHaveBeenWarned() - }) - - test('inject property is already declared in data', () => { - const Comp = { - data() { - return { - a: 1 - } - }, - provide() { - return { - a: this.a - } - }, - render() { - return [h(ChildA)] - } - } as any - const ChildA = { - data() { - return { - a: 1 - } - }, - inject: ['a'], - render() { - return this.a - } - } as any - - const root = nodeOps.createElement('div') - render(h(Comp), root) - expect( - `Inject property "a" is already defined in Data.` - ).toHaveBeenWarned() - }) - test('inject property is already declared in props', () => { const Comp = { data() { @@ -820,7 +707,159 @@ describe('api: options', () => { ).toHaveBeenWarned() }) - test('inject property is already declared in computed', () => { + test('methods property is not a function', () => { + const Comp = { + methods: { + foo: 1 + }, + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Method "foo" has type "number" in the component definition. ` + + `Did you reference the function correctly?` + ).toHaveBeenWarned() + }) + + test('methods property is already declared in props', () => { + const Comp = { + props: { + foo: Number + }, + methods: { + foo() {} + }, + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Methods property "foo" is already defined in Props.` + ).toHaveBeenWarned() + }) + + test('methods property is already declared in inject', () => { + const Comp = { + data() { + return { + a: 1 + } + }, + provide() { + return { + a: this.a + } + }, + render() { + return [h(ChildA)] + } + } as any + const ChildA = { + methods: { + a: () => null + }, + inject: ['a'], + render() { + return this.a + } + } as any + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Methods property "a" is already defined in Inject.` + ).toHaveBeenWarned() + }) + + test('data property is already declared in props', () => { + const Comp = { + props: { foo: Number }, + data: () => ({ + foo: 1 + }), + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Data property "foo" is already defined in Props.` + ).toHaveBeenWarned() + }) + + test('data property is already declared in inject', () => { + const Comp = { + data() { + return { + a: 1 + } + }, + provide() { + return { + a: this.a + } + }, + render() { + return [h(ChildA)] + } + } as any + const ChildA = { + data() { + return { + a: 1 + } + }, + inject: ['a'], + render() { + return this.a + } + } as any + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Data property "a" is already defined in Inject.` + ).toHaveBeenWarned() + }) + + test('data property is already declared in methods', () => { + const Comp = { + data: () => ({ + foo: 1 + }), + methods: { + foo() {} + }, + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Data property "foo" is already defined in Methods.` + ).toHaveBeenWarned() + }) + + test('computed property is already declared in props', () => { + const Comp = { + props: { foo: Number }, + computed: { + foo() {} + }, + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Computed property "foo" is already defined in Props.` + ).toHaveBeenWarned() + }) + + test('computed property is already declared in inject', () => { const Comp = { data() { return { @@ -852,40 +891,43 @@ describe('api: options', () => { const root = nodeOps.createElement('div') render(h(Comp), root) expect( - `Inject property "a" is already defined in Computed.` + `Computed property "a" is already defined in Inject.` ).toHaveBeenWarned() }) - test('inject property is already declared in methods', () => { + test('computed property is already declared in methods', () => { const Comp = { - data() { - return { - a: 1 - } + computed: { + foo() {} }, - provide() { - return { - a: this.a - } - }, - render() { - return [h(ChildA)] - } - } as any - const ChildA = { methods: { - a: () => null + foo() {} }, - inject: ['a'], - render() { - return this.a - } - } as any + render() {} + } const root = nodeOps.createElement('div') render(h(Comp), root) expect( - `Inject property "a" is already defined in Methods.` + `Computed property "foo" is already defined in Methods.` + ).toHaveBeenWarned() + }) + + test('computed property is already declared in data', () => { + const Comp = { + data: () => ({ + foo: 1 + }), + computed: { + foo() {} + }, + render() {} + } + + const root = nodeOps.createElement('div') + render(h(Comp), root) + expect( + `Computed property "foo" is already defined in Data.` ).toHaveBeenWarned() }) }) diff --git a/packages/runtime-core/src/componentOptions.ts b/packages/runtime-core/src/componentOptions.ts index 40c861cb..78ef9c3d 100644 --- a/packages/runtime-core/src/componentOptions.ts +++ b/packages/runtime-core/src/componentOptions.ts @@ -39,7 +39,8 @@ import { import { reactive, ComputedGetter, - WritableComputedOptions + WritableComputedOptions, + toRaw } from '@vue/reactivity' import { ComponentObjectPropsOptions, @@ -260,12 +261,15 @@ function createDuplicateChecker() { } } +type DataFn = (vm: ComponentPublicInstance) => any + export function applyOptions( instance: ComponentInternalInstance, options: ComponentOptions, + deferredData: DataFn[] = [], + deferredWatch: ComponentWatchOptions[] = [], asMixin: boolean = false ) { - const publicThis = instance.proxy! const { // composition mixins, @@ -295,6 +299,7 @@ export function applyOptions( errorCaptured } = options + const publicThis = instance.proxy! const ctx = instance.ctx const globalMixins = instance.appContext.mixins // call it only during dev @@ -303,15 +308,15 @@ export function applyOptions( if (!asMixin) { callSyncHook('beforeCreate', options, publicThis, globalMixins) // global mixins are applied first - applyMixins(instance, globalMixins) + applyMixins(instance, globalMixins, deferredData, deferredWatch) } // extending a base component... if (extendsOptions) { - applyOptions(instance, extendsOptions, true) + applyOptions(instance, extendsOptions, deferredData, deferredWatch, true) } // local mixins if (mixins) { - applyMixins(instance, mixins) + applyMixins(instance, mixins, deferredData, deferredWatch) } const checkDuplicateProperties = __DEV__ ? createDuplicateChecker() : null @@ -322,7 +327,55 @@ export function applyOptions( } } - // state options + // options initialization order (to be consistent with Vue 2): + // - props (already done outside of this function) + // - inject + // - methods + // - data (deferred since it relies on `this` access) + // - computed + // - watch (deferred since it relies on `this` access) + + if (injectOptions) { + if (isArray(injectOptions)) { + for (let i = 0; i < injectOptions.length; i++) { + const key = injectOptions[i] + ctx[key] = inject(key) + if (__DEV__) { + checkDuplicateProperties!(OptionTypes.INJECT, key) + } + } + } else { + for (const key in injectOptions) { + const opt = injectOptions[key] + if (isObject(opt)) { + ctx[key] = inject(opt.from, opt.default) + } else { + ctx[key] = inject(opt) + } + if (__DEV__) { + checkDuplicateProperties!(OptionTypes.INJECT, key) + } + } + } + } + + if (methods) { + for (const key in methods) { + const methodHandler = (methods as MethodOptions)[key] + if (isFunction(methodHandler)) { + ctx[key] = methodHandler.bind(publicThis) + if (__DEV__) { + checkDuplicateProperties!(OptionTypes.METHODS, key) + } + } else if (__DEV__) { + warn( + `Method "${key}" has type "${typeof methodHandler}" in the component definition. ` + + `Did you reference the function correctly?` + ) + } + } + } + if (dataOptions) { if (__DEV__ && !isFunction(dataOptions)) { warn( @@ -330,33 +383,29 @@ export function applyOptions( `Plain object usage is no longer supported.` ) } - const data = dataOptions.call(publicThis, publicThis) - if (__DEV__ && isPromise(data)) { - warn( - `data() returned a Promise - note data() cannot be async; If you ` + - `intend to perform data fetching before component renders, use ` + - `async setup() + .` - ) - } - if (!isObject(data)) { - __DEV__ && warn(`data() should return an object.`) - } else if (instance.data === EMPTY_OBJ) { - if (__DEV__) { - for (const key in data) { - checkDuplicateProperties!(OptionTypes.DATA, key) - // expose data on ctx during dev - Object.defineProperty(ctx, key, { - configurable: true, - enumerable: true, - get: () => data[key], - set: NOOP - }) - } - } - instance.data = reactive(data) + + if (asMixin) { + deferredData.push(dataOptions as DataFn) } else { - // existing data: this is a mixin or extends. - extend(instance.data, data) + resolveData(instance, dataOptions, publicThis) + } + } + if (!asMixin) { + if (deferredData.length) { + deferredData.forEach(dataFn => resolveData(instance, dataFn, publicThis)) + } + if (__DEV__) { + const rawData = toRaw(instance.data) + for (const key in rawData) { + checkDuplicateProperties!(OptionTypes.DATA, key) + // expose data on ctx during dev + Object.defineProperty(ctx, key, { + configurable: true, + enumerable: true, + get: () => rawData[key], + set: NOOP + }) + } } } @@ -397,27 +446,15 @@ export function applyOptions( } } - if (methods) { - for (const key in methods) { - const methodHandler = (methods as MethodOptions)[key] - if (isFunction(methodHandler)) { - ctx[key] = methodHandler.bind(publicThis) - if (__DEV__) { - checkDuplicateProperties!(OptionTypes.METHODS, key) - } - } else if (__DEV__) { - warn( - `Method "${key}" has type "${typeof methodHandler}" in the component definition. ` + - `Did you reference the function correctly?` - ) - } - } - } - if (watchOptions) { - for (const key in watchOptions) { - createWatcher(watchOptions[key], ctx, publicThis, key) - } + deferredWatch.push(watchOptions) + } + if (!asMixin && deferredWatch.length) { + deferredWatch.forEach(watchOptions => { + for (const key in watchOptions) { + createWatcher(watchOptions[key], ctx, publicThis, key) + } + }) } if (provideOptions) { @@ -429,30 +466,6 @@ export function applyOptions( } } - if (injectOptions) { - if (isArray(injectOptions)) { - for (let i = 0; i < injectOptions.length; i++) { - const key = injectOptions[i] - ctx[key] = inject(key) - if (__DEV__) { - checkDuplicateProperties!(OptionTypes.INJECT, key) - } - } - } else { - for (const key in injectOptions) { - const opt = injectOptions[key] - if (isObject(opt)) { - ctx[key] = inject(opt.from, opt.default) - } else { - ctx[key] = inject(opt) - } - if (__DEV__) { - checkDuplicateProperties!(OptionTypes.INJECT, key) - } - } - } - } - // asset options if (components) { extend(instance.components, components) @@ -536,10 +549,35 @@ function callHookFromMixins( function applyMixins( instance: ComponentInternalInstance, - mixins: ComponentOptions[] + mixins: ComponentOptions[], + deferredData: DataFn[], + deferredWatch: ComponentWatchOptions[] ) { for (let i = 0; i < mixins.length; i++) { - applyOptions(instance, mixins[i], true) + applyOptions(instance, mixins[i], deferredData, deferredWatch, true) + } +} + +function resolveData( + instance: ComponentInternalInstance, + dataFn: DataFn, + publicThis: ComponentPublicInstance +) { + const data = dataFn.call(publicThis, publicThis) + if (__DEV__ && isPromise(data)) { + warn( + `data() returned a Promise - note data() cannot be async; If you ` + + `intend to perform data fetching before component renders, use ` + + `async setup() + .` + ) + } + if (!isObject(data)) { + __DEV__ && warn(`data() should return an object.`) + } else if (instance.data === EMPTY_OBJ) { + instance.data = reactive(data) + } else { + // existing data: this is a mixin or extends. + extend(instance.data, data) } }