enableFreeze cause useIsFocused stop working #1198

r0b0t3d · 2021-11-02T04:43:46Z

Description

In bottom tabs, when enabling enableFreeze, useIsFocused is not triggered when screen blur.

Screenshots

Steps To Reproduce

Create tabs with 2 screens Home and Settings
In Settings, add const isFocused = useIsFocused()
Navigate to Settings screen, isFocused updated to true (this is correct)
Navigate back to Home, isFocused still true

Expected behavior

At step 4, isFocused in Settings should be false

Actual behavior

At step 4, isFocused in Settings is true

Reproduction

Platform

I just checked on iOS, not sure about other platforms

Workflow

Managed workflow
Bare workflow

Package versions

package	version
react-native	0.66.1
@react-navigation/native	6.0.6
@react-navigation/native-stack	6.2.5
@react-navigation/bottom-tabs	6.0.9
react-native-screens	3.9.0

The text was updated successfully, but these errors were encountered:

marhaupe · 2021-11-02T11:02:24Z

I can confirm this behaviour, but I think it's expected, isn't it? When screens freeze I assume them to not to render at all which actually makes calling useIsFocused redundant. This is just an educated guess though, confirmation from maintainers would be appreciated.

r0b0t3d · 2021-11-02T11:09:40Z

Currently, I'd like to refresh the content when a tab is focused. So I need useIsFocused to trigger the request.

Maybe you're right @marhaupe . That mean useIsFocused should be updated before the screen get frozen

tomasmozeris · 2021-11-04T12:30:49Z

Thanks for effort to make this feature. Same issue here, I use useIsFocused for periodical fetches when screen is in focus, I tried using useFocusEffect, but it doesn't catch unfocused state.

nandorojo · 2021-11-04T23:44:48Z

Isn't this the point of freezing the screen? You disable rendering when it's not focused.

marhaupe · 2021-11-08T14:33:08Z

After using it for some time we found some edge cases in our app where we rely on useIsFocused or useFocusEffect to refresh some data when the screen focuses (similar to what @tomasmozeris's case). This logic breaks when freezing screens. I would be open to suggestions on how to work around those constraints, but if there aren't any, I feel like there is still work to be done before this feature can be safely used in most apps.

nandorojo · 2021-11-08T15:20:04Z

Can you clarify the issues? Does it not trigger when you refocus?

tomasmozeris · 2021-11-08T15:37:52Z

Update the data then user refocus the screen:

const isFocused = useIsFocused();  
useEffect(() => {
  if (isFocused) {
    fetch();  
  }  
}, [isFocused])

Another case is I'm doing periodical fetches each minute, I'm stopping them then useIsFocused() === false, now if never reaches false state

nandorojo · 2021-11-08T15:58:33Z

That’s interesting. I wonder if that use case would be better solved with navigation.isFocused() in your fetcher anyway, so that you don’t have to trigger more renders. Does that function also not work with freeze?

Titozzz · 2021-11-08T16:24:40Z

Update the data then user refocus the screen:
const isFocused = useIsFocused();  
useEffect(() => {
  if (isFocused) {
    fetch();  
  }  
}, [isFocused])
Another case is I'm doing periodical fetches each minute, I'm stopping them then useIsFocused() === false, now if never reaches false state

I have the exact same pattern in my app. I'd expect the react state to still be updated but just not render/update views.
If all the hooks are stopped too thats way more complex 👀

kacperkapusciak · 2021-11-24T09:06:02Z

Hi @Titozzz, @r0b0t3d, @nandorojo, @marhaupe, and @tomasmozeris

Thank you so much for participating in this discussion. It helped a lot!

I think we've finally figured out how to overcome this issue and the solution is quite simple. We disable freeze for the first render so the useIsFocused can correctly pick up that screens have changed.

Could you guys confirm the solution works by testing the code in your app eg. by installing react-native-screens directly from the branch

yarn add git+https://github.com/software-mansion/react-native-screens#@kacperkapusciak/disable-freeze-for-first-render

or swapping the whole index.native.tsx file in your node_modules/react-native-screens?

That would be amazing! 🙌
Thanks!

marhaupe · 2021-11-25T17:01:09Z

This looks great! I can confirm that our logic that e.g. unmounts a react-native-camera instance when the screen is not focused, actually unmounts the camera instead of freezing it. On a side note: This is actually a case that would break too. Usually you have to reset the camera when navigating (or at least turn of things like the flash before you do), which wouldn't be possible without this fix.

r0b0t3d · 2021-11-26T02:42:54Z

@kacperkapusciak it works for me too 🚀. Thanks

Mohamed-kassim · 2022-01-27T13:20:38Z

Any one noticed that it's not working with stack, it's solved for me with tabs but not stack
after navigating away focus hook doesn't trigger in the previous screen
I tried to deactivate camera on previous screen after navigating away with focus, but didn't work

hisothreed · 2022-03-13T16:01:23Z

useIsFocused is still not working correctly when enabling enableFreeze in most cases, this is what I'm currently using:

"@react-navigation/bottom-tabs": "^6.2.0",
"@react-navigation/native": "^6.0.8",
"@react-navigation/native-stack": "^6.5.0",
"@react-navigation/stack": "^6.1.1",
"react-native-screens": "3.13.1",

EDIT

ok I solved this by adding a timeout.

diff --git a/node_modules/react-native-screens/src/index.native.tsx b/node_modules/react-native-screens/src/index.native.tsx
index 35ce64c..6e4feb7 100644
--- a/node_modules/react-native-screens/src/index.native.tsx
+++ b/node_modules/react-native-screens/src/index.native.tsx
@@ -170,7 +170,7 @@ function DelayedFreeze({ freeze, children }: FreezeWrapperProps) {
     // setImmediate is executed at the end of the JS execution block.
     // Used here for changing the state right after the render.
     setImmediate(() => {
-      setFreezeState(freeze);
+      setTimeout(() => setFreezeState(freeze), 100)
     });
   }

solves the problem but it doesn't sound like the right thing to do.

amadeus · 2022-11-08T01:24:01Z

Looking at that code for the delayed freeze, it's actually not concurrent safe (executing side effects in a render function, that could get thrown away). A more proper fix would be something like this:

diff --git a/node_modules/react-native-screens/src/index.native.tsx b/node_modules/react-native-screens/src/index.native.tsx
index edcd032..78aba2b 100644
--- a/node_modules/react-native-screens/src/index.native.tsx
+++ b/node_modules/react-native-screens/src/index.native.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {useEffect} from 'react';
 import {
   Animated,
   Image,
@@ -152,13 +152,11 @@ function DelayedFreeze({ freeze, children }: FreezeWrapperProps) {
   // flag used for determining whether freeze should be enabled
   const [freezeState, setFreezeState] = React.useState(false);
 
-  if (freeze !== freezeState) {
-    // setImmediate is executed at the end of the JS execution block.
-    // Used here for changing the state right after the render.
+  useEffect(() => {
     setImmediate(() => {
       setFreezeState(freeze);
     });
-  }
+  }, [freeze])
 
   return <Freeze freeze={freeze ? freezeState : false}>{children}</Freeze>;
 }

FWIW: This does fix the problem for us with Native Stacks and useIsFocused firing properly

jordanj77 · 2022-12-05T05:30:17Z

@amadeus your fix is also working for us. Do you want to open a PR maybe?

trickeyd · 2023-03-02T23:12:48Z

Looking at that code for the delayed freeze, it's actually not concurrent safe (executing side effects in a render function, that could get thrown away). A more proper fix would be something like this:

diff --git a/node_modules/react-native-screens/src/index.native.tsx b/node_modules/react-native-screens/src/index.native.tsx
index edcd032..78aba2b 100644
--- a/node_modules/react-native-screens/src/index.native.tsx
+++ b/node_modules/react-native-screens/src/index.native.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {useEffect} from 'react';
 import {
   Animated,
   Image,
@@ -152,13 +152,11 @@ function DelayedFreeze({ freeze, children }: FreezeWrapperProps) {
   // flag used for determining whether freeze should be enabled
   const [freezeState, setFreezeState] = React.useState(false);
 
-  if (freeze !== freezeState) {
-    // setImmediate is executed at the end of the JS execution block.
-    // Used here for changing the state right after the render.
+  useEffect(() => {
     setImmediate(() => {
       setFreezeState(freeze);
     });
-  }
+  }, [freeze])
 
   return <Freeze freeze={freeze ? freezeState : false}>{children}</Freeze>;
 }

FWIW: This does fix the problem for us with Native Stacks and useIsFocused firing properly

Hmmm - I came up with this exact solution, but in my case it drastically effects performance when moving between tabs. It is only marginally better than not freezing at all.

Has anyone else used react-freeze with @react-navigation/bottom-tabs? It works great, just can't figure out how to react to isFocused in a performant way 😭

BTW - I don't think there is any need to use setImmediate in the above solution - I think setting of the state alone is identical.

Aryk · 2023-11-08T23:22:35Z

Looking at that code for the delayed freeze, it's actually not concurrent safe (executing side effects in a render function, that could get thrown away). A more proper fix would be something like this:

diff --git a/node_modules/react-native-screens/src/index.native.tsx b/node_modules/react-native-screens/src/index.native.tsx
index edcd032..78aba2b 100644
--- a/node_modules/react-native-screens/src/index.native.tsx
+++ b/node_modules/react-native-screens/src/index.native.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {useEffect} from 'react';
 import {
   Animated,
   Image,
@@ -152,13 +152,11 @@ function DelayedFreeze({ freeze, children }: FreezeWrapperProps) {
   // flag used for determining whether freeze should be enabled
   const [freezeState, setFreezeState] = React.useState(false);
 
-  if (freeze !== freezeState) {
-    // setImmediate is executed at the end of the JS execution block.
-    // Used here for changing the state right after the render.
+  useEffect(() => {
     setImmediate(() => {
       setFreezeState(freeze);
     });
-  }
+  }, [freeze])
 
   return <Freeze freeze={freeze ? freezeState : false}>{children}</Freeze>;
 }

FWIW: This does fix the problem for us with Native Stacks and useIsFocused firing properly

This fixed it for me too! Why is this not merged in still and why is the ticket closed? 👀

https://github.com/software-mansion/react-native-screens/blob/3.27.0/src/index.native.tsx#L197

amadeus · 2023-12-02T03:22:46Z

Finally got around to opening up a PR for this here: #1980

## Description Executing side effects in render is usually considered bad manners in react land, and given concurrent rendering could have unexpected results if renders are thrown away. ## Changes This change makes it so setImmediate fires in an effect, and also cleans up after itself if for whatever reason the callback isn't fired. (although I think it's probably not possible with how setImmediate is implemented) ## Screenshots / GIFs N/A ## Test code and steps to reproduce We (at Discord) ran essentially this patch for probably over a year now without issue and figured it might be good to give back. Here's an issue related to it: #1198 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation:  - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes

## Description Executing side effects in render is usually considered bad manners in react land, and given concurrent rendering could have unexpected results if renders are thrown away. ## Changes This change makes it so setImmediate fires in an effect, and also cleans up after itself if for whatever reason the callback isn't fired. (although I think it's probably not possible with how setImmediate is implemented) ## Screenshots / GIFs N/A ## Test code and steps to reproduce We (at Discord) ran essentially this patch for probably over a year now without issue and figured it might be good to give back. Here's an issue related to it: software-mansion/react-native-screens#1198 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation:  - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes

…#1980) ## Description Executing side effects in render is usually considered bad manners in react land, and given concurrent rendering could have unexpected results if renders are thrown away. ## Changes This change makes it so setImmediate fires in an effect, and also cleans up after itself if for whatever reason the callback isn't fired. (although I think it's probably not possible with how setImmediate is implemented) ## Screenshots / GIFs N/A ## Test code and steps to reproduce We (at Discord) ran essentially this patch for probably over a year now without issue and figured it might be good to give back. Here's an issue related to it: software-mansion#1198 ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation:  - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes

kacperkapusciak added the Library: Freeze Issue related with react-freeze label Nov 16, 2021

kacperkapusciak mentioned this issue Nov 24, 2021

feat: disable freeze for first render #1220

Merged

2 tasks

kacperkapusciak closed this as completed in #1220 Dec 2, 2021

kacperkapusciak mentioned this issue Dec 13, 2021

Freezing with react navigation bottom-tabs software-mansion/react-freeze#3

Closed

what-the-diff bot mentioned this issue Jan 19, 2023

fix: lazy render when focus and update some deps OneKeyHQ/app-monorepo#2431

Merged

amadeus mentioned this issue Dec 2, 2023

fix: move DelayedFreeze setImmediate into an effect #1980

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enableFreeze cause useIsFocused stop working #1198

enableFreeze cause useIsFocused stop working #1198

r0b0t3d commented Nov 2, 2021

marhaupe commented Nov 2, 2021

r0b0t3d commented Nov 2, 2021

tomasmozeris commented Nov 4, 2021 •

edited

Loading

nandorojo commented Nov 4, 2021

marhaupe commented Nov 8, 2021

nandorojo commented Nov 8, 2021

tomasmozeris commented Nov 8, 2021 •

edited

Loading

nandorojo commented Nov 8, 2021

Titozzz commented Nov 8, 2021 •

edited

Loading

kacperkapusciak commented Nov 24, 2021

marhaupe commented Nov 25, 2021

r0b0t3d commented Nov 26, 2021

Mohamed-kassim commented Jan 27, 2022 •

edited

Loading

hisothreed commented Mar 13, 2022 •

edited

Loading

amadeus commented Nov 8, 2022 •

edited

Loading

jordanj77 commented Dec 5, 2022

trickeyd commented Mar 2, 2023 •

edited

Loading

Aryk commented Nov 8, 2023 •

edited

Loading

amadeus commented Dec 2, 2023

enableFreeze cause useIsFocused stop working #1198

enableFreeze cause useIsFocused stop working #1198

Comments

r0b0t3d commented Nov 2, 2021

Description

Screenshots

Steps To Reproduce

Expected behavior

Actual behavior

Reproduction

Platform

Workflow

Package versions

marhaupe commented Nov 2, 2021

r0b0t3d commented Nov 2, 2021

tomasmozeris commented Nov 4, 2021 • edited Loading

nandorojo commented Nov 4, 2021

marhaupe commented Nov 8, 2021

nandorojo commented Nov 8, 2021

tomasmozeris commented Nov 8, 2021 • edited Loading

nandorojo commented Nov 8, 2021

Titozzz commented Nov 8, 2021 • edited Loading

kacperkapusciak commented Nov 24, 2021

marhaupe commented Nov 25, 2021

r0b0t3d commented Nov 26, 2021

Mohamed-kassim commented Jan 27, 2022 • edited Loading

hisothreed commented Mar 13, 2022 • edited Loading

amadeus commented Nov 8, 2022 • edited Loading

jordanj77 commented Dec 5, 2022

trickeyd commented Mar 2, 2023 • edited Loading

Aryk commented Nov 8, 2023 • edited Loading

amadeus commented Dec 2, 2023

tomasmozeris commented Nov 4, 2021 •

edited

Loading

tomasmozeris commented Nov 8, 2021 •

edited

Loading

Titozzz commented Nov 8, 2021 •

edited

Loading

Mohamed-kassim commented Jan 27, 2022 •

edited

Loading

hisothreed commented Mar 13, 2022 •

edited

Loading

amadeus commented Nov 8, 2022 •

edited

Loading

trickeyd commented Mar 2, 2023 •

edited

Loading

Aryk commented Nov 8, 2023 •

edited

Loading