useFocusEffect should not retrigger on re-render

[slorber]

Reported by in https://github.com/react-navigation/hooks/pull/43/files#r334335721 by @ArrayZoneYour

The following code is likely to produce an infinite loop, even if we make sure the dependency is stable using a ref, because useFocusEffect use "navigation" as a dependency and the object is unstable.

useFocusEffect(useCallback(() => { 
  setParamsRef.current({});
},[setParamsRef])); 

The focus effect might retrigger unnecessarily due to the navigation being unstable

Somehow this is related to #40

@satya164 what do you think about using a useNavigationRef hook as you suggested in some other issue? Didn't get a clear answer from the React team on how to handle this kind of situation but that should work. Or maybe I should depend only on isFocused and addListener which are the only 2 fn used, but if core does not guarantee stability across render the problem remains.

[satya164]

I think it's okay to use useNavigationRef for the navigation prop for internal implementation of useFocusEffect in this library,

[slorber]

@satya164 I think it will only solve half of the problem for users.

useFocusEffect(useCallback(() => { 
   setParams({something: true});
},[setParams]));

The following code will trigger again and again without a stable setParams (cf #35)

If we don't export this API, people will have to find a solution themselves, like:

const useNavigationRef = () => {
  const navigation = useNavigation()
  const ref = useRef(navigation);
  useLayoutEffect(() => {
    ref.current = navigation;
  });
  return ref;
}; 

const navigationRef = useNavigationRef();
useFocusEffect(useCallback(() => { 
   navigationRef.setParams({something: true});
},[navigationRef]));

So basically they end up re-implementing the internally used useNavigationRef hook

[dngconsulting]

Is there any update on this issue ? I'm currently facing the problem and the previous solution (with useNavigationRef) doesn't work for me (I'm using 1.1.0).
thanx
Sami

[slorber]

hi @dngconsulting can you share a repro using useNavigationRef ?
What exactly isn't working, you have an infinite loop or something?

[dngconsulting]

Yes absolutely, an infinite loop, I have managed to do it in another way with useEffect and deps array parameter. Thanks.

[slorber]

Hi,

Do you remember what was the problem with your code? Is there something wrong in this lib or was it your fault?

[dngconsulting]

Frankly speaking I didn't dedicated too much time to this issue because it was flagged here. For me, it's not possible to update the navigation object (with setParam) when you are in FocusEffect hook. You can reproduce it very easily, by changing dynamically the title header of react-navigation in the hook. Hoping that react navigation next will ship soon ...

[slorber]

Are you sure you were using useCallback inside useFocusEffect ? It's important to provide a stable callback to useFocusEffect

The useFocusEffect of navigation next is quite similar in behavior because it has been backported from v5 to v4. If there's something that does not work well, it would be nice to report it nicely here so that we can eventually fix it in v5 too before an official release.