Clarify StackNavigator inside DrawerNavigator gotchas

[brentvatne]

Moved from https://github.com/react-navigation/react-navigation/issues/3890

---

Using multiple Navigators inside a DrawerNavigator, where the subnavigators shared a route name, led to unexpected (undocumented) behavior

software	version
react-navigation	1.5.9
react-native	expo@26

Context

I was an early user of @expo/ex-navigation, and have recently been upgrading to react-navigation. I came across some faults in either the DrawerNavigation documentation or in the general mental framework of react-navigation.

Current Behavior

When nesting navigators inside a DrawerNavigator, duplicated route names behave unexpectedly (in my mental model). Expo Snack repro

const WidgetStack = StackNavigator({
  Widget: {
    screen: WidgetScreen,
  },
  // This is very easy to get wrong, it is not clear in the Drawer documentation
  // that this route name should be distinct.
  Settings: {
    screen: SettingsScreen,
  },
});

const SettingsStack = StackNavigator({
  Settings: {
    screen: SettingsScreen,
  },
});

const DrawerApp = DrawerNavigator({
  Widget: {
    screen: WidgetStack,
  },
  Settings: {
    screen: SettingsStack,
  },
}, {
  initialRouteName: 'Widget',
});

When tapping on the "Settings" drawer item, I expected the currently selected "tab" in the drawer to be "Settings", I expected no routes to be "pushed" onto my stack navigator. Instead, the "widget" tab stays selected, and a settings route is pushed onto the WidgetStack.

There were no pieces of documentation to warn me of this behavior, and it was actually very difficult to even tell this is what was happening (it was much easier when I decided to build the minimal repro).

My current mental model

My mental model for react-navigation was previously that each nested navigator was a "subtree". With that model, I saw no issue with a duplicated route name.

               DrawerApp
          /         \
      WidgetStack    SettingsStack
          |     |          |
      /      \           Settings
    Widget   Settings

I'm not here to debate whether this is the correct model, but a simple piece of documentation would have saved me about an entire day :(

I would be happy to submit a PR to address this, but it appears with the 2.0 push, this might be out of date, and furthermore - I am not sure if my current model is correct enough to even write docs.

[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.

[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!

[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.

[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.

[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 notifications@github.com 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 https://github.com/react-navigation/react-navigation.github.io/issues/117#issuecomment-386457778, 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