Skip to content

Commit

Permalink
fix: prevent navigation state updates after state cleanup (#9688)
Browse files Browse the repository at this point in the history
Problem:
When using nested navigators, unmounts cause race cleanup races.

Imagine following hierarchy (from tree root downwards, parent to children):
TabNavigator (1) [renders useNavigationBuilder]
  SceneView (from TabNavigator)
StackNavigators (N) [each renders useNavigationBuilder] 
  SceneView (from StackNavigator)

Now lets test following flow:
1. Mount above navigators with given navigation params (e.g. navigation for unauthenticated users) 
2. Unmount all navigators (e.g. during login process)
3. Mount above navigation with different navigation params than in 1) (e.g. navigation for authenticated users)

What you'll observe, there will be old navigation params preserved in 3) coming from 1).

Source of problem:
BaseNavigationContainer holds global navigation state, exposes API to modify it via NavigationStateContext. When useNavigationBuilder unmounts, it attempts to clear navigation state. (see cleanup effect in useNavigationBuilder.tsx).

(I) First clear occurs in TabNavigator's effect, which successfully clears BaseNavigationContainer's state (sets it to undefined).

(II) Second clear comes from StackNavigator unmount. It's useNavigationBuilder cleanup effect also calls NavigationStateContext.setState(undefined).
But this time - we meet SceneView as closest NavigationStateContext.Provider. SceneView attempts to merge state change with current navigation state, which is reasonable. But current navigation state should be already undefined... It is, but:
```
[useNavigationBuilder.tsx]

const getState = React.useCallback((): State => {
    const currentState = getCurrentState();

    return isStateInitialized(currentState)
      ? (currentState as State)
      : (initializedStateRef.current as State);
  }, [getCurrentState, isStateInitialized]);
```
"undefined" state is treated is non-initialized state, and freshly computed state (initializedStateRef.current) is returned instead.
SceneView does merge this old state with `undefined` value, and passes to BaseNavigationContainer. Now we have some legacy global state, despite all navigators being unmounted.

After mounting navigators again (3), we can observe old params being restored. These params might come e.g. from old `initialParams` prop (from 1)).

Solution:
Do not propagate `setState` upwards in `useNavigationBuilder` after state cleanup. This way we'll omit such races.
  • Loading branch information
DrRefactor committed Jul 16, 2021
1 parent ee12690 commit 16f0e11
Showing 1 changed file with 22 additions and 2 deletions.
24 changes: 22 additions & 2 deletions packages/core/src/useNavigationBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,32 @@ export default function useNavigationBuilder<
const {
state: currentState,
getState: getCurrentState,
setState,
setState: setCurrentState,
setKey,
getKey,
getIsInitial,
} = React.useContext(NavigationStateContext);

const stateCleanedUp = React.useRef(false);

const cleanUpState = React.useCallback(() => {
setCurrentState(undefined);
stateCleanedUp.current = true;
}, [setCurrentState]);

const setState = React.useCallback(
(state: NavigationState | PartialState<NavigationState> | undefined) => {
if (stateCleanedUp.current) {
// State might have been already cleaned up due to unmount
// We do not want to expose API allowing to override this
// This would lead to old data preservation on main navigator unmount
return;
}
setCurrentState(state);
},
[setCurrentState]
);

const [initializedState, isFirstStateInitialization] = React.useMemo(() => {
// If the current state isn't initialized on first render, we initialize it
// We also need to re-initialize it if the state passed from parent was changed (maybe due to reset)
Expand Down Expand Up @@ -444,7 +464,7 @@ export default function useNavigationBuilder<
// Otherwise, our cleanup step will cleanup state for the other navigator and re-initialize it
setTimeout(() => {
if (getCurrentState() !== undefined && getKey() === navigatorKey) {
setState(undefined);
cleanUpState();
}
}, 0);
};
Expand Down

0 comments on commit 16f0e11

Please sign in to comment.