remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.11k stars 10.31k forks source link

The null match problem #4560

Closed pshrmn closed 7 years ago

pshrmn commented 7 years ago

I have been thinking a lot about https://github.com/ReactTraining/react-router/pull/4539#discussion_r101928586 and what the correct behavior for resolving when match === null should be. This is only an issue with the children prop when the <Route> also has a path prop. When we use component, render, or there is no path, there is always* a parent match to resolve with.

This isn't exclusive to #4539 and #4459. The problem also exists with users who are manually "resolving" using string concatenation.

My conclusion is that relative paths inside of a <Route path={path} children> should be considered unsupported. When match is null, we just do not have the necessary information to resolve a path.

*Right now, there is not a root match object, so this statement isn't technically true. However, one could fairly trivially be added. As it stands context.router.match is undefined, not null. matchPath could differentiate between an undefined and a null parentMatch, but the tendency to use condition == null throughout this project makes me think that having a root match is preferable.

Below I have outlined a number of possible attempts at resolving when match = null and why they do not work.

Case 0: A Match A control example. When the <Route> with a children prop matches, resolving will work as expected.

// location.pathname = '/beverages/kefir'
<Route path='/beverages' children={({ match }) => (
  // match = { path: '/beverages', url: '/beverages'

  <Route path='tea' component={Tea} /> // /beverages/tea
  <Route path={`${match.url}/coffee`} component={Coffee} /> // /beverages/coffee

  <Link to='tea'>Tea</Link> // /beverages/tea
  <Link to={`${match.url}/coffee`}>Coffee</Link> // /beverages/coffee
)} />

Case 1: No Match One solution (which I had been using, but now realize is wrong) would be to resolve using the root (/) when match is null. This results in an incorrect path with automatic resolving and TypeErrors with manual resolving.

// location.pathname = '/food'
<Route path='/beverages' children={({ match }) => (
  // match = null

  <Route path='tea' component={Tea} /> // /tea
  <Route path={`${match.url}/coffee`} component={Coffee} /> // TypeError

  <Link to='tea'>Tea</Link> // /tea
  <Link to={`${match.url}/coffee`}>Coffee</Link> // TypeError
)} />

Case 2: Parent Match Another approach would be to inherit the parent match. This approach would require adding an extra property to match that indicates that it isn't a "real" match (match.matches = false). It gets rid of the TypeErrors, but still results in incorrect URLs.

// location.pathname = '/food'
// match = { path: '/', url: '/' }
<Route path='/beverages' children={({ match }) => (
  // match = { path: '/', url: '/', matches: false }

  <Route path='tea' component={Tea} /> // /tea
  <Route path={`${match.url}/coffee`} component={Coffee} /> // /coffee

  <Link to='tea'>Tea</Link> // /tea
  <Link to={`${match.url}/coffee`}>Coffee</Link> // /coffee
)} />

Case 3: Join with Parent Match The above could be expanded on by joining the path of the route being matched with its parent match's url and path. This will not work because the route's path might include parameters. That is acceptable for resolving <Route> paths, but fails when resolving anything that expects an actual URL.

// location.pathname = '/food'
// match = { path: '/', url: '/' }
<Route path='/beverages/:temp' children={({ match }) => (
  // match = { path: '/beverages/:temp', url: '/beverages/:temp', matches: false } 

  <Route path='tea' component={Tea} /> // /beverages/:temp/tea
  <Route path={`${match.url}/coffee`} component={Coffee} /> // /beverages/:temp/coffee

  <Link to='tea'>Tea</Link> // /beverages/:temp/tea
  <Link to={`${match.url}/coffee`}>Coffee</Link> // /beverages/:temp/coffee
)} />

Working Solution As I stated above, this is an information problem. When match = null, we cannot properly resolve a path. In this situation, the only way to guarantee that you get the correct URL is to use absolute paths.

// location.pathname = '/food'
// match = { path: '/', url: '/' }
<Route path='/beverages/:temp' children={({ match }) => (
  // match = null

  <Route path='/beverages/hot/tea' component={Tea} />
  <Route path='/beverages/cold/tea' component={IcedTea} />
  <Route path='/beverages/hot/coffee' component={Coffee} />
  <Route path='/beverages/cold/coffee' component={IcedCoffee} />

  <Link to='/beverages/hot/tea'>Tea</Link>
  <Link to='/beverages/cold/tea'>Iced Tea</Link>
  <Link to='/beverages/hot/coffee'>Coffee</Link>
  <Link to='/beverages/cold/coffee'>Iced Coffe</Link>
)} />

Generally speaking, I don't think that <Route path={path} children> will be used that often, especially for actual path matching. The one situation that comes to mind is "active" matching, like with the <NavLink>. In that situation, the location should be resolved prior to passing it to the <Route> because a location.pathname supports more complex relative paths than path does (<Link to='..'>Parent</Link> is valid, <Route path='..'> is not).

ryanflorence commented 7 years ago

Is this a true statement: These problems exist only when passing a relative path?

pshrmn commented 7 years ago

This problem exists if you need to use the parent match to build the URL, but it is null. That can either be through built-in resolving or manually concatenating a path with match.url. I'm not sure if you consider the latter case to be passing a relative path.