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)

* 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 committed Oct 18, 2024
1 parent 55f9895 commit 585de28
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' );
} );
} );
} );

0 comments on commit 585de28

Please sign in to comment.