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

Deprecate <Route exact>, match path exactly by default #4958

Open
mjackson opened this Issue Apr 12, 2017 · 39 comments

Comments

Projects
None yet
@mjackson
Member

mjackson commented Apr 12, 2017

I'm beginning to think that <Route exact> should default to true. Seems like it would be less surprising if it did.

Currently, when someone does:

<Route path="/" ... />
<Route path="/about" ... />

and the URL is /about, both routes render. Of course, there's a logical explanation for why they do, but I think it would be less surprising if they didn't.

I honestly can't think of any valid use cases that would break if we did make this change. Anyone?

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 12, 2017

Member

Bah, nevermind. My brain wasn't working when I wrote this issue.

Member

mjackson commented Apr 12, 2017

Bah, nevermind. My brain wasn't working when I wrote this issue.

@mjackson mjackson closed this Apr 12, 2017

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Apr 12, 2017

I found defaulting to exact useful because I wanted /foo/bar/some-garbage to 404 and that's really difficult if your routes aren't exact by default.

ForbesLindesay commented Apr 12, 2017

I found defaulting to exact useful because I wanted /foo/bar/some-garbage to 404 and that's really difficult if your routes aren't exact by default.

@mjackson mjackson reopened this Apr 12, 2017

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 12, 2017

Member

@ForbesLindesay Can you please say more? I'd like to hear about your use case.

Member

mjackson commented Apr 12, 2017

@ForbesLindesay Can you please say more? I'd like to hear about your use case.

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Apr 12, 2017

I want to make sure I always show an appropriate error page when the user tries to browse to a page that doesn't exist.

If I have something like:

function MyComponent() {
  return (
    <Switch>
      <Route path="/some-path" component={LeafComponent}/>
      <Route component={NoMatch}/>
    </Switch>
  );
}

then if the user accidentally types /some-path/some-non-existant-path into their browser, it will seem to match, and will display LeafComponent. If I then change the code in the future, there might be invalid links in the wild that I don't know about (because I didn't expect people to add arbitrary strings to the end of URLs).

This problem is made worse, by the fact that I won't notice it in normal operation. I rarely test the error pages, so I won't notice that there are lots of URLs that should error, but instead just display content. The solution is to add exact, but I may not notice the problem for a long time.

Conversely, if we had a hasChildren property, which defaulted to false, we could instantly throw an error when it was used incorrectly, because we can detect that the moment someone tries to render some child routes.

ForbesLindesay commented Apr 12, 2017

I want to make sure I always show an appropriate error page when the user tries to browse to a page that doesn't exist.

If I have something like:

function MyComponent() {
  return (
    <Switch>
      <Route path="/some-path" component={LeafComponent}/>
      <Route component={NoMatch}/>
    </Switch>
  );
}

then if the user accidentally types /some-path/some-non-existant-path into their browser, it will seem to match, and will display LeafComponent. If I then change the code in the future, there might be invalid links in the wild that I don't know about (because I didn't expect people to add arbitrary strings to the end of URLs).

This problem is made worse, by the fact that I won't notice it in normal operation. I rarely test the error pages, so I won't notice that there are lots of URLs that should error, but instead just display content. The solution is to add exact, but I may not notice the problem for a long time.

Conversely, if we had a hasChildren property, which defaulted to false, we could instantly throw an error when it was used incorrectly, because we can detect that the moment someone tries to render some child routes.

@timdorr

This comment has been minimized.

Show comment
Hide comment
@timdorr

timdorr Apr 12, 2017

Collaborator

Yeah, this would default to not matching on 404-ish URLs, which is the same behavior as <= 3.0.

Collaborator

timdorr commented Apr 12, 2017

Yeah, this would default to not matching on 404-ish URLs, which is the same behavior as <= 3.0.

@pshrmn

This comment has been minimized.

Show comment
Hide comment
@pshrmn

pshrmn Apr 12, 2017

Collaborator

FWIW, path-to-regexp's default end value is true*.

Switching to this would break a lot of code, but adding exact={false} is a pretty easy fix.

*Also, is the description for the end option really confusing for other people too?

When false the path will match at the beginning.

To me, that makes it sound like it starts matching anywhere when end is true, when really it just adds a $ to the end of the regex.

Collaborator

pshrmn commented Apr 12, 2017

FWIW, path-to-regexp's default end value is true*.

Switching to this would break a lot of code, but adding exact={false} is a pretty easy fix.

*Also, is the description for the end option really confusing for other people too?

When false the path will match at the beginning.

To me, that makes it sound like it starts matching anywhere when end is true, when really it just adds a $ to the end of the regex.

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Apr 12, 2017

I think exact is ok as a name, although I dislike having any options that default to true. I don't want to derail this with a naming bikeshed though unless we first agree on the default value.

ForbesLindesay commented Apr 12, 2017

I think exact is ok as a name, although I dislike having any options that default to true. I don't want to derail this with a naming bikeshed though unless we first agree on the default value.

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 12, 2017

Member

Agree this change would break a lot of code @pshrmn, but I can't shake the feeling that we chose the wrong default behavior just for the sake of child routes.

Just so everyone is clear what I mean by that, take e.g. our Basic example code. There's a <Route> that looks like this:

<Route path="/topics" component={Topics}/>

Inside the Topics component there are more routes, one of which matches the URL /topics/:topicId.

If we were to change the default behavior to try to match the entire URL, this pattern would break and we would need a new way to indicate that the parent route might have child <Route>s. Right now, we just assume that every route could possibly have child routes.

I also think it's very telling that (as @pshrmn pointed out) the path-to-regexp folks chose the opposite default to what we did. That right there should give us cause to think twice about this.

So right now I'm probably 75% convinced we chose the wrong default.

Member

mjackson commented Apr 12, 2017

Agree this change would break a lot of code @pshrmn, but I can't shake the feeling that we chose the wrong default behavior just for the sake of child routes.

Just so everyone is clear what I mean by that, take e.g. our Basic example code. There's a <Route> that looks like this:

<Route path="/topics" component={Topics}/>

Inside the Topics component there are more routes, one of which matches the URL /topics/:topicId.

If we were to change the default behavior to try to match the entire URL, this pattern would break and we would need a new way to indicate that the parent route might have child <Route>s. Right now, we just assume that every route could possibly have child routes.

I also think it's very telling that (as @pshrmn pointed out) the path-to-regexp folks chose the opposite default to what we did. That right there should give us cause to think twice about this.

So right now I'm probably 75% convinced we chose the wrong default.

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 12, 2017

Member

Also a HUGE +1 for being able to warn when someone uses the router incorrectly, as @ForbesLindesay suggested in #4958 (comment). Making child routes opt-in lets us do that.

Member

mjackson commented Apr 12, 2017

Also a HUGE +1 for being able to warn when someone uses the router incorrectly, as @ForbesLindesay suggested in #4958 (comment). Making child routes opt-in lets us do that.

@pshrmn

This comment has been minimized.

Show comment
Hide comment
@pshrmn

pshrmn Apr 13, 2017

Collaborator

Thinking about this more, I think that swapping to end=true would make more sense.

Also, because it is a breaking change, I agree that it would be a good opportunity to rethink the exact prop name. I like how clean using exact as a boolean attribute looks.

<Route exact path='/here' ... />

(side note, why were they removed from the jsx docs https://facebook.github.io/react/docs/jsx-in-depth.html#boolean-attributes?)

While it is superficial, it would be nice to maintain that with a defaults-to-false prop.

<Route exact={false} path='/here' ... />
<Route hasChildren path='/here' />
Collaborator

pshrmn commented Apr 13, 2017

Thinking about this more, I think that swapping to end=true would make more sense.

Also, because it is a breaking change, I agree that it would be a good opportunity to rethink the exact prop name. I like how clean using exact as a boolean attribute looks.

<Route exact path='/here' ... />

(side note, why were they removed from the jsx docs https://facebook.github.io/react/docs/jsx-in-depth.html#boolean-attributes?)

While it is superficial, it would be nice to maintain that with a defaults-to-false prop.

<Route exact={false} path='/here' ... />
<Route hasChildren path='/here' />
@timdorr

This comment has been minimized.

Show comment
Hide comment
@timdorr

timdorr Apr 13, 2017

Collaborator

partial might be better.

Collaborator

timdorr commented Apr 13, 2017

partial might be better.

@mehcode

This comment has been minimized.

Show comment
Hide comment
@mehcode

mehcode Apr 13, 2017

Contributor

👍 to startsWith

<Route startsWith path='/here' ... />
Contributor

mehcode commented Apr 13, 2017

👍 to startsWith

<Route startsWith path='/here' ... />
@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Apr 13, 2017

@pshrmn I think, long term, the react team might prefer to make <Route exact /> be a short hand for <Route exact={exact} /> rather than a shorthand for <Route exact={true} />, but that would be a big breaking change. This may explain them downplaying property shorthands in the docs.

@mehcode I quite like startsWith

ForbesLindesay commented Apr 13, 2017

@pshrmn I think, long term, the react team might prefer to make <Route exact /> be a short hand for <Route exact={exact} /> rather than a shorthand for <Route exact={true} />, but that would be a big breaking change. This may explain them downplaying property shorthands in the docs.

@mehcode I quite like startsWith

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 13, 2017

Member

Well it sounds like we're all leaning toward making this change. If we do, I'd like to:

  • deprecate <Route exact> (we don't need two props for the same thing)
  • recommend <Route somePropThatDefaultsToFalseAndIndicatesThisRouteHasChildren>

I'm also 👍 for <Route startsWith>, especially since that's already the name of a native String method. The less we have to explain the better :)

Member

mjackson commented Apr 13, 2017

Well it sounds like we're all leaning toward making this change. If we do, I'd like to:

  • deprecate <Route exact> (we don't need two props for the same thing)
  • recommend <Route somePropThatDefaultsToFalseAndIndicatesThisRouteHasChildren>

I'm also 👍 for <Route startsWith>, especially since that's already the name of a native String method. The less we have to explain the better :)

@pshrmn

This comment has been minimized.

Show comment
Hide comment
@pshrmn

pshrmn Apr 13, 2017

Collaborator

My only issue with startsWith is that as a boolean it doesn't sound right to me. I would expect a prop called startsWith it to take a string.

<Route startsWith={true} path='/test' ... />

I think that partial or parent make more sense in that respect.

<Route partial={true} path='/test' ... />
<Route parent={true} path='/test' ... />

At the end of the day though, as long as the documentation is there, it doesn't really matter to me what the name is.

Collaborator

pshrmn commented Apr 13, 2017

My only issue with startsWith is that as a boolean it doesn't sound right to me. I would expect a prop called startsWith it to take a string.

<Route startsWith={true} path='/test' ... />

I think that partial or parent make more sense in that respect.

<Route partial={true} path='/test' ... />
<Route parent={true} path='/test' ... />

At the end of the day though, as long as the documentation is there, it doesn't really matter to me what the name is.

@mjackson mjackson changed the title from Default <Route exact> to true to Deprecate <Route exact>, match path exactly by default Apr 20, 2017

@merrywhether

This comment has been minimized.

Show comment
Hide comment
@merrywhether

merrywhether Apr 20, 2017

Another option would be to change things so that path=... is always an exact match and something like pathStartsWith=... or partialPath=... be inexact. Then you don't need the boolean for signaling at all.

The downside is that would lead to more code breakage.

merrywhether commented Apr 20, 2017

Another option would be to change things so that path=... is always an exact match and something like pathStartsWith=... or partialPath=... be inexact. Then you don't need the boolean for signaling at all.

The downside is that would lead to more code breakage.

@mehcode

This comment has been minimized.

Show comment
Hide comment
@mehcode

mehcode Apr 20, 2017

Contributor

Yet another option is drop this "partial" idea and do something more (pseudo-)standard:

// Exact match
<Route path='/document' ... />

// Exact match with parameter
<Route path='/document/:param' ... />

// "Starts With" Match
<Route path='/document/*' ... />

This matches expectations set by the community:


The more I think on this, the more I like it. It's simple. It's intuitive.

Contributor

mehcode commented Apr 20, 2017

Yet another option is drop this "partial" idea and do something more (pseudo-)standard:

// Exact match
<Route path='/document' ... />

// Exact match with parameter
<Route path='/document/:param' ... />

// "Starts With" Match
<Route path='/document/*' ... />

This matches expectations set by the community:


The more I think on this, the more I like it. It's simple. It's intuitive.

@pshrmn

This comment has been minimized.

Show comment
Hide comment
@pshrmn

pshrmn Apr 20, 2017

Collaborator

Relying on the path would not work.

Given the pathname /document/one/two, the route:

<Route path='/document/*' ... />

The generated match.url would be /document/testing. However, only the parent part of the path should be included in match.url (i.e. /document or maybe /document/).

/document/* also would fail to match the pathname /document.

Collaborator

pshrmn commented Apr 20, 2017

Relying on the path would not work.

Given the pathname /document/one/two, the route:

<Route path='/document/*' ... />

The generated match.url would be /document/testing. However, only the parent part of the path should be included in match.url (i.e. /document or maybe /document/).

/document/* also would fail to match the pathname /document.

@mehcode

This comment has been minimized.

Show comment
Hide comment
@mehcode

mehcode Apr 21, 2017

Contributor

/document/* also would fail to match the pathname /document.

We control this here. We can either make the trailing slash unimportant (not sure what react-router's stance on that is) or allow /document* which exists and works just as well in all frameworks I linked.


The generated match.url would be /document/testing. However, only the parent part of the path should be included in match.url (i.e. /document or maybe /document/).

I don't follow your assertions. We can control this. To me, it's far more intuitive for match.url to be /document or /document/ (depending on what is before the * or depending on what react-router's stance on the importance of trailing slashes).

Contributor

mehcode commented Apr 21, 2017

/document/* also would fail to match the pathname /document.

We control this here. We can either make the trailing slash unimportant (not sure what react-router's stance on that is) or allow /document* which exists and works just as well in all frameworks I linked.


The generated match.url would be /document/testing. However, only the parent part of the path should be included in match.url (i.e. /document or maybe /document/).

I don't follow your assertions. We can control this. To me, it's far more intuitive for match.url to be /document or /document/ (depending on what is before the * or depending on what react-router's stance on the importance of trailing slashes).

@mjackson

This comment has been minimized.

Show comment
Hide comment
@mjackson

mjackson Apr 21, 2017

Member

@mehcode We don't actually control (or want to control) this anymore. As of v4 we delegate all pattern matching to path-to-regexp. Doing something different than what they do would just mean we have to explain ourselves.

Member

mjackson commented Apr 21, 2017

@mehcode We don't actually control (or want to control) this anymore. As of v4 we delegate all pattern matching to path-to-regexp. Doing something different than what they do would just mean we have to explain ourselves.

@ryanflorence

This comment has been minimized.

Show comment
Hide comment
@ryanflorence

ryanflorence Jun 7, 2017

Contributor

Alright, I'm convinced we got exact wrong here (thanks for bearing w/ my idiocy in Austin @mjackson, as usual).

When I first added exact in the v4 rewrite I viewed it as nothing more than the replacement for IndexRoute. Since index routes were less common than routes, the default was true. However, the v4 API, especially with <Switch> makes it harder to use/understand than we expected.

I'd like to do partial (I actually was debating using exact vs. partial when exact was first added), but I'm fine with hasChildren too. One references the algorithm (match partially) the other the developer's intent ("I'm gonna nest some routes"). It's a dumb reason, but I like partial because it's shorter, no camelCase, so it's prettier in the docs (🤦🏽‍♂️).

Providing a smooth upgrade on a default boolean that flips the other way is kind of a pain though.

In order to do the react-like thing which is:

  1. upgrade
  2. get warnings
  3. fix warnings
  4. upgrade
  5. app works the same

I think we'd need to

  1. ship a 4.x+1 that warns you should add an exact prop on every route, no more default to true.
  2. ship 5.0 that warns to remove all exact props (😂) and only add partial when needed, which, as @ForbesLindesay points out, we can actually know their intent now (!!!) and give a great console message ("Hi! It looks like you're trying to nest routes, add a partial prop to the parent route so")

So developer intent? hasChildren, isParent, hasChildRoutes
Or algorithm branch? partial, matchPartially, startsWith

Contributor

ryanflorence commented Jun 7, 2017

Alright, I'm convinced we got exact wrong here (thanks for bearing w/ my idiocy in Austin @mjackson, as usual).

When I first added exact in the v4 rewrite I viewed it as nothing more than the replacement for IndexRoute. Since index routes were less common than routes, the default was true. However, the v4 API, especially with <Switch> makes it harder to use/understand than we expected.

I'd like to do partial (I actually was debating using exact vs. partial when exact was first added), but I'm fine with hasChildren too. One references the algorithm (match partially) the other the developer's intent ("I'm gonna nest some routes"). It's a dumb reason, but I like partial because it's shorter, no camelCase, so it's prettier in the docs (🤦🏽‍♂️).

Providing a smooth upgrade on a default boolean that flips the other way is kind of a pain though.

In order to do the react-like thing which is:

  1. upgrade
  2. get warnings
  3. fix warnings
  4. upgrade
  5. app works the same

I think we'd need to

  1. ship a 4.x+1 that warns you should add an exact prop on every route, no more default to true.
  2. ship 5.0 that warns to remove all exact props (😂) and only add partial when needed, which, as @ForbesLindesay points out, we can actually know their intent now (!!!) and give a great console message ("Hi! It looks like you're trying to nest routes, add a partial prop to the parent route so")

So developer intent? hasChildren, isParent, hasChildRoutes
Or algorithm branch? partial, matchPartially, startsWith

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Jun 7, 2017

I wonder if you could get away with:

  1. Ship a 4.x+1 that warns you to add "partial" to every rout that we detect has children.
  2. Ship a 5.0 that warns you to remove "exact" as it's now the default.

It's still two steps, but at least the steps aren't "Hey, you need to add this", "Joke, you can remove it again now".

Personally I still prefer hasChildren or hasChildRoutes for the clear expression of intent, but I have no big objection to partial.

ForbesLindesay commented Jun 7, 2017

I wonder if you could get away with:

  1. Ship a 4.x+1 that warns you to add "partial" to every rout that we detect has children.
  2. Ship a 5.0 that warns you to remove "exact" as it's now the default.

It's still two steps, but at least the steps aren't "Hey, you need to add this", "Joke, you can remove it again now".

Personally I still prefer hasChildren or hasChildRoutes for the clear expression of intent, but I have no big objection to partial.

@ryanflorence

This comment has been minimized.

Show comment
Hide comment
@ryanflorence

ryanflorence Jun 7, 2017

Contributor

@ForbesLindesay Then in 5.0 do we log "Hey, you used false, that's the default, you can remove it"?

Contributor

ryanflorence commented Jun 7, 2017

@ForbesLindesay Then in 5.0 do we log "Hey, you used false, that's the default, you can remove it"?

@ryanflorence

This comment has been minimized.

Show comment
Hide comment
@ryanflorence

ryanflorence Jun 7, 2017

Contributor

Another option is to just export a new Route

import { Route } from 'react-router-dom/next'

Then warn about usage of the current Route completely. Then in 5.0 we remove /next.

I think I like that a lot, actually. Let's us keep the code cleaner in the /next and just a basic warning in the current Route, instead of branching and warning and making a mess of things.

Contributor

ryanflorence commented Jun 7, 2017

Another option is to just export a new Route

import { Route } from 'react-router-dom/next'

Then warn about usage of the current Route completely. Then in 5.0 we remove /next.

I think I like that a lot, actually. Let's us keep the code cleaner in the /next and just a basic warning in the current Route, instead of branching and warning and making a mess of things.

@pshrmn

This comment has been minimized.

Show comment
Hide comment
@pshrmn

pshrmn Jun 7, 2017

Collaborator

I'd say that the more descriptive the prop name is, the better.

  • hasChildRoutes requires basically no explanation. Perhaps Nested could be substituted for Child, but either way it should be pretty clear what this means.
  • hasChildren is similar, although children is already a React concept, so it is maybe slightly less clear?
  • isParent is basically the same as the above and shorter. This could also just be parent.
  • partial makes sense to me, but I think that it would require more documentation to explain what it means to people that are learning React Router.
<Route hasChildRoutes path='/who' component={Who} />
<Route hasChildren path='/what' component={What} />
<Route isParent path='/where' component={Where} />
<Route parent path='/when' component={When} />
<Route partial path='/why' component={Why} />

I like hasChildRoutes/isParent/parent the most. If I had to pick one, I would say parent for the same dumb reason of it being shorter.

Collaborator

pshrmn commented Jun 7, 2017

I'd say that the more descriptive the prop name is, the better.

  • hasChildRoutes requires basically no explanation. Perhaps Nested could be substituted for Child, but either way it should be pretty clear what this means.
  • hasChildren is similar, although children is already a React concept, so it is maybe slightly less clear?
  • isParent is basically the same as the above and shorter. This could also just be parent.
  • partial makes sense to me, but I think that it would require more documentation to explain what it means to people that are learning React Router.
<Route hasChildRoutes path='/who' component={Who} />
<Route hasChildren path='/what' component={What} />
<Route isParent path='/where' component={Where} />
<Route parent path='/when' component={When} />
<Route partial path='/why' component={Why} />

I like hasChildRoutes/isParent/parent the most. If I had to pick one, I would say parent for the same dumb reason of it being shorter.

@CrocoDillon

This comment has been minimized.

Show comment
Hide comment
@CrocoDillon

CrocoDillon Jun 8, 2017

I definitely agree on the previous comment that because children is already a React concept, hasChildren would be too confusing. I also like short names.

  • parent is very clear, implies that it has child routes/paths
  • partial is less clear since a leaf route depending on match.url can be considered partial too, e.g. <Route path={match.url + '/foo'} ... />. But it's clear enough anyway.
  • Speaking about tree terminology, what about root (might not be perfect since it doesn't have to be the actual root) or branch (less obvious than root)?

CrocoDillon commented Jun 8, 2017

I definitely agree on the previous comment that because children is already a React concept, hasChildren would be too confusing. I also like short names.

  • parent is very clear, implies that it has child routes/paths
  • partial is less clear since a leaf route depending on match.url can be considered partial too, e.g. <Route path={match.url + '/foo'} ... />. But it's clear enough anyway.
  • Speaking about tree terminology, what about root (might not be perfect since it doesn't have to be the actual root) or branch (less obvious than root)?
@sercanov

This comment has been minimized.

Show comment
Hide comment
@sercanov

sercanov Jul 22, 2017

I certainly think so, was felt awkward when both parent and child component rendered at the same time.

sercanov commented Jul 22, 2017

I certainly think so, was felt awkward when both parent and child component rendered at the same time.

@dbackeus

This comment has been minimized.

Show comment
Hide comment
@dbackeus

dbackeus Jul 27, 2017

Personally prefer the algorithmically based prop names. hasChildren etc can be confused with non route-related children while partial is immediately associated with the routing. Also as mentioned it is shorter / cleaner which tips the scale in a tie breaker situation.

dbackeus commented Jul 27, 2017

Personally prefer the algorithmically based prop names. hasChildren etc can be confused with non route-related children while partial is immediately associated with the routing. Also as mentioned it is shorter / cleaner which tips the scale in a tie breaker situation.

@rnYulianto

This comment has been minimized.

Show comment
Hide comment
@rnYulianto

rnYulianto Aug 26, 2017

Hello im new to react router or react, i have 2 router with param

<Route path='/myhome/:param' ... />
<Route path='/myhome/:param/:param2' ... />

How to match one route but not both?

rnYulianto commented Aug 26, 2017

Hello im new to react router or react, i have 2 router with param

<Route path='/myhome/:param' ... />
<Route path='/myhome/:param/:param2' ... />

How to match one route but not both?

@sorahn

This comment has been minimized.

Show comment
Hide comment
@sorahn

sorahn Aug 26, 2017

@rnYulianto if you wrap them with a <Switch> tag it will only ever render one route.

sorahn commented Aug 26, 2017

@rnYulianto if you wrap them with a <Switch> tag it will only ever render one route.

@dnaploszek

This comment has been minimized.

Show comment
Hide comment
@dnaploszek

dnaploszek Oct 19, 2017

Is this thread dead? I had these thoughts exactly, and was pointed here from there.
hasChildren sounds great 👍

dnaploszek commented Oct 19, 2017

Is this thread dead? I had these thoughts exactly, and was pointed here from there.
hasChildren sounds great 👍

@timdorr

This comment has been minimized.

Show comment
Hide comment
@timdorr

timdorr Oct 25, 2017

Collaborator

See #5612

Collaborator

timdorr commented Oct 25, 2017

See #5612

@alex996

This comment has been minimized.

Show comment
Hide comment
@alex996

alex996 Oct 26, 2017

@mjackson Why wasn't your brain working? I find it very counter-intuitive that the router doesn't perform exact matching by default. What's the argument against this idea?

alex996 commented Oct 26, 2017

@mjackson Why wasn't your brain working? I find it very counter-intuitive that the router doesn't perform exact matching by default. What's the argument against this idea?

@r3wt

This comment has been minimized.

Show comment
Hide comment
@r3wt

r3wt Dec 27, 2017

What's the argument against this idea?

@alex996 it was completely arbitrary from what i can tell. maybe an exercise in discovering what not to do.

r3wt commented Dec 27, 2017

What's the argument against this idea?

@alex996 it was completely arbitrary from what i can tell. maybe an exercise in discovering what not to do.

@internationalhouseofdonald

This comment has been minimized.

Show comment
Hide comment
@internationalhouseofdonald

internationalhouseofdonald Jan 5, 2018

This ended up happening, didn't it? I am using React ^16.0.0 and I'm experiencing trouble when I use exact. Everything works fine without it. No Whoops404 pages but that's something that could be fixed with a server-side redirect. Why apologize with a "Whoops 404!" page when you can just fix the problem?

internationalhouseofdonald commented Jan 5, 2018

This ended up happening, didn't it? I am using React ^16.0.0 and I'm experiencing trouble when I use exact. Everything works fine without it. No Whoops404 pages but that's something that could be fixed with a server-side redirect. Why apologize with a "Whoops 404!" page when you can just fix the problem?

@timdorr

This comment has been minimized.

Show comment
Hide comment
@timdorr

timdorr Jan 5, 2018

Collaborator

@internationalhouseofdonald Nothing has changed. No code has been committed, or even PR'ed at this point.

Collaborator

timdorr commented Jan 5, 2018

@internationalhouseofdonald Nothing has changed. No code has been committed, or even PR'ed at this point.

@ForbesLindesay

This comment has been minimized.

Show comment
Hide comment
@ForbesLindesay

ForbesLindesay Jan 13, 2018

@internationalhouseofdonald How you handle the "no route matched" is a separate issue, it would still be up to the maintainer to choose to 404 or redirect, but without exact, you don't get the opportunity to do either.

ForbesLindesay commented Jan 13, 2018

@internationalhouseofdonald How you handle the "no route matched" is a separate issue, it would still be up to the maintainer to choose to 404 or redirect, but without exact, you don't get the opportunity to do either.

@medv

This comment has been minimized.

Show comment
Hide comment
@medv

medv Jun 5, 2018

non-exact by default is better for my apps, because generally I have a few top-level routes that catch-all and then several catch-all routes as children and in turn childrens' children. I would imagine it's a common pattern of using this package because it allows nested components to behave as their own views that could be taken entirely out of context of the app and still work (given a root match url of the parent view component it's mounted in, or string url if it's mounted standalone). Children routes define their own path with params, so the parent view has a clean match to pass into these children for their own subnavs and is unaware of any functionality other than mounting child views.

exact/!exact default choice is arbitrary in the first place, so at least it's fewer lines of code the way it is right now. Across 6 apps, about one in ten routes are exact for me. I could see how I would need more exact routes if I had to state my routes from the get-go (outdated pattern for my liking). just 2c from what I experience.

image

TripView fetches by id from graphql if given id, or fetches last trip and redirects to trip/id, and all subnav+subviews within are mounted as standalone Routes or within a Switch. They are given TripView.match / TripView.location to extract params/location.state but also form its own subnav/subviews that might have their own params/location.state

image

This way the original top-level route still matches, and we also don't care that the sub-route is non-exact because it might have its own sub-views using the same pattern. You can take any piece of the chain and mount it in a completely separate app or Storybook story and pass them dummy parent matches, or written to just support base_url: string prop or similar. You can also remove any children routes and not affect the functionality of the containing component or route definitions anywhere above in the chain.

medv commented Jun 5, 2018

non-exact by default is better for my apps, because generally I have a few top-level routes that catch-all and then several catch-all routes as children and in turn childrens' children. I would imagine it's a common pattern of using this package because it allows nested components to behave as their own views that could be taken entirely out of context of the app and still work (given a root match url of the parent view component it's mounted in, or string url if it's mounted standalone). Children routes define their own path with params, so the parent view has a clean match to pass into these children for their own subnavs and is unaware of any functionality other than mounting child views.

exact/!exact default choice is arbitrary in the first place, so at least it's fewer lines of code the way it is right now. Across 6 apps, about one in ten routes are exact for me. I could see how I would need more exact routes if I had to state my routes from the get-go (outdated pattern for my liking). just 2c from what I experience.

image

TripView fetches by id from graphql if given id, or fetches last trip and redirects to trip/id, and all subnav+subviews within are mounted as standalone Routes or within a Switch. They are given TripView.match / TripView.location to extract params/location.state but also form its own subnav/subviews that might have their own params/location.state

image

This way the original top-level route still matches, and we also don't care that the sub-route is non-exact because it might have its own sub-views using the same pattern. You can take any piece of the chain and mount it in a completely separate app or Storybook story and pass them dummy parent matches, or written to just support base_url: string prop or similar. You can also remove any children routes and not affect the functionality of the containing component or route definitions anywhere above in the chain.

@gnapse

This comment has been minimized.

Show comment
Hide comment
@gnapse

gnapse Sep 10, 2018

To makes matters worse, you cannot even think of making your own Route wrapper that would put exact={true} in your routes, and then use that inside a Switch. Because the Switch relies on traversing its direct children list and finding out if each one of them is exact or not.

What if the Switch had a flag to this end? Like <Switch exact={true}>...</Switch> in which case it will assume each child route is exact? There must be something wrong with this idea, or maybe someone already posted it, because I find it deceivingly simple, yet I failed to found that suggestion in the discussion above, and I hardly can think I'm the first that came up with it. So what's wrong with it? Because I'm considering creating such a switch in my app.

gnapse commented Sep 10, 2018

To makes matters worse, you cannot even think of making your own Route wrapper that would put exact={true} in your routes, and then use that inside a Switch. Because the Switch relies on traversing its direct children list and finding out if each one of them is exact or not.

What if the Switch had a flag to this end? Like <Switch exact={true}>...</Switch> in which case it will assume each child route is exact? There must be something wrong with this idea, or maybe someone already posted it, because I find it deceivingly simple, yet I failed to found that suggestion in the discussion above, and I hardly can think I'm the first that came up with it. So what's wrong with it? Because I'm considering creating such a switch in my app.

@mjackson mjackson added this to the 5.0 milestone Sep 12, 2018

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