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

Idempotent navigate with key for StackRouter #3393

Merged
merged 1 commit into from
Feb 1, 2018

Conversation

ericvicenti
Copy link
Contributor

The navigation action will now always behave idempotently when called with a key. When pushing, it will push a route with that key. If the route already exists, it will navigate to that route and set the params.

This is a commonly requested feature, and will address the needs of #2334 #135 #1313, #2400, #2578

Copy link

@sameer-ahmed sameer-ahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test case check failed

@codecov-io
Copy link

codecov-io commented Jan 31, 2018

Codecov Report

Merging #3393 into master will decrease coverage by 0.2%.
The diff coverage is 64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3393      +/-   ##
==========================================
- Coverage   72.95%   72.75%   -0.21%     
==========================================
  Files          52       53       +1     
  Lines        1638     1659      +21     
  Branches      428      435       +7     
==========================================
+ Hits         1195     1207      +12     
- Misses        349      357       +8     
- Partials       94       95       +1
Impacted Files Coverage Δ
src/NavigationActions.js 92.72% <100%> (+0.27%) ⬆️
src/routers/KeyGenerator.js 100% <100%> (ø)
src/addNavigationHelpers.js 60% <33.33%> (-20%) ⬇️
src/routers/StackRouter.js 93.67% <58.33%> (-2.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffcf179...495be80. Read the comment docs.

@ericvicenti ericvicenti force-pushed the @ericvicenti/idempotent branch 2 times, most recently from 08b07b6 to de0ee44 Compare February 1, 2018 03:42
@dann1609
Copy link

@ericvicenti please include same solution to NavigationActions. Other way redux user can get your solution.

@brentvatne
Copy link
Member

look at the diff @dann1609, you can use this with redux, just include the key in the action...

@dann1609
Copy link

dann1609 commented Feb 19, 2018

@brentvatne I did and didn't work.

@ericvicenti
Copy link
Contributor Author

@dann1609, can you please file a detailed issue with the exact example code that demonstrates the problem? Otherwise we can’t help you.

@dann1609
Copy link

dann1609 commented Feb 19, 2018

@ericvicenti I do my navigation using the next code:

dispatch(NavigationActions.navigate( { routeName: route, params: params, key:key } ))

App navigate without problem, but next page (in stack navigator) set a random key, not the key provided. So, the next time I call the same dispatch. Navigation will create the same page, but a new one, since key provided is not the same key of previous pages.

@lucaslz2020
Copy link

lucaslz2020 commented Mar 24, 2018

Using key StackNavigator triggers transition twice when double tapping fast .
may lead to the following situation:
#3231
In this environment the sources for assign MUST be an object. This error is a performance optimization and not spec compliant.

@marsonmao
Copy link

I tried supplying the key of the current screen to navigate, it works and prevents multiple stacking with multiple touching, but it only works once. If I want to navigate one more level deeper then it won't do it.

I think the reason is that once I supply the current key, the router propagates/memorizes it so that I cant navigate twice since the key remains the same at the second navigate.

To be more specific the navigation structure looks like this:

[screen1]: { Screen1 }
[screen2]: { Screen2 }
[screen3]: { Screen3 }

in Screen1:
dispatch(NavigationActions.navigate({ routeName: 'screen2', key: this.props.navigation.state.key})); is doing fine. But if I do it again in Screen2:
dispatch(NavigationActions.navigate({ routeName: 'screen3', key: this.props.navigation.state.key})); has no effect, since the 2 keys are the same.

The this.props.navigation.state.key are from different components (Screen1 and Screen2) so they should be different, which is the case If I do not supply the key, then each route key ends with the counter++ so that each key is unique.

So the question is, how do I correctly use this new feature? Thanks.

@marsonmao
Copy link

@ericvicenti Hope you got some time to take a look 😄

@ericvicenti
Copy link
Contributor Author

I can only help if you post a MVCE of your problem: https://stackoverflow.com/help/mcve

Otherwise it’s not clear what is going on

@sonaye
Copy link

sonaye commented Apr 28, 2018

I've been looking for something like this for ages.

@marsonmao If you are using a StackNavigator, the first route by default will have a random key unless you specify it in the StackNavigator configs initialRouteKey: 'Home' (#3540).

@taylorkline
Copy link

Does the documentation include any examples of what makes for a good key?

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

Successfully merging this pull request may close these issues.

9 participants