Skip to content

Commit 2bf0958

Browse files
authored
fix: static container memo check (react-navigation#6825)
Memo check compared elements of prevProps to nextProps but failed to take new props into account. Fixed the logic and added a new test.
1 parent 94cff23 commit 2bf0958

File tree

2 files changed

+34
-3
lines changed

2 files changed

+34
-3
lines changed

packages/core/src/StaticContainer.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,19 @@ function StaticContainer(props: any) {
88
}
99

1010
export default React.memo(StaticContainer, (prevProps: any, nextProps: any) => {
11-
for (const prop in prevProps) {
12-
if (prop === 'children') {
11+
const prevPropKeys = Object.keys(prevProps);
12+
const nextPropKeys = Object.keys(nextProps);
13+
14+
if (prevPropKeys.length !== nextPropKeys.length) {
15+
return false;
16+
}
17+
18+
for (const key of prevPropKeys) {
19+
if (key === 'children') {
1320
continue;
1421
}
1522

16-
if (prevProps[prop] !== nextProps[prop]) {
23+
if (prevProps[key] !== nextProps[key]) {
1724
return false;
1825
}
1926
}

packages/core/src/__tests__/StaticContainer.test.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,27 @@ it('updates element if any props changed', () => {
4949

5050
expect(root).toMatchInlineSnapshot(`"second"`);
5151
});
52+
53+
it('updates element if any props are added', () => {
54+
expect.assertions(2);
55+
56+
const Test = ({ label }: any) => {
57+
return label;
58+
};
59+
60+
const root = render(
61+
<StaticContainer count={42}>
62+
<Test label="first" />
63+
</StaticContainer>
64+
);
65+
66+
expect(root).toMatchInlineSnapshot(`"first"`);
67+
68+
root.update(
69+
<StaticContainer count={42} moreCounts={12}>
70+
<Test label="second" />
71+
</StaticContainer>
72+
);
73+
74+
expect(root).toMatchInlineSnapshot(`"second"`);
75+
});

0 commit comments

Comments
 (0)