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

StackRouter.getStateForAction(reset()) should work without passing state 2nd arg #5175

Open
wants to merge 1 commit into
base: 2.x
from

Conversation

Projects
None yet
2 participants
@slorber
Contributor

slorber commented Oct 26, 2018

I just migrated from 1.0-beta to 2.18.1 and the behavior of StackRouter.getStateForAction changed regarding reset action.

Let's consider this navigator:

const AppNavigation = createStackNavigator(
  {
    HomeScreen: { screen: HomeScreen },
    SecondScreen: { screen: SecondScreen },
  },
);

Before, one could do:

const SECOND_SCREEN_STATE = getStateForAction(
  StackActions.reset({
    index: 0,
    actions: [NavigationActions.navigate({ routeName: 'SecondScreen' })],
  }),
);

now, the 2nd arg is mandatory for the reset action to behave correctly. If you don't pass the initial state arg, getStateForAction always reset to the initial route name ("HomeScreen")

const INITIAL_STATE = getStateForAction(
  NavigationActions.navigate({ routeName: 'HomeScreen' }),
);

const SECOND_SCREEN_STATE = getStateForAction(
  StackActions.reset({
    index: 0,
    actions: [NavigationActions.navigate({ routeName: 'SecondScreen' })],
  }),
  INITIAL_STATE, // => this is now needed
);

I think this behavior is confusing for the users that would expect reset to work without providing any state. For example: #3031 (comment)

I don't know when this behavior changed, but as far as I know I don't remember seing it in breaking changes of 2.x.

About the PR: it is probably not a very elegant implementation. Maybe I should handle this in "getInitialState(action)" instead, not sure how however. Also there's the "replace" action to consider? If this change is approved, please tell me how to polish my PR

@slorber slorber changed the title from fix StackRouter.getStateForAction requiring state to behave correctly to StackRouter.getStateForAction(reset()) should work without passing state 2nd arg Oct 26, 2018

@react-navigation-ci

This comment has been minimized.

@brentvatne brentvatne requested a review from ericvicenti Nov 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment