Skip to content

Commit

Permalink
Interactivity API: Update iterable signals when deepMerge() adds ne…
Browse files Browse the repository at this point in the history
…w properties (#65135)

* Add failing test

* Fix `deepMerge` implementation

* Update changelog

* Move iterable signal updating outside `for...in` loop

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: michalczaplinski <czapla@git.wordpress.org>
Co-authored-by: cbravobernal <cbravobernal@git.wordpress.org>
  • Loading branch information
4 people authored Sep 9, 2024
1 parent 6108be0 commit a89c8bd
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
1 change: 1 addition & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug Fixes

- Prevent calling `proxifyContext` over an already-proxified context inside `wp-context` ([#65090](https://github.com/WordPress/gutenberg/pull/65090)).
- Update iterable signals when `deepMerge()` adds new properties ([#65135](https://github.com/WordPress/gutenberg/pull/65135)).

## 6.7.0 (2024-09-05)

Expand Down
14 changes: 11 additions & 3 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,17 @@ const deepMergeRecursive = (
override: boolean = true
) => {
if ( isPlainObject( target ) && isPlainObject( source ) ) {
let hasNewKeys = false;
for ( const key in source ) {
const isNew = ! ( key in target );
hasNewKeys = hasNewKeys || isNew;

const desc = Object.getOwnPropertyDescriptor( source, key );
if (
typeof desc?.get === 'function' ||
typeof desc?.set === 'function'
) {
if ( override || ! ( key in target ) ) {
if ( override || isNew ) {
Object.defineProperty( target, key, {
...desc,
configurable: true,
Expand All @@ -296,12 +300,12 @@ const deepMergeRecursive = (
}
}
} else if ( isPlainObject( source[ key ] ) ) {
if ( ! ( key in target ) ) {
if ( isNew ) {
target[ key ] = {};
}

deepMergeRecursive( target[ key ], source[ key ], override );
} else if ( override || ! ( key in target ) ) {
} else if ( override || isNew ) {
Object.defineProperty( target, key, desc! );

const proxy = getProxyFromObject( target );
Expand All @@ -311,6 +315,10 @@ const deepMergeRecursive = (
}
}
}

if ( hasNewKeys && objToIterable.has( target ) ) {
objToIterable.get( target )!.value++;
}
}
};

Expand Down
15 changes: 15 additions & 0 deletions packages/interactivity/src/proxies/test/deep-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,5 +374,20 @@ describe( 'Interactivity API', () => {

expect( spy ).toHaveBeenCalledTimes( 3 );
} );

it( 'should update iterable signals when new keys are added', () => {
const target = proxifyState< any >( 'test', { a: 1, b: 2 } );
const source = { a: 1, b: 2, c: 3 };

const spy = jest.fn( () => Object.keys( target ) );
effect( spy );

expect( spy ).toHaveBeenCalledTimes( 1 );

deepMerge( target, source, false );

expect( spy ).toHaveBeenCalledTimes( 2 );
expect( spy ).toHaveLastReturnedWith( [ 'a', 'b', 'c' ] );
} );
} );
} );

1 comment on commit a89c8bd

@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 a89c8bd.
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/10777720740
📝 Reported issues:

Please sign in to comment.