Skip to content

Commit

Permalink
Fix the logic path that merges plain objects
Browse files Browse the repository at this point in the history
  • Loading branch information
luisherranz committed Jan 16, 2025
1 parent e5ef7c6 commit 8efefe5
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,16 +332,21 @@ const deepMergeRecursive = (
}
}
} 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 ) {
const ns = getNamespaceFromProxy( proxy );
propSignal.setValue(
proxifyState( ns, target[ key ] as Object )
);
}
deepMergeRecursive( target[ key ], source[ key ], override );
}
if ( isPlainObject( target[ key ] ) ) {
// Both target and source are plain objects, merge them recursively
else if ( isPlainObject( targetValue ) ) {
deepMergeRecursive( target[ key ], source[ key ], override );
}
} else if ( override || isNew ) {
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 @@ -473,6 +473,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

0 comments on commit 8efefe5

Please sign in to comment.