Skip to content
New issue

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

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

Already on GitHub? # to your account

[Discussion] Usefulness of the Switch #5918

Closed
danielkcz opened this issue Jan 31, 2018 · 9 comments
Closed

[Discussion] Usefulness of the Switch #5918

danielkcz opened this issue Jan 31, 2018 · 9 comments

Comments

@danielkcz
Copy link

danielkcz commented Jan 31, 2018

This is a kinda spin-off from #5785.

Current state

Currently, the react-router is built so we can have routes all over the app and don't bother much with static routing configuration. That's definitely great concept, but there is one flaw.

The Switch most common use case is to have a fallback route that should be rendered when nothing else matches. Apparently, that doesn't work well with dynamic routing nature. It forces us to have all routes in a single place again to try match against all of them till it reaches the fallback one.

Another issue is in case I simply want to wrap a single Route to another component because there is some additional logic within the route that would clutter whole Switch

<Switch>
  <TooSmartRoute /> // notice no props here, everything is inside the component
</Switch>

Such route is always matched by the Switch simply because it's lacking path in the props.

Solution?

Letting Switch to traverse the whole tree of children to find a match is definitely a bad idea.

Honestly, I don't have any proven solution, just a hot idea. It would be something along the lines of having a new component called eg. Fallback which would accept prop of the fallback route.

<Fallback route={<NotFound />}>
  // ... some deeply nested routing
</Fallback>

This component would just render everything without any traversal. When some Route would match, it would internally use context to inform Router about that. Then the Fallback would be able to pick up that information (not sure if that's possible actually) and render fallback route if there wasn't any match.

Any other thoughts/ideas?

@jamesplease
Copy link

I agree that this is a problem. Many apps only need a single NotFound route, conceptually, but when I use RR v4 my apps tend to have many. This is mostly a problem because it’s easy to forget to add it.

I’d love to see a solution, such as ‘Fallback‘, to this.

@timdorr
Copy link
Member

timdorr commented Jan 31, 2018

While you might be able to build something like this now, this will essentially be impossible when async rendering drops in React. Namely because you can never know when the state of all Routes within a tree are fully materialized. Routes can pop in and out of the tree at any moment asynchronously. And the lifecycle methods to synchronize them (cWM, cWRP) are going away. So you'll end up with weird synchronization issues, mostly in the form of trailing content in the DOM while the render loop finishes processing and reconciling.

This is going to be an interesting transition for most apps and libraries, where certain guarantees are no longer possible.

Really the only solution here will be to go back to the old way of doing things via a top-level route configuration, aka react-router-config. That can manage the config internally and stays synchronous within its own lifecycle (async doesn't do any preemption). I'd like to get it out of beta and close out things like #5429 and #4962. It can solve a lot of these problems, but it needs some more superpowers to be truly useful.

@danielkcz
Copy link
Author

danielkcz commented Jan 31, 2018

Hmm, did not know about such drastic changes coming with React lifecycle.

So let me get this straight. Currently, it's necessary to centralize routing just to have a fallback route? That's almost funny how such a small and relatively boring thing can be so troublesome :/

It almost feels like there would need to be some kind of CLI traversal tool to collect all possible routes from the app code, store list of those and use them in runtime for a Switch to work correctly. Is that insane enough? 🙄

@timdorr
Copy link
Member

timdorr commented Jan 31, 2018

That's usually how it goes. 😄

You can't traverse the code because the execution is dynamic. And there is no CLI in the browser, so that would exclude many classes of applications. The library approach of react-router-config is really what you're describing anyways, so that just needs to be leveled up and released as 1.0.0.

@danielkcz
Copy link
Author

danielkcz commented Jan 31, 2018

To be honest, I am not entirely sure how is RRC relevant to this issue? I mean it's nice to have a config which is somewhat easier to analyze than a component tree, but it uses Switch internally too so it has same problem in the end, right? Am I missing something here?

@timdorr
Copy link
Member

timdorr commented Jan 31, 2018

It currently uses Switch, but I'd like to refactor it to use matchRoutes() instead.

@timdorr
Copy link
Member

timdorr commented Jan 31, 2018

That's particularly necessary to solve things like #5429, which Switch's semantics don't allow for (nor should they).

@danielkcz
Copy link
Author

danielkcz commented Feb 23, 2018

I think we can close it here. I took a different approach. This is how my main Routing looks like now and I am almost happy about it.

<Switch>
  <Route path="/#" component={LoginRoutes} />
  <SecureRoute exact path="/" component={DashboardPage} />
  <SecureRoute path="/order" component={OrderRoutes} />
  <Route component={NotFoundPage} />
</Switch>

This way I can still keep a modularized approach with an assumption that each module sits at some specific path. When that path is hit, I can let that module decide on what to do next.

export const OrderRoutes = ({ match }) => (
  <Switch>
    <SecureRoute exact path={match.path} component={OrderOverviewPage} />
    <SecureRoute exact path={`${match.path}/create`} component={OrderCreatePage} />
    <Route component={NotFoundPage} />
  </Switch>
)

I don't particularly like that string interpolation there, but I wasn't able to figure out a better way since Switch is again matching against immediate children's path.

@edkalina
Copy link

edkalina commented Jun 3, 2018

It was announced that React Call Return won't be released. So I've implemented similar pattern as react-aggregation library.

Here is concept of new Switch implemented with react-aggregation. It allows to nest routes any way you want.

What do you think about this? @FredyC @timdorr @jamesplease

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants