From 976a550f62972881dbada210af5f7dc185c252d4 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Fri, 18 Oct 2024 12:54:39 +0200 Subject: [PATCH] Interactivity API: Fix reactivity of undefined objects and arrays added with `deepMerge` (#66183) * Fix types in test * Add failing tests * Fix the bug * Update changelog * Fix changelog * Update tests Co-authored-by: DAreRodz Co-authored-by: michalczaplinski --- packages/interactivity/CHANGELOG.md | 4 ++ packages/interactivity/src/proxies/state.ts | 11 ++++- .../src/proxies/test/deep-merge.ts | 42 ++++++++++++++++++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index b5a8fbcb9dde5..73212578ac109 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### 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 diff --git a/packages/interactivity/src/proxies/state.ts b/packages/interactivity/src/proxies/state.ts index f63c84738bcb1..d4d573acba884 100644 --- a/packages/interactivity/src/proxies/state.ts +++ b/packages/interactivity/src/proxies/state.ts @@ -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 ] ) ) { @@ -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 + ); } } } diff --git a/packages/interactivity/src/proxies/test/deep-merge.ts b/packages/interactivity/src/proxies/test/deep-merge.ts index f475385a43787..267e4850f9af9 100644 --- a/packages/interactivity/src/proxies/test/deep-merge.ts +++ b/packages/interactivity/src/proxies/test/deep-merge.ts @@ -379,7 +379,10 @@ 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 ); @@ -387,7 +390,7 @@ describe( 'Interactivity API', () => { 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', () => { @@ -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', () => { @@ -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' ); + } ); } ); } );