Closed r0b0t3d closed 2 years ago
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.
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: https://github.com/software-mansion/react-native-screens/pull/1980
Description
In bottom tabs, when enabling
enableFreeze
,useIsFocused
is not triggered when screen blur.Screenshots
Steps To Reproduce
Home
andSettings
Settings
, addconst isFocused = useIsFocused()
Settings
screen,isFocused
updated totrue
(this is correct)Home
,isFocused
stilltrue
Expected behavior
At step 4,
isFocused
inSettings
should befalse
Actual behavior
At step 4,
isFocused
inSettings
istrue
Reproduction
Platform
I just checked on iOS, not sure about other platforms
Workflow
Package versions