From 1574edd490bd5cc0a213bc9f48ff41a1dc43ab22 Mon Sep 17 00:00:00 2001 From: lidlanca <8693091+lidlanca@users.noreply.github.com> Date: Sun, 13 Feb 2022 20:40:12 -0500 Subject: [PATCH] fix(runtime-core): allow spying on proxy methods regression (#5417) fix #5415 (regression by #4216) --- .../__tests__/componentPublicInstance.spec.ts | 151 +++++++++++++++++- .../src/componentPublicInstance.ts | 5 +- 2 files changed, 151 insertions(+), 5 deletions(-) diff --git a/packages/runtime-core/__tests__/componentPublicInstance.spec.ts b/packages/runtime-core/__tests__/componentPublicInstance.spec.ts index fe588115d31..2bea4e0eedc 100644 --- a/packages/runtime-core/__tests__/componentPublicInstance.spec.ts +++ b/packages/runtime-core/__tests__/componentPublicInstance.spec.ts @@ -257,13 +257,17 @@ describe('component: proxy', () => { expect(instanceProxy.isDisplayed).toBe(true) }) - test('allow spying on proxy methods', () => { + + test('allow jest spying on proxy methods with Object.defineProperty', () => { + // #5417 let instanceProxy: any const Comp = { render() {}, setup() { return { - toggle() {} + toggle() { + return 'a' + } } }, mounted() { @@ -275,13 +279,154 @@ describe('component: proxy', () => { app.mount(nodeOps.createElement('div')) - const spy = jest.spyOn(instanceProxy, 'toggle') + // access 'toggle' to ensure key is cached + const v1 = instanceProxy.toggle() + expect(v1).toEqual('a') + + // reconfigure "toggle" to be getter based. + let getCalledTimes = 0 + Object.defineProperty(instanceProxy, 'toggle', { + get() { + getCalledTimes++ + return () => 'b' + } + }) + // getter should not be evaluated on initial definition + expect(getCalledTimes).toEqual(0) + + // invoke "toggle" after "defineProperty" + const v2 = instanceProxy.toggle() + expect(v2).toEqual('b') + expect(getCalledTimes).toEqual(1) + + // expect toggle getter not to be cached. it can't be instanceProxy.toggle() + expect(getCalledTimes).toEqual(2) + // attaching jest spy, triggers the getter once, cache it and override the property. + // also uses Object.defineProperty + const spy = jest.spyOn(instanceProxy, 'toggle') + expect(getCalledTimes).toEqual(3) + + // expect getter to not evaluate the jest spy caches its value + const v3 = instanceProxy.toggle() + expect(v3).toEqual('b') expect(spy).toHaveBeenCalled() + expect(getCalledTimes).toEqual(3) + }) + + test('defineProperty on proxy property with value descriptor', () => { + // #5417 + let instanceProxy: any + const Comp = { + render() {}, + setup() { + return { + toggle: 'a' + } + }, + mounted() { + instanceProxy = this + } + } + + const app = createApp(Comp) + + app.mount(nodeOps.createElement('div')) + + const v1 = instanceProxy.toggle + expect(v1).toEqual('a') + + Object.defineProperty(instanceProxy, 'toggle', { + value: 'b' + }) + const v2 = instanceProxy.toggle + expect(v2).toEqual('b') + + // expect null to be a settable value + Object.defineProperty(instanceProxy, 'toggle', { + value: null + }) + const v3 = instanceProxy.toggle + expect(v3).toBeNull() + }) + + test('defineProperty on public instance proxy should work with SETUP,DATA,CONTEXT,PROPS', () => { + // #5417 + let instanceProxy: any + const Comp = { + props: ['fromProp'], + data() { + return { name: 'data.name' } + }, + computed: { + greet() { + return 'Hi ' + (this as any).name + } + }, + render() {}, + setup() { + return { + fromSetup: true + } + }, + mounted() { + instanceProxy = this + } + } + + const app = createApp(Comp, { + fromProp: true + }) + + app.mount(nodeOps.createElement('div')) + expect(instanceProxy.greet).toEqual('Hi data.name') + + // define property on data + Object.defineProperty(instanceProxy, 'name', { + get() { + return 'getter.name' + } + }) + + // computed is same still cached + expect(instanceProxy.greet).toEqual('Hi data.name') + + // trigger computed + instanceProxy.name = '' + + // expect "greet" to evaluated and use name from context getter + expect(instanceProxy.greet).toEqual('Hi getter.name') + + // defineProperty on computed ( context ) + Object.defineProperty(instanceProxy, 'greet', { + get() { + return 'Hi greet.getter.computed' + } + }) + expect(instanceProxy.greet).toEqual('Hi greet.getter.computed') + + // defineProperty on setupState + expect(instanceProxy.fromSetup).toBe(true) + Object.defineProperty(instanceProxy, 'fromSetup', { + get() { + return false + } + }) + expect(instanceProxy.fromSetup).toBe(false) + + // defineProperty on Props + expect(instanceProxy.fromProp).toBe(true) + Object.defineProperty(instanceProxy, 'fromProp', { + get() { + return false + } + }) + expect(instanceProxy.fromProp).toBe(false) }) + // #864 test('should not warn declared but absent props', () => { const Comp = { diff --git a/packages/runtime-core/src/componentPublicInstance.ts b/packages/runtime-core/src/componentPublicInstance.ts index 0413730032c..9bfbfc10371 100644 --- a/packages/runtime-core/src/componentPublicInstance.ts +++ b/packages/runtime-core/src/componentPublicInstance.ts @@ -455,8 +455,9 @@ export const PublicInstanceProxyHandlers: ProxyHandler = { descriptor: PropertyDescriptor ) { if (descriptor.get != null) { - this.set!(target, key, descriptor.get(), null) - } else if (descriptor.value != null) { + // invalidate key cache of a getter based property #5417 + target.$.accessCache[key] = 0; + } else if (hasOwn(descriptor,'value')) { this.set!(target, key, descriptor.value, null) } return Reflect.defineProperty(target, key, descriptor)