Skip to content

Commit 045ab1f

Browse files
test(geometry): cover the nested-mutation case in a component
The deep copy added in 57b2827 was guarded by a single unit test, so a review reasonably suggested dropping it. Checking through the components shows it is load-bearing: mutating an `icons` entry in place and changing another option in the same render sends only the other option without it, and the icon change is lost. Mutating in place with no other change is a different matter, since `useMemoized` masks it before the hook runs. The doc comment and the unit test name both implied that case worked, so both now say what is actually guaranteed.
1 parent d1b1fb8 commit 045ab1f

3 files changed

Lines changed: 37 additions & 7 deletions

File tree

src/components/__tests__/polyline.test.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,33 @@ test('polyline should have a click listener', () => {
7474
);
7575
});
7676

77+
test('polyline should send a nested option mutated in place', () => {
78+
const path = [
79+
{lat: 1, lng: 2},
80+
{lat: 3, lng: 4}
81+
];
82+
const icons = [{offset: '50%'}];
83+
84+
const {rerender} = render(
85+
<Polyline path={path} icons={icons as never} strokeColor="#ff0000" />
86+
);
87+
88+
const polyline = mockInstances.get(google.maps.Polyline)[0];
89+
(polyline.setOptions as jest.Mock).mockClear();
90+
91+
// mutating in place keeps the same reference, so a shallow copy of the
92+
// tracked options would compare the array against itself and drop the change
93+
icons[0].offset = '75%';
94+
95+
rerender(
96+
<Polyline path={path} icons={icons as never} strokeColor="#00ff00" />
97+
);
98+
99+
const [sent] = (polyline.setOptions as jest.Mock).mock.calls[0];
100+
expect(Object.keys(sent).sort()).toEqual(['icons', 'strokeColor']);
101+
expect(sent.icons).toEqual([{offset: '75%'}]);
102+
});
103+
77104
test('polyline should only send the options that changed', () => {
78105
const path = [
79106
{lat: 1, lng: 2},

src/libraries/__tests__/get-changed-options.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('getChangedOptions', () => {
7777
});
7878

7979
describe('snapshotOptions', () => {
80-
test('copies nested values so later mutation is visible', () => {
80+
test('does not alias nested values from the source object', () => {
8181
const icons = [{offset: '50%'}];
8282
const tracked = snapshotOptions({icons});
8383

src/libraries/get-changed-options.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,16 @@ export function getChangedOptions<T extends object>(
6161
}
6262

6363
/**
64-
* Copies an options object deeply enough that a later comparison sees changes
65-
* made in place.
64+
* Copies an options object deeply enough that nested values are not aliased by
65+
* the caller's.
6666
*
67-
* A shallow copy aliases nested values, so mutating something like a polyline's
68-
* `icons` array and re-rendering compares the array against itself and reports
69-
* no change. Plain objects and arrays are copied; anything else, including maps
70-
* API instances, is kept by reference.
67+
* A shallow copy shares nested values with the props, so an `icons` array
68+
* mutated in place is compared against itself and reported unchanged. That only
69+
* matters once some other option also changes, since `useMemoized` otherwise
70+
* masks an in-place mutation before the hook runs at all.
71+
*
72+
* Plain objects and arrays are copied; anything else, including maps API
73+
* instances, is kept by reference.
7174
*
7275
* @internal
7376
*/

0 commit comments

Comments
 (0)