From cf1f5517bacf7a527c9bebc9dd393ee8eac0099c Mon Sep 17 00:00:00 2001 From: David Date: Thu, 9 Jan 2025 18:20:31 +0100 Subject: [PATCH] Fix the logic path that merges plain objects --- packages/interactivity/src/proxies/state.ts | 7 ++-- .../src/proxies/test/deep-merge.ts | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/src/proxies/state.ts b/packages/interactivity/src/proxies/state.ts index e86ec05c48461..08977e8e18efa 100644 --- a/packages/interactivity/src/proxies/state.ts +++ b/packages/interactivity/src/proxies/state.ts @@ -340,7 +340,9 @@ const deepMergeRecursive = ( // Handle nested objects } else if ( isPlainObject( source[ key ] ) ) { - if ( isNew || ( override && ! isPlainObject( target[ key ] ) ) ) { + const targetValue = Object.getOwnPropertyDescriptor( target, key ) + ?.value; + if ( isNew || ( override && ! isPlainObject( targetValue ) ) ) { // Create a new object if the property is new or needs to be overridden target[ key ] = {}; if ( propSignal ) { @@ -350,9 +352,10 @@ const deepMergeRecursive = ( proxifyState( ns, target[ key ] as Object ) ); } + deepMergeRecursive( target[ key ], source[ key ], override ); } // Both target and source are plain objects, merge them recursively - if ( isPlainObject( target[ key ] ) ) { + else if ( isPlainObject( targetValue ) ) { deepMergeRecursive( target[ key ], source[ key ], override ); } diff --git a/packages/interactivity/src/proxies/test/deep-merge.ts b/packages/interactivity/src/proxies/test/deep-merge.ts index aaa762cb979f3..c1e32763a01ef 100644 --- a/packages/interactivity/src/proxies/test/deep-merge.ts +++ b/packages/interactivity/src/proxies/test/deep-merge.ts @@ -455,6 +455,38 @@ describe( 'Interactivity API', () => { expect( target.message.fontStyle ).toBeUndefined(); } ); + it( 'should not overwrite getters that become objects if `override` is false', () => { + const target: any = proxifyState( 'test', { + get message() { + return 'hello'; + }, + } ); + + const getterSpy = jest.spyOn( target, 'message', 'get' ); + + let message: any; + const spy = jest.fn( () => ( message = target.message ) ); + effect( spy ); + + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( message ).toBe( 'hello' ); + + deepMerge( + target, + { message: { content: 'hello', fontStyle: 'italic' } }, + false + ); + + // The effect callback reads `target.message`, so the getter is executed once as well. + expect( spy ).toHaveBeenCalledTimes( 1 ); + expect( getterSpy ).toHaveBeenCalledTimes( 1 ); + + expect( message ).toBe( 'hello' ); + expect( target.message ).toBe( 'hello' ); + expect( target.message.content ).toBeUndefined(); + expect( target.message.fontStyle ).toBeUndefined(); + } ); + it( 'should keep reactivity of arrays that are initially undefined', () => { const target: any = proxifyState( 'test', {} );