Clarify StackNavigator inside DrawerNavigator gotchas about react-navigation.github.io

[skabbes]

Thanks, Brent!

I did already figure it out, the "bug" is not with how it works, but with how it is documented. I really struggled conceptually with what is a "route", what is a "navigator", and what is a "screen". And most importantly, how they compose together.

since the deepest active stack navigator can handle the Settings route
While this makes sense inside a stack navigator, it did not make sense to me when the "active" navigator was the DrawerNavigator.

I didn't see reference to this anywhere in the documentation, not complaining just mentioning.

unless you have a clear purpose for making multiple routes have the same name, you should always be unique.

This was also due to a complete misunderstanding of "routes". In my mind, a route and a screen were the same concept. Coming from the web, a route is the "URL" and the screen is the component rendered. Relative URLs was the way I expected navigate to work, more or less.

navigate('Settings') would navigate you to the Settings Screen. But if you're inside a stack router, it pushes it. If you are inside a tab, it switches tabs. Similar to "./settings" or "../settings" potentially on web.

I did not expect the router to reach into my routes to try to deep match them, just like on a website - you wouldn't expect ahref="settings" to automatically select /tabs/TabC/settings . I know web is different from mobile, but when you talk about "navigation" it is surely the most ubiquitous "routing" mental model.

unless you have a clear purpose for making multiple routes have the same name, you should always be unique.

IMO this really hurts the compose-ability Navigators in react-navigation, as far as I understand it. It means that routers need to be globally aware of all other routes and be extra careful to not have them overlap.

[brentvatne]

hey @skabbes I agree this is a little bit hard to wrap your head around and not well documented. I try to explain how this works in more detail in this talk: https://www.youtube.com/watch?v=wJJZ9Od8MjM - I need to port this to the documentation at some point. related slides: http://url.brentvatne.ca/Vb9cTb

IMO this really hurts the compose-ability Navigators in react-navigation, as far as I understand it. It means that routers need to be globally aware of all other routes and be extra careful to not have them overlap.

cc @ericvicenti

[skabbes]

Awesome, just watched the whole thing.

@brentvatne sounded like you were talking directly to me in minutes 15 to 17 :)

I still don't understand why deepest navigator wins, but doesn't matter - if its documented, I can work with it or work against it!

[ericvicenti]

Thanks for the honest feedback! There are definitely some trade-offs here.

In larger apps I think this route resolution does make sense, because different screens may show up in different contexts, and be linked to from anywhere.

Imagine a settings modal that can come up and have inner navigation to go between the settings screens. Different areas of the app may need to deep link to particular settings screens. So when the user taps a link to one of these specific screens, who is supposed to know which navigator should handle it? The link should be simple, and there may be several contexts where the settings screen can appear. Without this delegation behavior, every single routing behavior would need to be hardcoded within each link!

I'm not really sure what the problem is with using the push action in cases when you want the previous behavior.

Also I'll note that this router delegation behavior is only used as a mechanism of last resort, if a navigator doesn't have a route registered in it. If you don't want to rely on this behavior, you can register all of your routes in the top level navigator and handle things explicitly.

If you want to publish your navigation-aware component to NPM for general consumption, and you want to avoid having conflicting route names with apps that use it, you can use the namespace approach of "my-package/RouteName" to avoid conflicts.

[brentvatne]

hi there!

the issue here is with route names. unless you have a clear purpose for making multiple routes have the same name, you should always be unique.

so, in this case, what is happening is that when you select the settings tab it dispatches a navigation action to navigate to the route Settings. since the deepest active stack navigator can handle the Settings route, it pushes the screen to the widgets stack. there are some things we can do to make this behave more sensibly, but avoiding the same routename will resolve the issue here.

[brentvatne]

@skabbes - it may be useful to imagine what would happen if that wasn't the rule and see how that would play out in various situations. I didn't actually make this design decision but I have found it does make sense in most cases

[skabbes]

I mean - I respectfully disagree - given I was making a pretty vanilla app and hit issues due to this behavior. From your talk, I'm convinced of the merit of "if a route exists, it should be routable from anywhere". I guess I'm just not sold on the resolution order when multiple routes of the same name exist (which is the only issue I have).

I couldn't really come up with any legitimate use cases for multiple routes of the same name with the current route-resolution behavior (a child-first version of DFS it seems). Perhaps that should just be an error - and this discussion ends here. The desire for a global namespace of route names (which seems like a product feature) will always be at odds with maintainability of large apps. I think a version of BFS that prefers routes in the active route's subtree would "do the right thing" almost always, but that's a major change - and probably not worth it.

[skabbes]

Hey Eric,

I think you misunderstood me - I didn't say route delegation is bad, I was merely trying to point out instances where the resolution algorithm is very confusing. I try to follow the principle of least astonishment when designing APIs like this, and this behavior was ... well ... astonishing :)

In fact, the example you gave is the one that caused me pain. It caused me pain because the resolution chosen was very confusing.

For instance:

      Drawer
     /     \
Settings   RouterA
              \
             RouterB
                \
               RouterC
                  \
                 RouterD
                    \
                   Settings

Wouldn't you agree, that if you navigate('Settings') inside a custom drawer component should switch the active route to the top level settings route. Given what Brent described in his talk - it does not, it chooses the deep one. Very confusing.

Also if you watch his talk @16:47 he confirms this, and kind of glosses over why it is that way.

I'm aware of all the possible workarounds you can do here, I just believe for a library like this - usability bugs are bugs, so I thought I'd report it. A simple statement in the docs like "re-using route names is strongly discouraged" would have been really helpful. Anyway, no action needed - just selfishly reporting bugs hoping other people fix them :)

[ericvicenti]

In the apps I’ve seen, when you register the same routeName into multiple navigators, you generally want to get the user to the correct place with minimal disruption, so you want the deepest active router to optionally handle the action. This is generally desirable in the UI so the user doesn’t jump to a different area of the app. I can show further examples if you’d like

Edit: in other words, for your example: the deep Settings would only be navigated to if ‘RouterD’ is already active. I think that’s the desirable behavior. If you don’t want to happen, you can set a different routeName in the deep router.

[skabbes]

you generally want to get the user to the correct place with minimal disruption

Agreed, if not always. It it at least a sensible default.

so you want the deepest active router to optionally handle the action

Agree (except maybe in the special case of a custom drawer component). I agreed with this earlier - stated a different way with "[...] a version of BFS that prefers routes in the active route's subtree [...]".

What I disagree with is, when you have to "hop" to a non-active branch of a router (another tab/switch), from there you should prefer the shallowest matching route, not the deepest one. This corresponds to minimal disruption to the UI.

const MyStackNavigator = createStackNavigator({
  Home: HomeStackScreen,
  Profile: ProfileScreen,
});

const MySwitchNavigator = createStackNavigator({
  Home: HomeSwitchScreen,
  Login: SpecialLoginNavigator,
  Main: MyStackNavigator,
});

If the Login switch is active, and you navigate('Home'), then the SpecialLoginNavigator should handle the action if it can (I agree on this point). If it cannot... It should do the minimally disruptive thing - and simply switch to HomeSwitchScreen. It should not switch to Main, then push HomeStackScreen. That would be much more disruptive ¯\_(ツ)_/¯

[ericvicenti]

Yeah, I think I agree. Does it currently switch to a deeper "Home" than the topmost-level-one?

Its possible we have a bug here, maybe you could write a failing test test for StackRouter that describes this case?

[skabbes]

Yes, this is the bug I originally filed.

[brentvatne]

@skabbes - in the example you shared:

const MyStackNavigator = createStackNavigator({
  Home: HomeStackScreen,
  Profile: ProfileScreen,
});

// there was a typo in example above, where fn below was createStackNavigator, fixed here:
const MySwitchNavigator = createSwitchNavigator({
  Home: HomeSwitchScreen,
  Login: SpecialLoginNavigator,
  Main: MyStackNavigator,
});

this actually works how you expect when you're on Login and navigate to Home- I just wrote a test to confirm it and the logic in the switch makes sense here. I believe I described it incorrectly in my talk!

that said, it's still not the same as the original example you provided with Settings being in multiple route names, including part of the active route subtree

[skabbes]

Thanks, the expo snack I linked should have had all the detail needed too though.

…

On Thu, May 3, 2018, 3:37 PM Brent Vatne ***@***.***> wrote: @skabbes <https://github.com/skabbes> - in the example you shared: const MyStackNavigator = createStackNavigator({ Home: HomeStackScreen, Profile: ProfileScreen, }); // there was a typo in your original example where v was createStackNavigator, fixed here const MySwitchNavigator = createSwitchNavigator({ Home: HomeSwitchScreen, Login: SpecialLoginNavigator, Main: MyStackNavigator, }); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#117 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAkJMsGdaAkSBVquL1CtjsC3Rg2hWLldks5tu4angaJpZM4TFod4> .

[brentvatne]

@skabbes - my bad, I posted too early and edited a bunch

[brentvatne]

https://github.com/react-navigation/react-navigation/blob/ab5481a290a78d75a7bc9f983ced803a0f933ceb/src/routers/__tests__/SwitchRouter-test.js#L186-L216