Skip to content
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

Browser back button not working with react-router@4.0.0-beta.7 and react@16-alpha.4 #9214

Closed
maierson opened this issue Mar 18, 2017 · 13 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@maierson
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
After installing react@next (fiber) Clicking to a new page inside the same domain (same app) and then clicking the back button doesn't move the page back.
It all works correctly with react@15.4.2

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).
see above

What is the expected behavior?
Clicking the back button should move back to the previous location.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react@16.alpha4
chrome 56
osX Sierra

It all works correctly with react@15.4.2 and react-router@4.0.0-beta.7

@gaearon
Copy link
Collaborator

gaearon commented Mar 18, 2017

If you can isolate the issue to React itself, we'd love to look.
Unfortunately it's very hard to help if it involves third party libraries.

At the very least we'll need a minimal example demonstrating the issue.

@gaearon gaearon added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 18, 2017
@maierson
Copy link
Author

Thanks Dan. Will look into a stripped down version of the project as soon as I get a moment.

Fiber looks extremely promising. It was a breeze to migrate (just a couple of - very helpful - warnings about render needing to be pure).

What makes me think it's react16 is the fact that I only swapped the libs (react, react-dom) for the issue to happen. Will try to isolate.

@gaearon
Copy link
Collaborator

gaearon commented Mar 18, 2017

Yea, it could be a slight difference in behavior (either intentional or unintentional), but it's hard to say where it is without having a way to reproduce.

@maierson
Copy link
Author

I've created a basic project with create-react-app + react-router v.4 and swapped in fiber but it's not reproducing (all works ok). I'll have to dig deeper in our main project but it's fairly large so it will take me a while. I'd rather wait until at least a beta version of fiber.

I'll close this for now and revert once (if) I can reproduce reliably.

@gaearon
Copy link
Collaborator

gaearon commented Mar 20, 2017

Sounds good! One thing to note is that in Fiber, when a component replaces another component, the order of their mounting and unmounting lifecycle hooks is swapped.

In React 15, if A is replaced by B, we unmount A, and then create and mount B:

  • A.componentWillUnmount
  • B.constructor
  • B.componentWillMount
  • B.componentDidMount

In Fiber, we create B first, and only later unmount A and mount B:

  • B.constructor
  • B.componentWillMount
  • A.componentWillUnmount
  • B.componentDidMount

So if you relied on this somewhere in the app (e.g. by triggering side effects while unmounting or mounting), it might break.

@olegstepura
Copy link

Hi!

I experienced an issue with react 16 and react-router 4 as well.
After digging I found out that my issue was related to some strange behavior of componentWillUnmount

In my case I produce a component which uses componentWillMount to update app state (set current section and subroutes to state) and componentWillUnmount to reset this.
The issue is that after entering the section I get both callbacks almost immediately with react 16.
E.g. when entering the section I render a component which has a Switch as children

import React, { Component } from 'react'
import PropTypes from 'prop-types'
import { Route, Switch, Redirect } from 'react-router'
import { connect } from 'react-redux'
import NotFound from 'component/NotFound'
import { routesNestedUpdate, routesNestedReset } from 'action'
import { withRouter } from 'react-router-dom'

/**
 * Updates nested routes when entering the subsection so that menu can be expanded.
 * Renders nested routes to perform inner switch.
 * @param routeGen Routes generator
 */
const subsectionFactory = routeGen => {
  class Subsection extends Component {
    routes = []

    componentWillMount() {
      const { url } = this.props.match
      this.routes = routeGen(url)
      this.props.routesNestedUpdate({ path: url, routes: this.routes })
    }

    componentWillUnmount() {
      this.props.routesNestedReset()
    }

    render() {
      const { children } = this.props

      return (
        <div style={{ border: '4px solid red' }}>
          {children}
          <Switch>
            {this.routes.map((route, index) => (
              route.redirect
                ? <Route { ...route } key={index} render={() => <Redirect to={route.redirect} />} />
                : <Route key={index} { ...route} />
            ))}
            <Route component={NotFound} />
          </Switch>
        </div>
      )
    }
  }

  const mapStateToProps = /*...*/
  const mapDispatchToProps = /*...*/

  return withRouter(connect(mapStateToProps, mapDispatchToProps)(Subsection))
}

export default subsectionFactory

Notice the red border. When I enter the section, it's index route is rendered inside this red border. My code in componentWillMount is called. This code modifies state in reducer.

And what I noticed is that immediately after that componentWillUnmount is called. This resets the state and I get no menu for this section...
But everything rendered on screen inside of red border is still there. Component was not unmounted, all his children from Switch are visible so componentWillUnmount is not expected to be called.

image

Text Dashboard page is rendered by index component inside current section.

This does not happen if I do first render of this url. But this happens if I navigate to this url via dispatching an action (click menu item link which does dispatch @@router/LOCATION_CHANGE). In this case one such subsection element is replaced with another. All later dispatches are broken as well.

I also must notice that root routing is implemented using this code:

  <Switch>
    <For each="props" index="index" of={routes}>
      <Route key={index} { ...props} /> // <-- subsectionFactory result is mounted here after dispatching location change
    </For>
    <Route component={NotFound} />
  </Switch>

this worked ok in React 15.

@gaearon
Copy link
Collaborator

gaearon commented Aug 15, 2017

Did you get a chance to read my comment above? I believe I explained exactly this case: #9214 (comment).

@olegstepura
Copy link

Yes, I did. Sorry, only now, after I thought again about this, I figured out what is wrong.

Any suggestions on how to solve this case? I can remove this.props.routesNestedReset() from componentWillUnmount but this is not a complete solution (since reset will not be called on navigating away from section which is wrapped with the factory to a standalone section without factory).

@gaearon
Copy link
Collaborator

gaearon commented Aug 15, 2017

It's a bit hard for me to understand what exactly you're trying to do. Maybe moving logic from WillMount to DidMount would help?

@olegstepura
Copy link

Well, yes, but I had to calc routes before render. So after separating the logic into two methods everything is fine.

    componentWillMount() {
      const { url } = this.props.match
      this.routes = routeGen(url)
    }

    componentDidMount() {
      const { url } = this.props.match
      this.props.routesNestedUpdate({ path: url, routes: this.routes })
    }

Thanks!

@maierson
Copy link
Author

A quick gotcha here on what was happening on our side. The Routes were inside a blocking App component (see here why this happens)

const App = () => (
   <Switch>
      <Route path="/comp1" component="Comp1"/>
      <Route path="/comp2" component="Comp2"/>
   </Switch>
)

Its container in turn was a child of the Router. Since the AppContainer is not inside a route it wouldn't re-render when the url changed.

ReactDom.render(
    <Provider store={store}>
      <ConnectedRouter history={history}>
          <AppContainer />
      </ConnectedRouter>
    </Provider>, 
    document.getElementById("root")
),

The solution was to render the App inside a no path route. This way it is always rendered when the url changes triggering the update of inner routes.

ReactDom.render(
    <Provider store={store}>
      <ConnectedRouter history={history}>
          <Route component={AppContainer} />
      </ConnectedRouter>
    </Provider>, 
    document.getElementById("root")
),

@livemixlove
Copy link

livemixlove commented Aug 23, 2018

I'm encountering this today. react-router-dom Links links work fine, but back button broken (UI doesn't change). maierson's solution fixed it for me. Seems like a bug or needing documentation.
Packages:
Edit:
My packages:

"react-router": "^4.3.1",
"react-router-config": "^1.0.0-beta.4",
"react-router-dom": "^4.3.1",
"react-router-redux": "^5.0.0-alpha.9",

@gaearon
Copy link
Collaborator

gaearon commented Aug 23, 2018

This discussion is turning into a React Router discussion. It's not an issue with React. Therefore, I'll lock this to avoid further confusion. If you have a problem with React Router, please bring it up on their repository or on support forums. We use this issue tracker only for bugs in React itself.

@facebook facebook locked as off-topic and limited conversation to collaborators Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants