Skip to content

Commit

Permalink
iAPI: Fix the logic path that merges plain objects (#68579)
Browse files Browse the repository at this point in the history
* Fix the logic path that merges plain objects

* Add changelog

---------

Co-authored-by: DAreRodz <[email protected]>
Co-authored-by: luisherranz <[email protected]>
Co-authored-by: priethor <[email protected]>
  • Loading branch information
4 people authored Jan 9, 2025
1 parent 98cb5d6 commit be95ec3
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
4 changes: 4 additions & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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 );
}

Expand Down
32 changes: 32 additions & 0 deletions packages/interactivity/src/proxies/test/deep-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {} );

Expand Down

1 comment on commit be95ec3

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in be95ec3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12697207474
📝 Reported issues:

Please sign in to comment.