Skip to content

Commit

Permalink
Interactivity API: Fix reactivity of undefined objects and arrays add…
Browse files Browse the repository at this point in the history
…ed with `deepMerge` (#66183) (#66236)

* Fix types in test

* Add failing tests

* Fix the bug

* Update changelog

* Fix changelog

* Update tests

Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Co-authored-by: michalczaplinski <czapla@git.wordpress.org>
  • Loading branch information
3 people authored Oct 18, 2024
1 parent 64f9146 commit 737fac0
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 4 deletions.
12 changes: 12 additions & 0 deletions packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,21 @@

### Bug Fixes

- Fix reactivity of undefined objects and arrays added with `deepMerge()` ([#66183](https://github.com/WordPress/gutenberg/pull/66183)).

## 6.10.0 (2024-10-16)

### Internal

- Upgrade preact libraries [#66008](https://github.com/WordPress/gutenberg/pull/66008).

### Bug Fixes

- Fix an issue where "default" could not be used as a directive suffix ([#65815](https://github.com/WordPress/gutenberg/pull/65815)).
- Correctly handle lazily added, deeply nested properties with `deepMerge()` ([#65465](https://github.com/WordPress/gutenberg/pull/65465)).

## 6.9.0 (2024-10-03)

## 6.8.0 (2024-09-19)

### Enhancements
Expand Down
11 changes: 9 additions & 2 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,10 @@ const deepMergeRecursive = (
if ( isNew || ( override && ! isPlainObject( target[ key ] ) ) ) {
target[ key ] = {};
if ( propSignal ) {
propSignal.setValue( target[ key ] );
const ns = getNamespaceFromProxy( proxy );
propSignal.setValue(
proxifyState( ns, target[ key ] as Object )
);
}
}
if ( isPlainObject( target[ key ] ) ) {
Expand All @@ -344,7 +347,11 @@ const deepMergeRecursive = (
} else if ( override || isNew ) {
Object.defineProperty( target, key, desc );
if ( propSignal ) {
propSignal.setValue( desc.value );
const { value } = desc;
const ns = getNamespaceFromProxy( proxy );
propSignal.setValue(
shouldProxy( value ) ? proxifyState( ns, value ) : value
);
}
}
}
Expand Down
42 changes: 40 additions & 2 deletions packages/interactivity/src/proxies/test/deep-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,15 +379,18 @@ describe( 'Interactivity API', () => {
const target = proxifyState< any >( 'test', { a: 1, b: 2 } );
const source = { a: 1, b: 2, c: 3 };

const spy = jest.fn( () => Object.keys( target ) );
let keys: any;
const spy = jest.fn( () => {
keys = Object.keys( target );
} );
effect( spy );

expect( spy ).toHaveBeenCalledTimes( 1 );

deepMerge( target, source, false );

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

it( 'should handle deeply nested properties that are initially undefined', () => {
Expand All @@ -412,6 +415,13 @@ describe( 'Interactivity API', () => {

// Reading the value directly should also work
expect( target.a.b.c.d ).toBe( 'test value' );

// Modify the nested value
target.a.b.c.d = 'new test value';

// The effect should be called again
expect( spy ).toHaveBeenCalledTimes( 3 );
expect( deepValue ).toBe( 'new test value' );
} );

it( 'should overwrite values that become objects', () => {
Expand Down Expand Up @@ -462,5 +472,33 @@ describe( 'Interactivity API', () => {
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', {} );

let deepValue: any;
const spy = jest.fn( () => {
deepValue = target.array?.[ 0 ];
} );
effect( spy );

// Initial call, the deep value is undefined
expect( spy ).toHaveBeenCalledTimes( 1 );
expect( deepValue ).toBeUndefined();

// Use deepMerge to add an array to the target
deepMerge( target, { array: [ 'value 1' ] } );

// The effect should be called again
expect( spy ).toHaveBeenCalledTimes( 2 );
expect( deepValue ).toBe( 'value 1' );

// Modify the array value
target.array[ 0 ] = 'value 2';

// The effect should be called again
expect( spy ).toHaveBeenCalledTimes( 3 );
expect( deepValue ).toBe( 'value 2' );
} );
} );
} );

1 comment on commit 737fac0

@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 737fac0.
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/11405005859
📝 Reported issues:

Please sign in to comment.