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
DAreRodz committed Jan 9, 2025
1 parent e746a95 commit cf1f551
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
7 changes: 5 additions & 2 deletions packages/interactivity/src/proxies/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,9 @@ const deepMergeRecursive = (

// Handle nested objects
} 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 ) {
Expand All @@ -350,9 +352,10 @@ const deepMergeRecursive = (
proxifyState( ns, target[ key ] as Object )
);
}
deepMergeRecursive( target[ key ], source[ key ], override );
}
// Both target and source are plain objects, merge them recursively
if ( isPlainObject( target[ key ] ) ) {
else if ( isPlainObject( targetValue ) ) {
deepMergeRecursive( target[ key ], source[ key ], override );
}

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 @@ -455,6 +455,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 cf1f551

Please sign in to comment.