-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Freeze non top screens to prevent extra-rerenders 2 #85443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1f66947
611e70e
390daa4
2c92405
0da1bdd
e5450de
70063f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # `react-native-screens` patches | ||
|
|
||
| ### [react-native-screens+4.15.4+001+delay-freeze-until-paint.patch](react-native-screens+4.15.4+001+delay-freeze-until-paint.patch) | ||
|
|
||
| - Reason: Delays `DelayedFreeze` before freezing a screen. On iOS (Fabric), freezing a screen while an animation is still in flight (e.g. a popover closing or a swipe-back gesture finishing) blocks React rendering and can cause the app to become unresponsive. | ||
| - Upstream PR/issue: N/A | ||
| - E/App issue: https://github.com/Expensify/App/issues/33725 | ||
| - PR introducing patch: https://github.com/Expensify/App/pull/82764 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| diff --git a/node_modules/react-native-screens/src/components/helpers/DelayedFreeze.tsx b/node_modules/react-native-screens/src/components/helpers/DelayedFreeze.tsx | ||
| index 443da63..72517fd 100644 | ||
| --- a/node_modules/react-native-screens/src/components/helpers/DelayedFreeze.tsx | ||
| +++ b/node_modules/react-native-screens/src/components/helpers/DelayedFreeze.tsx | ||
| @@ -7,17 +7,27 @@ interface FreezeWrapperProps { | ||
| children: React.ReactNode; | ||
| } | ||
|
|
||
| +// Delay freezing long enough for screen transition and modal/popover dismiss | ||
| +// animations to complete. On iOS, freezing a screen while an animation | ||
| +// is still in flight (e.g. a popover closing or a swipe-back gesture finishing) | ||
| +// blocks React rendering and can cause the app to become unresponsive. | ||
| +const FREEZE_DELAY_MS = 400; | ||
| + | ||
| // This component allows one more render before freezing the screen. | ||
| // Allows activityState to reach the native side and useIsFocused to work correctly. | ||
| function DelayedFreeze({ freeze, children }: FreezeWrapperProps) { | ||
| // flag used for determining whether freeze should be enabled | ||
| const [freezeState, setFreezeState] = React.useState(false); | ||
|
|
||
| - React.useEffect(() => { | ||
| - const id = setTimeout(() => { | ||
| - setFreezeState(freeze); | ||
| - }, 0); | ||
| + React.useEffect(() => { | ||
| + let rafID: number; | ||
| + // Wait for FREEZE_DELAY_MS, then schedule the state update after the next | ||
| + // frame paint so transitional UI states are fully flushed before suspending. | ||
| + const id = setTimeout(() => { | ||
| + rafID = requestAnimationFrame(() => setFreezeState(freeze)); | ||
| + }, FREEZE_DELAY_MS); | ||
| return () => { | ||
| clearTimeout(id); | ||
| + cancelAnimationFrame(rafID); | ||
| }; | ||
| }, [freeze]); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| diff --git a/node_modules/@react-navigation/native-stack/lib/module/views/NativeStackView.native.js b/node_modules/@react-navigation/native-stack/lib/module/views/NativeStackView.native.js | ||
| index 35dfc05..1758a24 100644 | ||
| --- a/node_modules/@react-navigation/native-stack/lib/module/views/NativeStackView.native.js | ||
| +++ b/node_modules/@react-navigation/native-stack/lib/module/views/NativeStackView.native.js | ||
| @@ -376,9 +376,9 @@ export function NativeStackView({ | ||
| const isModal = modalRouteKeys.includes(route.key); | ||
| const isPreloaded = preloadedDescriptors[route.key] !== undefined && descriptors[route.key] === undefined; | ||
|
|
||
| - // On Fabric, when screen is frozen, animated and reanimated values are not updated | ||
| - // due to component being unmounted. To avoid this, we don't freeze the previous screen there | ||
| - const shouldFreeze = isFabric() ? !isPreloaded && !isFocused && !isBelowFocused : !isPreloaded && !isFocused; | ||
| + // Freezing the screen below the focused one is safe on Fabric because | ||
| + // DelayedFreeze defers it to the next macrotask, and transition animations run on the UI thread. | ||
| + const shouldFreeze = !isPreloaded && !isFocused; | ||
| return /*#__PURE__*/_jsx(SceneView, { | ||
| index: index, | ||
| focused: isFocused, | ||
| diff --git a/node_modules/@react-navigation/native-stack/src/views/NativeStackView.native.tsx b/node_modules/@react-navigation/native-stack/src/views/NativeStackView.native.tsx | ||
| index ca15b1d..3191475 100644 | ||
| --- a/node_modules/@react-navigation/native-stack/src/views/NativeStackView.native.tsx | ||
| +++ b/node_modules/@react-navigation/native-stack/src/views/NativeStackView.native.tsx | ||
| @@ -542,11 +542,9 @@ export function NativeStackView({ | ||
| preloadedDescriptors[route.key] !== undefined && | ||
| descriptors[route.key] === undefined; | ||
|
|
||
| - // On Fabric, when screen is frozen, animated and reanimated values are not updated | ||
| - // due to component being unmounted. To avoid this, we don't freeze the previous screen there | ||
| - const shouldFreeze = isFabric() | ||
| - ? !isPreloaded && !isFocused && !isBelowFocused | ||
| - : !isPreloaded && !isFocused; | ||
| + // Freezing the screen below the focused one is safe on Fabric because | ||
| + // DelayedFreeze defers it to the next macrotask, and transition animations run on the UI thread. | ||
| + const shouldFreeze = !isPreloaded && !isFocused; | ||
|
|
||
| return ( | ||
| <SceneView |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import {createContext, useContext} from 'react'; | ||
|
|
||
| type ScreenFreezeContextType = { | ||
| /** | ||
| * Register a freeze defer — signals that this component has cleanup work | ||
| * (e.g., keyboard shortcut unsubscribing) that must run before the screen is frozen. | ||
| * When any defers are registered, ScreenFreezeWrapper delays freezing by one frame | ||
| * so cleanup can execute first. | ||
| * Returns an unregister function. | ||
| */ | ||
| registerFreezeDefer: () => () => void; | ||
| }; | ||
|
|
||
| const ScreenFreezeContext = createContext<ScreenFreezeContextType>({ | ||
| registerFreezeDefer: () => () => {}, | ||
| }); | ||
|
|
||
| function useScreenFreezeContext() { | ||
| return useContext(ScreenFreezeContext); | ||
| } | ||
|
|
||
| export default ScreenFreezeContext; | ||
| export {useScreenFreezeContext}; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import React, {useLayoutEffect, useRef, useState} from 'react'; | ||
| import {Freeze} from 'react-freeze'; | ||
| import TooltipSense from '@components/Tooltip/TooltipSense'; | ||
| import {areAllModalsHidden} from '@userActions/Modal'; | ||
| import ScreenFreezeContext from './ScreenFreezeContext'; | ||
|
|
||
| type ScreenFreezeWrapperProps = { | ||
| /** Whether the screen is not currently visible to the user */ | ||
| isScreenBlurred: boolean; | ||
|
|
||
| /** The screen content to freeze when blurred */ | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| function ScreenFreezeWrapper({isScreenBlurred, children}: ScreenFreezeWrapperProps) { | ||
| const [frozen, setFrozen] = useState(false); | ||
| const freezeDeferCountRef = useRef(0); | ||
|
|
||
| const registerFreezeDefer = () => { | ||
| freezeDeferCountRef.current++; | ||
| return () => { | ||
| freezeDeferCountRef.current--; | ||
| }; | ||
| }; | ||
|
|
||
| const contextValue = {registerFreezeDefer}; | ||
|
|
||
| // Decouple the Suspense render task so it won't be interrupted by React's concurrent mode | ||
| // and stuck in an infinite loop | ||
| useLayoutEffect(() => { | ||
| // When unfreezing, always apply immediately so the screen is visible right away. | ||
| if (!isScreenBlurred) { | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-5 (docs)The Add a comment directly explaining why the rule is safe to disable, for example: // eslint-disable-next-line react-hooks/set-state-in-effect -- Synchronous unfreeze is intentional; the early return prevents infinite loops since isScreenBlurred is the only dependency and won't change as a result of this setState.
setFrozen(false);Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency. |
||
| setFrozen(false); | ||
| return; | ||
| } | ||
|
|
||
| // When there are active freeze defers (e.g. keyboard shortcuts that need to unsubscribe), | ||
| // or when a modal/tooltip is still open, defer the freezing by one frame. | ||
| if (freezeDeferCountRef.current > 0 || TooltipSense.isActive() || !areAllModalsHidden()) { | ||
| const id = requestAnimationFrame(() => setFrozen(isScreenBlurred)); | ||
| return () => { | ||
| cancelAnimationFrame(id); | ||
| }; | ||
| } | ||
|
|
||
| // No blockers or overlays — freeze immediately. | ||
| // eslint-disable-next-line react-hooks/set-state-in-effect | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-5 (docs)The Add a comment directly explaining why the rule is safe to disable, for example: // eslint-disable-next-line react-hooks/set-state-in-effect -- Synchronous freeze is safe; isScreenBlurred is the only dependency and setFrozen(true) won't trigger further isScreenBlurred changes.
setFrozen(isScreenBlurred);Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency. |
||
| setFrozen(isScreenBlurred); | ||
| }, [isScreenBlurred]); | ||
|
|
||
| return ( | ||
| <ScreenFreezeContext.Provider value={contextValue}> | ||
| <Freeze freeze={frozen}>{children}</Freeze> | ||
| </ScreenFreezeContext.Provider> | ||
| ); | ||
| } | ||
|
|
||
| export default ScreenFreezeWrapper; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effect now waits
FREEZE_DELAY_MSfor allfreezetransitions, not just when entering the frozen state. When a route becomes focused again (freezeflipstrue -> false),setFreezeState(false)is delayed by ~400ms, so the newly focused screen can remain suspended and miss immediate UI updates/taps after back/pop navigation. The delay should only apply whenfreezeistrue, while unfreezing should happen immediately.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
DelayedFreezeif freeze is false it always applied right away, https://github.com/software-mansion/react-native-screens/blob/63b3baab65a1fd36da04ae426f98ad460217e1e0/src/components/helpers/DelayedFreeze.tsx#L24