software-mansion/react-native-screens

enableFreeze cause useIsFocused stop working

Closed this issue Β· 19 comments

Description

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

Screenshots

Steps To Reproduce

  1. Create tabs with 2 screens Home and Settings
  2. In Settings, add const isFocused = useIsFocused()
  3. Navigate to Settings screen, isFocused updated to true (this is correct)
  4. 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

  • iOS
  • Android
  • Web
  • Windows
  • tvOS

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

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.

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

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.

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

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.

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

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

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?

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 πŸ‘€

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!

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.

@kacperkapusciak it works for me too πŸš€. Thanks

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

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.

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

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

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 commented

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

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