React-router: Miss is broken

[pshrmn]

In alpha 6 the big refactor, the <Miss> component is broken. This happens because when the location subscriber resets the match count when it is called, the components do not re-register themselves, so the <Miss> component sees the matchCount as 0.

[timdorr]

Have you tried this against the latest v4 branch. @ryanflorence did a big refactor, so this might not be an issue now.

[pshrmn]

The issue is with the latest branch. I'll dig into it a little bit later, but I'm failing with the most basic example possible.

import React from 'react'
import { render } from 'react-dom'

import { BrowserRouter as Router, Match, Link, Miss } from 'react-router'

const Home = props => (
  <div>Home</div>
)

const Nav = props => (
  <div>
    <Link to='/about'>About</Link>
    <Link to='/other'>Other</Link>
  </div>
)

const About = props => (
  <div>About</div>
)

const Missed = () => (
  <div>Miss</div>
)

const App = () => (
  <Router>
    <div>
      <Nav />
      <Match pattern='/' exactly component={Home} />
      <Match pattern='/about' component={About} />
      <Miss component={Missed} />
    </div>
  </Router>
)

export default App

[pshrmn]

My original thought that the components do not re-register themselves was not correct. Rather, it has to do with <Match> components' subscriber functions' order of execution. This does not affect the components rendered for the initial location.

<Match> components functions are being subscribed in the componentDidMount function, which means that child <Match> components's subscriber functions will be included in the <StaticRouter>'s subscribers array ahead of their parent's subscriber function.

Now, in the subscriber function, the <Match> resets the number of child matches that are registered with it. The subscriber function always registers itself with its parent <Match> (updating the parent <Match>'s matchCount). Unfortunately, because the child subscriber functions are called before the parent's subscriber function, this ends up meaning that any registering in the subscriber function gets wiped out when the parent's subscriber function is called and the parent's matchCount is reset to 0.

At this point, nothing has actually re-rendered but the <Match> and <Miss> components are all set to re-render. When the <Miss> checks if it should render, it looks at its parent <Match>'s matchCount, and only; renders if the value if 0. As stated before, the value will always be 0, so the <Miss> will always render.

[r3verser]

Same thing, have tried to make auth from docs example, just refactored const MatchWhenAuthorized to class structure, and Miss starts firing every time, so two components renders - matched one and Miss. Is there any fix? [[email protected]]

<Match exactly pattern="/" component={Home}/>
<Match pattern="/login" component={Login}/>
<MatchAuth pattern="/dashboard" component={Dashboard}/>
<Miss render={({location}) => (
    <div>Nothing matched {location.pathname}.</div>
)}/>

class MatchAuth extends Component {
  render() {
    let { component: Component, ...rest } = this.props;
    return (
      <Match {...rest} render={
          (props) => (
            this.props.store.isAuthenticated ? (<Component {...props}/>) : (<Redirect to={{pathname: '/login',state: {from: props.location}}}/>)
          )
        }
      />
    )
  }
}

[jaydenseric]

Interestingly:

render(
  <Page>
    <BrowserRouter>
      <Match pattern='/' exactly component={Home} />
      <Miss component={ErrorPage} />
    </BrowserRouter>
  </Page>,
  document.getElementById('root')
)

Results in Uncaught Error: React.Children.only expected to receive a single React element child.. This happens when adding more than one Match or Miss; replacing the Miss with <Match pattern='/test' component={Test} /> results in the same error.

Adding a container element under BrowserRouter seems to fix the issues:

render(
  <Page>
    <BrowserRouter>
      <div>
        <Match pattern='/' exactly component={Home} />
        <Miss component={ErrorPage} />
      </div>
    </BrowserRouter>
  </Page>,
  document.getElementById('root')
)

[dtipson]

Seeing the same thing here & when using the MatchWhenAuthorized component pattern as well (so maybe something racey caused by that using subcomponents).

I actually may be seeing a similar bit of weirdness with componentWillMount and componentWillUnmount on the subcomponents of what are two mutually exclusive routes.

When a first, currently active route (but listed second in the markup) goes invalid and unmatches at the same time that the second one (but listed first) matches, the componentWillUnmount effects in the subcomponents from the original matched route end up firing AFTER the componentWillMount effects of the subcomponents in the newly matched one. And if both of those overlapping mismatched methods modify, say, the same place of a redux store, then the state will end up as the now unmounted component intended rather than as the newly mounted component would have it.

Reverse the markup order of the two Matches (and start on the other one from before and navigate back), and the oddity reverses as well.

Now obviously one problem here is causing side-effects in componentWillMount when per React docs those should go in componentDidMount (though that leads to double rendering). But it's an unintuitive a gotcha that could be at work here in Match/Miss, maybe? Here's a pen of the effect, http://codepen.io/dtipson/pen/dOKYvr?editors=1111 (open the console, note how the switch from B to A works).

Also note that this is a factor when you list HOCs parallel to each other and each decides whether to render their subcomponents or not. If they're mutually exclusive (that is, some prop decides which to display in a ternary or logical stack), the weirdness vanishes.

[billscheidel]

Is there a build of 4.0 that doesn't exhibit these issues? Does going back to alpha-5 work as expected?

[v12]

@billscheidel no, the issue is present in alpha 5 as well.

[pshrmn]

Mostly semantics, but I just realized that alpha 6 released before the refactor came out and there hasn't been a release for the new code. When I said that <Miss> is broken in alpha 6, I guess that what I actually meant is that it is broken in published but unreleased code.

[ryancole]

I may be encountering this issue.

I see that the current state of v4 branch differs greatly from what the 4.0.0-alpha.6 release had. I'm unsure of how to clone v4 and try it out, because createServerRenderContext was removed and based on the code changes it's unclear as to how to proceed without it. The tests seem to just use an empty object. I tried that, but then began encountering additional errors, so I've held off on trying to use v4 branch for now. I'm curious if it fixes the Miss issues, though.

Currently, I'm experiencing a situation where if I hit a route that would cause Miss to render, everything works as you'd expect when navigating client-side via Links. But, when using server rendering of that route that would render Miss, I'm encountering a scenario where the client code is attempting to render something different than the server. In this case, the server is rendering the appropriate Miss component, but the client appears to be rendering an empty component there.

[Pajn]

Is this issue still about the published version, or the rewrite?
We are experiencing problems with alpha 6 and are unsure how to proceed, is it still too early to try to use version 4?

[ryancole]

This issue is about the published alpha version. The current state of the
v4 branch is much different. It doesn't even have Miss anymore, etc.

I think the published version is too broken to use, at the moment.

On Mon, Dec 19, 2016 at 7:45 AM Rasmus Eneman [e-mail]
wrote:

I this issue still about the published version, or the rewrite?

We are experiencing problems with alpha 6 and are unsure how to proceed,
is it still too early to try to use version 4?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ReactTraining/react-router/issues/4221#issuecomment-267967198,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAdlwYU2-1tohAph5QBI7RO_cDJjQtUWks5rJop_gaJpZM4K9FSJ
.

[seeden]

@ryancole How you are going to handle missing route in the v4-alt version? I can see just Route without patter for NotFound components

[ryancole]

@seeden no idea, I'm not familiar with the code for react router. I was just trying to skim over recent commits to v4-alt branch to see if I could figure out whether or not a new approach to Miss had been added. I couldn't quite figure it out on my own, though.

[timdorr]

Just so everyone knows, @ryanflorence and @mjackson are still finalizing the API. The v4-alt branch is some work by Michael to try out a different potential implementation. The v4 branch is basically Ryan's version of things that has been the status quo for a while now. Neither of these are considered complete and the APIs are still very much in flux. Hence, the "alpha" release names, not beta or anything approaching a final version name.

As with all alpha software, proceed at your own peril. Playing around with these versions and giving feedback is helpful to have us know more use cases and potential problems with the APIs chosen thus far. But please don't rely on this stuff for production software just yet. We're likely to pull the rug out from under you at some point during the process of nailing things down.

Getting back to the issue at hand, <Miss> is certainly a big pain point for the current API. <Match>/<Route> are easy by comparison. Getting the semantics and API right is important, but in some ways this is fighting against React best practices, so it's tough work. Please give the v4 and v4-alt versions a try and let us know your feedback.

[just-boris]

Please give the v4 and v4-alt versions a try and let us know your feedback.

@timdorr I'd like to try v4-alt, as far as I am already using v4-alpha.6, how can I install it?

[timdorr]

@seeden

How you are going to handle missing route in the v4-alt version? I can see just Route without patter for NotFound components

"Miss"ing in the v4-alt branch is equivalent to <=3.x, you would use a /* pattern to catch-all any missing routes and provide a <NotFound> kind of component back.

[timdorr]

@just-boris

I'd like to try v4-alt, as far as I am already using v4-alpha.6, how can I install it?

git clone ReactTraining/react-router && cd react-router && npm install && npm link should do the trick. Then npm link react-router in any of your projects to use the link'ed version.

[Pajn]

"Miss"ing in the v4-alt branch is equivalent to <=3.x, you would use a /* pattern to catch-all any missing routes and provide a kind of component back.

If I understand correctly, in v4-alt and you can't have multiple routes matching at once. But instead you can nest multiple routers?

I have not been able to try v4-alt yet but one thing I try to deduce from the code is if sub-routers must concern themselves with the full url or just the part relative to the parent router (like in v4 with match). That compose-ability that v4 allows by only concerning children with only the part that is relative is quite amazing and it would be sad if that isn't kept.
If it is kept I don't have much to say for v4 vs v4-alt. Both have things going for them and the react-approach is really the nicest router API I have ever seen so I think either one is great.

[dminkovsky]

The current state of the v4 branch is much different. It doesn't even have Miss anymore, etc.

Confused by this. As of right now, the v4 branch has Miss.js.

It is, however, indeed require different from the latest published alpha.

[dminkovsky]

Oh, we're talking the v4-alt branch. My bad! Sorry for the spam.

[ryancole]

After writing my own basic code using v4's approach, to help better understand the issues, I think that my current assumption is that it's not possible to determine a complete "route structure" using purely Match components, without performing some sort of up front render or some more manually-defined static route data structure.

The v4 approach is a top-down, declarative react approach, in that you can't know what the rendered component tree will look like until you actually render. In order to see down past each component, you must render that component. Without rendering, or providing a static route structure, it's my understanding that doing things like server 404 are not possible. There's just no way to know what the component tree will look like (meaning which Match were rendered, or Miss, etc).

I believe this is why v4 branch requires two renders to properly server render. Issues surrounding this technical barrier may also contribute to why Miss is so broken. It's a complicated thing to code when taking server rendering into account, as well.

While this component-based approach is amazing, it seems to me that it has a fundamental issue in that a render will be required in order to know what Match were actually rendered, or if a Miss was encountered, etc. That or some sort of static analysis step that builds that route structure for you (via babel or webpack maybe?), or manually defining that route structure (similar to react router versions 3 and older).

Without server rendering though, this approach is opened to a lot more options.

[pshrmn]

With the <Miss>-less v4-alt branch having been merged into v4 and a beta releasing soon, I think that it is safe to close this issue.