feat: disable freeze for first render #1220
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Krzysztof Magiera <[email protected]>
7 tasks
7 tasks
WoLewicki
approved these changes
Dec 1, 2021
kacperkapusciak
deleted the
@kacperkapusciak/disable-freeze-for-first-render
branch
|
@kacperkapusciak I don't see the code from this merge in the current version: Or am I missing something? I have version 3.22.0 which from June 2023 and I needed to use patch package and add the fix from here to get it to work: I can see 3.27 still has the old code. |
2 tasks
kligarski
added a commit
that referenced
this pull request
Jun 9, 2025
…ite loop (#2963) ## Description When analyzing Test791 in order to create an e2e test for it, I noticed some weird behavior that is probably caused by multiple overlapping issues. This PR addresses one of them - when pushing modals with a header in `setInterval` from a regular push screen that is not freezed (we prevent freezing it by using `freezeOnBlur: false` option), only 4 of them are pushed. Then the interval stops and the screens are not being pushed anymore. What is even more interesting, interacting with a button in the modal fixes the issue, sometimes only for one more push, but sometimes it just starts working without any issues. DelayedFreeze component was introduced in #1220 in order to fix react-navigation's `useIsFocused` hook and some animation bugs. DelayedFreeze allows one more render before freezing the screen. If we change `freeze` to `true`, we still pass previous freezeState (`false`) to the `Freeze` component. `useEffect` runs and uses `setImmediate` to update the `freezeState` right after the render. This mechanism works for regular `push`/`card` screens but for some unknown reason, it does not work with multiple screens with `presentation: 'modal'` and `headerShown: true` (more details below). Changing `setImmediate` to `setInterval(fn, 0)` fixes the issue. **EDIT:** Seems like the issue happens also in different scenarios with freeze (e.g. #2971). The bug is present only for RN 0.78+, it might be related to React 19. Fixes #2971. ### Details `setImmediate` creates a [microtask](https://github.com/facebook/react-native/blob/v0.79.3/packages/react-native/Libraries/Core/Timers/immediateShim.js#L44). Microtasks run after a macrotask, before another interation of the event loop. `setTimeout` is a regular macrotask. Somehow, when using `setImmediate` in `DelayedFreeze`, after pushing 4 `modal` screens with a header, when the first modal gets freezed, an infinite loop of commits is created (you can see the CPU usage go to max, allocated memory increases). Even though each commit has multiple `affectedLayoutableNodes`, they don't result in any mutations when diffing the trees. These updates probably result in starvation of the function passed to `setInterval`. This bug happens with `presentation: 'formSheet'` as well but **not** for other types of modals **including `presentation: 'pageSheet'`!** This suggests that there is a problem with `UIModalPresentationFormSheet` (to which `UIModalPresentationAutomatic` usually resolves to since iOS 18) but not `UIModalPresentationPageSheet` (which was the default before iOS 18). #### Bug mechanism described step-by-step Please note that react-navigation freezes currently focused screen and one below (this is necessary for animations). ##### Step 1 |Screen|Presentation|Freeze|Details| |------|------------|------|-------| |Home |push |`false`| We enforce no freeze with `freezeOnBlur: false`| ##### Step 2 |Screen|Presentation|Freeze|Details| |------|------------|------|-------| |Home |push |`false`| We enforce no freeze with `freezeOnBlur: false`| |Second|modal|`false`|The modal `isFocused` so we don't freeze it ##### Step 3 |Screen|Presentation|Freeze|Details| |------|------------|------|-------| |Home |push |`false`| We enforce no freeze with `freezeOnBlur: false`| |Modal1|modal|`false`|The modal `isBelowFocused` so we don't freeze it |Modal2|modal|`false`|The modal `isFocused` so we don't freeze it ##### Step 4 |Screen|Presentation|Freeze|Details| |------|------------|------|-------| |Home |push |`false`| We enforce no freeze with `freezeOnBlur: false`| |Modal1|modal|`false`|The modal will be freezed after next render (because of `DelayedFreeze`) |Modal2|modal|`false`|The modal `isBelowFocused` so we don't freeze it |Modal3|modal|`false`|The modal `isFocused` so we don't freeze it ##### Step 5 |Screen|Presentation|Freeze|Details| |------|------------|------|-------| |Home |push |`false`| We enforce no freeze with `freezeOnBlur: false`| |Modal1|modal|`true`| |Modal2|modal|`false`|The modal will be freezed after next render (because of `DelayedFreeze`) |Modal3|modal|`false`|The modal `isBelowFocused` so we don't freeze it |Modal4|modal|`false`|The modal `isFocused` so we don't freeze it This is the moment when infinite loop happens. ## Changes - change `setImmediate` to `setTimeout(fn, 0)` in `DelayedFreeze` - add `Test2963` test screen ## Screenshots / GIFs ### Before https://github.com/user-attachments/assets/c571e25a-d17f-4ae9-8bb6-06270ba6e1e9 ### After https://github.com/user-attachments/assets/ebb461f5-9eef-4b50-8f41-473a38232cfd ## Test code and steps to reproduce Open `Test2963` in example app. ## Checklist - [x] Included code example that can be used to test this change - [ ] Ensured that CI passes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
React-navigation's
useIsFocusedworks based on changes between renders. Freezing stops all renders on non-visible screens thus breaking theuseIsFocusedhook.This issue can be mitigated by allowing one more render before freezing the screen / disabling freeze for the first render.
This also fixes unnecessary pop animation on on stack reset.
Fixes #1198, #1209
Changes
DelayedFreezecomponent inindex.native.tsxScreenshots / GIFs
#1198
Screen.Recording.2021-11-24.at.09.26.50.mov
#1209
Before
Simulator.Screen.Recording.-.iPhone.11.-.2021-11-24.at.10.59.38.mp4
After
Simulator.Screen.Recording.-.iPhone.11.-.2021-11-24.at.10.59.02.mp4
Test code and steps to reproduce
Test1198.tsx&Test1209.tsxin TestsExample/ projectChecklist