Skip to content

Commit

Permalink
Complete the partial memoization of provider (#427)
Browse files Browse the repository at this point in the history
* Make onInsetsChange more referentially stable

It should only change reference if setFrame or setInsets change, which
should be never.

Related: #426

* Make style passed to provider stable

Related: #426

* Memoize provider

Related: #426

* Only add displayName to provider if in dev env

* Revert "Only add displayName to provider if in dev env"

This reverts commit 23fc49e.

* Revert "Memoize provider"

This reverts commit 8f34f22.

* Move to StyleSheet.compose

Related: #426

* Remove stable references from onInsetsChange deps

* Update snapshot tests
  • Loading branch information
Noitidart committed Sep 1, 2023
1 parent 930f703 commit c862c86
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 89 deletions.
27 changes: 16 additions & 11 deletions src/SafeAreaContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ export function SafeAreaProvider({
height: Dimensions.get('window').height,
},
);
const onInsetsChange = React.useCallback(
(event: InsetChangedEvent) => {
const {
nativeEvent: { frame: nextFrame, insets: nextInsets },
} = event;
const onInsetsChange = React.useCallback((event: InsetChangedEvent) => {
const {
nativeEvent: { frame: nextFrame, insets: nextInsets },
} = event;

setFrame((frame) => {

Check warning on line 61 in src/SafeAreaContext.tsx

View workflow job for this annotation

GitHub Actions / js-tests

'frame' is already declared in the upper scope on line 46 column 10
if (
// Backwards compat with old native code that won't send frame.
nextFrame &&
Expand All @@ -67,25 +67,30 @@ export function SafeAreaProvider({
nextFrame.x !== frame.x ||
nextFrame.y !== frame.y)
) {
setFrame(nextFrame);
return nextFrame;
} else {
return frame;
}
});

setInsets((insets) => {

Check warning on line 76 in src/SafeAreaContext.tsx

View workflow job for this annotation

GitHub Actions / js-tests

'insets' is already declared in the upper scope on line 43 column 10
if (
!insets ||
nextInsets.bottom !== insets.bottom ||
nextInsets.left !== insets.left ||
nextInsets.right !== insets.right ||
nextInsets.top !== insets.top
) {
setInsets(nextInsets);
return nextInsets;
} else {
return insets;
}
},
[frame, insets],
);
});
}, []);

return (
<NativeSafeAreaProvider
style={[styles.fill, style]}
style={StyleSheet.compose(styles.fill, style)}
onInsetsChange={onInsetsChange}
{...others}
>
Expand Down
81 changes: 27 additions & 54 deletions src/__tests__/__snapshots__/SafeAreaContext-test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ exports[`SafeAreaContext does not render child until inset values are received 1
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
/>
`;
Expand All @@ -18,12 +15,9 @@ exports[`SafeAreaContext renders 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
/>
`;
Expand All @@ -32,12 +26,9 @@ exports[`SafeAreaContext renders child when inset values are received 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
/>
`;
Expand All @@ -46,12 +37,9 @@ exports[`SafeAreaContext renders child when inset values are received 2`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<View
Expand All @@ -75,12 +63,9 @@ exports[`SafeAreaContext supports setting initial insets 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<View
Expand All @@ -104,23 +89,17 @@ exports[`SafeAreaContext uses inner insets 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<View
Expand All @@ -145,23 +124,17 @@ exports[`SafeAreaContext uses parent insets when available 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<View
Expand Down
36 changes: 12 additions & 24 deletions src/__tests__/__snapshots__/SafeAreaView-test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ exports[`SafeAreaView can override padding styles 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaView
Expand All @@ -36,12 +33,9 @@ exports[`SafeAreaView can set edges 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaView
Expand All @@ -63,12 +57,9 @@ exports[`SafeAreaView can set edges value type 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaView
Expand Down Expand Up @@ -96,12 +87,9 @@ exports[`SafeAreaView renders 1`] = `
<RNCSafeAreaProvider
onInsetsChange={[Function]}
style={
[
{
"flex": 1,
},
undefined,
]
{
"flex": 1,
}
}
>
<RNCSafeAreaView
Expand Down

0 comments on commit c862c86

Please sign in to comment.