Fix pathless route's match when parent is null

[pshrmn]

This moves the logic for getting the match object for a pathless <Route> matchPath. When automatic path resolving is added, matchPath will need access to the parent match, so we will need to pass that argument to matchPath eventually anyways.

When a parent match is null, a match object with default values is returned by matchPath. This match object should not be relied upon for resolving paths, but it will allow a <Route> that uses the component or render prop to render.

Fixes #4695

[timdorr]

LGTM. What was the previous reasoning for not doing this?

[pshrmn]

Change happened in f90d753. I think that it was because we can detect a pathless route inside of the <Route>'s render method (<Switch> too) and since we already have access to the parent match there, we could skip calling matchPath.

This could still be done in the render methods, but we would still need to handle null parent matches, so I think that it simpler to have the logic for this in matchPath.

I'm not actually sure what the match object should be when the parent match is null. It is basically a filler to get pathless <Route>s to work. Attempting to resolve using it would fail, so it doesn't really matter.

Edit: Also, apparently if you rebase and force update a commit that references an issue, GitHub doesn't forget 🤦‍♂️

[timdorr]

It would seem to need to be recursive, going up the tree until it runs out of ancestors. That would mean the final ancestor would have the default path of /.

[pshrmn]

Having match objects store a reference to their parent match has been discussed in #4459, and it will be useful for resolving double dot relative links. That would still break when a parent match is null, though, because there would be no way to reference that match's parent match.

I sort of went into a train of thought/rant about it in #4560, but even if we could access an ancestor match prior to the null match, we would end up with unexpected behavior. Perhaps we could take inspiration from React and set the match object to be:

{ url: '/_FAKE_MATCH_DO_NOT_ATTEMPT_TO_RESOLVE_USING_THIS_OR_YOU_WILL_BE_DISAPPOINTED' }

[pshrmn]

Okay, I actually did this. So far, the only use for match.url that I have seen* has been with concatenating paths using the parent route. When the parent match is null, we cannot provide an accurate parent match.url. Having the error explicitly defined in the URL like this will make it more obvious to anyone who attempts to resolve using it that something is not right with what they are trying to do.

This isn't a make or break change for me, but I think that if we try to use some other value like root or the current URL, we will end up with a lot of people asking why they are getting the wrong URL when they attempt to resolve.

*It can actually be useful as a transition key for animated switches, but it isn't actually accessible with the <Switch> component, so I'm ignoring that here.

[pshrmn]

I think that I might actually prefer leaving match null for pathless <Route>s when their parent match is null.

The only real change that this would require is that withRouter would have to use children instead of render. Either way, it is just calling the prop as a function.

The downside is that any user that uses a pathless <Route> inside of <Route children> would have to use the children prop to render.

<Route path='/some-place' children={() => (
  // this would not work
  <Route render={() => ...}/>
  // this would
  <Route children={() => ...}/>
)}/>

When it comes to actually resolving paths, we could then reliably add a warning whenever someone attempts to resolve when the parent match is null.

warning(
  !(isRelative && parent == null),
  'You are attempting to use a relative path when the parent match is null. This most likely ' +
  'occurred because a parent <Route> uses the children prop and its path does not match ' +
  'the current location\'s pathname'
)

[pshrmn]

Just bumping this for consideration when you have the time @mjackson @ryanflorence. No rush, but it is holding up the relative routing PRs.

[ryanflorence]

I have some loose thoughts but nothing I can articulate yet, hopefully soon!

[wmertens]

So is there a way this situation can arise that is actually a sane situation? It seems to me that if you are using relative paths in a react tree that is not supposed to be matching, you are doing something wrong?

In which case, relative Routes that get a null match should print a console warning (in dev mode) and not render anything?

[roboxdev]

I've faced an issue that my component
export default withRouter(connect(mapStatetoProps)(MyComponent)) was not rendered at all.
I'm new to react router but I guess my problem related to this PR. Pathless routes are broken.

Workaround that works for me:

const ConnectedMyComponent = connect(mapStatetoProps)(MyComponent);
export default () => <Route path='*' component={ConnectedMyComponent}/>;

[wmertens]

@roboxv can you describe your tree up to your component? Indeed, withRouter is implemented with render (https://github.com/ReactTraining/react-router/blob/ef2f8f90d6243d007b32f262e1215a5d40f0577b/packages/react-router/modules/withRouter.js#L13) but that should work…

[roboxdev]

@wmertens is it okay if I just show you my files?

From top to bottom:
index
App
Page → MainBlock → MainWithSidebars → RightSidebar
SidebarCart

[wmertens]

@roboxv
Use a <Switch> here with a pathless second <Route/>: https://github.com/roboxv/fuchtard/blob/8b17b848142af4ca9c6bb6f9be08d9c956ebeba4/frontend/src/components/Page.jsx#L61-L66

That way, the children have a match in context and this issue does not apply.

@pshrmn Is there anything blocking this issue from being merged? If so, what would be the best way for me to use your branch in my app in the meantime?

[pshrmn]

@wmertens I don't think that there is anything blocking this. I'm pretty sure that @ryanflorence and @mjackson are just really busy right now.

There isn't really a good way to install this branch. You could probably clone the repo from my fork, checkout the correct branch, and npm link it. Not the prettiest solution, but it should work.

[wmertens]

@ryanflorence is there anything I can do to convince you to click that [Merge] button? :)

[nodkz]

@ryanflorence should this PR be merged or not?
Tnx.

[wmertens]

@pshrmn the branch got stale again - maybe this time it will get merged! 😅

[trygveaa]

Sorry for asking, but is there any progress in reviewing this?

I'm running into an issue when using react-transition-group together with routes. When using a Route with children for the transition group (because the TransitionGroup always has to be mounted in order for the transition to work), components using withRouter inside of this doesn't render anything when the path doesn't match anymore, which is the case when the exit animation occurs.

An example demonstrating the issue is here: https://github.com/trygveaa/react-router-pathless-route-issue. A modal is shown when going to /test. When going back, the text inside the modal disappears before the close animation for the modal finishes.

This PR fixes the issue. Any other suggestions on how to fix it within my code would also be appreciated though.

[dwick]

I'm also having the issues described in #5141.

[wmertens]

[timdorr]

Closing in favor of #5964