From be95ec381f847b1ca1adb96527966bc10b84551d Mon Sep 17 00:00:00 2001 From: Luis Herranz Date: Thu, 9 Jan 2025 20:33:31 +0100 Subject: [PATCH] iAPI: Fix the logic path that merges plain objects (#68579) * Fix the logic path that merges plain objects * Add changelog --------- Co-authored-by: DAreRodz Co-authored-by: luisherranz Co-authored-by: priethor --- packages/interactivity/CHANGELOG.md | 4 +++ packages/interactivity/src/proxies/state.ts | 7 ++-- .../src/proxies/test/deep-merge.ts | 32 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 818a16b8dd5e6..cdc44dca80741 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Bug Fixes + +- Fix the logic path that merges plain objects ([#68579](https://github.com/WordPress/gutenberg/pull/68579)). + ## 6.15.0 (2025-01-02) ### Enhancements 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', {} );