Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when reset from nested navigator #6715

Closed
leethree opened this issue Sep 24, 2019 · 6 comments
Closed

Crash when reset from nested navigator #6715

leethree opened this issue Sep 24, 2019 · 6 comments

Comments

@leethree
Copy link
Contributor

Current Behavior

The following exception is thrown when tries to reset app to top-level from nested navigator:

TypeError: undefined is not an object (evaluating 'screen.options')

Stack trace:
  /<redacted>/navigation-ex/packages/core/src/useDescriptors.tsx:130:30 in options
  /<redacted>/navigation-ex/packages/stack/src/views/Stack/StackView.tsx:91:23 in isAnimationEnabled
  /<redacted>/navigation-ex/packages/stack/src/views/Stack/StackView.tsx:100:29 in
  ...

Expected Behavior

App should be reset to the destination state.

How to reproduce

  • Add the reset code to the example app:
diff --git a/packages/example/src/Screens/SimpleStack.tsx b/packages/example/src/Screens/SimpleStack.tsx
index 7d16651..05dc8a0 100644
--- a/packages/example/src/Screens/SimpleStack.tsx
+++ b/packages/example/src/Screens/SimpleStack.tsx
@@ -27,7 +27,10 @@ const ArticleScreen = ({
     <View style={styles.buttons}>
       <Button
         mode="contained"
-        onPress={() => navigation.push('album')}
+        onPress={() => navigation.reset({
+          index: 0,
+          routes: [{ name: 'compat-api', key: 'whatever' }],
+        })}
         style={styles.button}
       >
         Push album
  • Open the example app, go to "Simple Stack" or "Bottom Tabs".
  • Tap "Push Album"
  • Crash

Your Environment

software version
react-navigation 76de32f
react-native 0.59.10
expo ^34.0.1
node 10.15.0
npm or yarn yarn 1.17.3
@satya164
Copy link
Member

satya164 commented Sep 24, 2019

The reset method resets the state of the current navigator. Looks like you're trying to update the state of a parent navigator. In which case, you need to specify which navigator's state you want to change using the target key:

navigation.dispatch({
  ...CommonActions.reset({
    index: 0,
    routes: [{ name: 'compat-api', key: 'whatever' }],
  }),
  target: 'state.key-of-the-navigator'
});

Also, here the top-level navigator is a drawer navigator. Not the stack navigator whose state you're trying to reset.

@leethree
Copy link
Contributor Author

Ah I see. It's not ideal because the screens would need to know about how the navigation is structured.

reset action seems to be very difficult to use in this way as it requires a lot of knowledge and also we need to generate a key for all the routes as well.

@satya164
Copy link
Member

Ah I see. It's not ideal because the screens would need to know about how the navigation is structured.
reset action seems to be very difficult to use in this way as it requires a lot of knowledge

In this case you're trying to reset state of a parent stack navigator which is not root. Without key, there'd be no way to know which navigator to reset. But reset is usually for advanced tasks, so I think it's okay that it requires additional knowledge.

We could add another resetRoot method to easily reset the root navigator's state. This method already exists in the ref of the container.

we need to generate a key for all the routes as well

The key should be generated automatically if you don't provide them.

@leethree
Copy link
Contributor Author

I think resetRoot makes sense.

The key should be generated automatically if you don't provide them.

In my testing, the keys are missing from the state after @@RESET_ROOT action is dispatched. But they are generated whenever the next navigation action is dispatched. Not sure if it's intended behaviour but it seems problematic to not always have keys in the state.

@satya164
Copy link
Member

It's how the library works. The state can be in 2 modes: stale, and non-stale (denoted by the stale key in the state object). When a stale state is rehydrated, necessary fields like keys are added. The navigators always use non-stale state. The root navigation state is updated with the non-stale version the next time a navigation action happens.

@leethree
Copy link
Contributor Author

What you said works fine with resetRoot. But with the regular reset, I got a warning from React for not having keys:

Warning: Each child in a list should have a unique "key" prop.

Check the render method of `Stack`. See https://fb.me/react-warning-keys for more information.
    in MaybeScreen (at Stack.tsx:367)
    in Stack (at StackView.tsx:275)
    in StackView (at createStackNavigator.tsx:69)
    in KeyboardManager (at createStackNavigator.tsx:67)
    in StackNavigator (at AppNavigator.tsx:64)

@satya164 satya164 transferred this issue from react-navigation/navigation-ex Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants