Skip to content

Proposal: non-blocking updates refactor #5534

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

Closed
wants to merge 1 commit into from

Conversation

eloytoro
Copy link
Contributor

@eloytoro eloytoro commented Sep 19, 2017

I do not intend for this PR to get merged, its just a proposal

Non-Blocking react-router

The main issue here is that in order to know the router state from a component this component must be either rendered using a <Route component={Component} or HoC'd withRouter(Component) and even then the component will probably not update itself when the route changes.

This happens because Route doesnt update itself when the history changes. Because of react-router's context re-definition strategy Route components will basically hide the history from Routes that are deep into the VDOM tree behind a virtual history layer thats defined by its parent.

I myself would much rather pay attention to the Router's history context because its global in a sense that I only care about what's in the user's browser address field, not what the current context allows me to see.

This approach would make it much easier to listen to history changes from each of the Routes, each one would be able to tell if it should update itself or not depending on the way the path matches the current history.location.

Here's a test case of what I'm trying to achieve (and the current implementation of react-router wont do)

  it('updates itself even when the match is different', () => {
    const node = document.createElement('div')

    let push
    let count = 0
    ReactDOM.render((
      <MemoryRouter initialEntries={[ '/sushi/california' ]}>
        <Route path="/sushi/:roll" render={({ history, match }) => {
          push = history.push
          count = count + 1
          return <div>{count}</div>
        }}/>
    </MemoryRouter>
    ), node)
    push('/sushi/dynamite')
    expect(node.innerHTML).toContain(2)
    push('/sushi/dynamite')
    expect(node.innerHTML).toContain(2)
    push('/sushi/dynamite/eat')
    expect(node.innerHTML).toContain(2)
    push('/sushi/california')
    expect(node.innerHTML).toContain(3)
  })

Notice that the component only re-renders when the match is different

@timdorr
Copy link
Member

timdorr commented Sep 22, 2017

The inverse of this was already discussed in #4598.

Lots of details in there. But to re-hash Ryan's comments:

  1. They are hard to understand
  2. They don’t feel like React
  3. They were causing lots of re-rendering on popstate that we’d have to start doing our own scheduling/batching to get around D: With every new nested Route you'd get another round of rendering.

@timdorr timdorr closed this Sep 22, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants