Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

react-route's onEnter handler called twice #481

Closed
semurgx opened this issue Nov 8, 2016 · 38 comments
Closed

react-route's onEnter handler called twice #481

semurgx opened this issue Nov 8, 2016 · 38 comments

Comments

@semurgx
Copy link

semurgx commented Nov 8, 2016

I've found, that applying current react-router-redux (4.0.7) causes twice calling react-router's asynchronous (with 3 arguments) onEnter handler while initial loading. Here is a gist, describing this issue https://gist.github.com/semurgx/9abb0d02ea49f49aa97ef34cac117f5f .

@wphampton
Copy link

I'm seeing this as well since upgrading this week, though in a different scenario, not on the initial page render but at other times in the application after a push. Also, for me I only see it with hash history (I see you are using browser history). Also, using the example from your gist, if you don't put a setTimeout in there then it only gets called once. It's interesting that even a setTimeout value of zero causes it to be called twice.

const handleEnter = (nextState, replace, cb) => {
    console.log('whoa! called twice');
    //setTimeout(cb, 500); <-- will cause onEnter to be called twice
    //setTimeout(cb, 0); <-- will cause onEnter to be called twice
    cb(); // <-- Will cause onEnter to be called only once
};

I recently upgraded a number of packages along with this one and can't figure out what is causing this. With react-router-redux 4.0.7 comes react-router v.3 and history v.3 but even when I fall back to previous versions I can't get it back to how it used to be, which makes me think I'm missing something else here, maybe another supporting package or something else I blindly upgraded and can't identify.

In the meantime I am planning to use a debouce on my onEnter function...

@hilkeheremans
Copy link

Experiencing exactly the same issue. Also working around using debounce for now.

@mofelee
Copy link

mofelee commented Nov 17, 2016

Delete this line can fix this issue.

listener(lastPublishedLocation)

@semurgx
Copy link
Author

semurgx commented Nov 17, 2016

Another way to fix this issue:
wrap

handleLocationChange(history.getCurrentLocation())
line with isTimeTravelling = true and isTimeTravelling = false.

@gerhardsletten
Copy link

I did also had this issue and I fixed it by downgrading react-router to 2.x

@amarinov
Copy link

I stumbled upon this issue as well. Seems like a serious one to me. A downgrade to 4.0.6 fixed it for the time being.

@tvanro
Copy link

tvanro commented Dec 22, 2016

Encountered the same issue and I implemented a workaround with shouldComponentUpdate for this:

export default class RootRoute extends Component {

  render() {
    // Create an enhanced history that syncs navigation events with the store
    const history = syncHistoryWithStore(browserHistory, store)
    return (
      <Router history={history}>
        <Route component={App}>
          <Route path="/foo" component={Foo} />
          <Route path="/bar" component={Bar} />
        </Route>
      </Router>
    );
  }
}
export default class App extends Component {

  shouldComponentUpdate(nextProps) {
    // performance workaround until react-router-redux fixes https://github.com/reactjs/react-router-redux/issues/481
    return nextProps.location.action === 'POP';
  }

  render() {
    return (
      <h1>Hello Word!</h1>
    );
  }
}

@neverfox
Copy link

Oh man, I was pulling my hair out because my injected sagas (in onEnter) were running twice and I thought I had messed up somewhere. Reverted to 4.0.6 and the problem went away.

@timdorr
Copy link
Member

timdorr commented Jan 13, 2017

Just to get some more information: What version of react-router and history is everyone using?

@michaelBenin
Copy link

Latest stable.

@timdorr
Copy link
Member

timdorr commented Jan 13, 2017

Technically, history 4.5.1 is the latest stable version, but we don't support that. I need your exact version numbers to be helpful.

@michaelBenin
Copy link

OK, I have an example set up on this project: https://github.com/michaelBenin/react-ssr-spa. On the master branch, upgrade react-redux to 4.0.7.

@timdorr
Copy link
Member

timdorr commented Jan 13, 2017

You're using history 4.4.1 with react-router 3.0.0, but we only support history 3.x.x. You have fundamentally incompatible versions installed.

@michaelBenin
Copy link

I've created a branch that has the correct versions:

https://github.com/michaelBenin/react-ssr-spa/compare/develop-react-router-redux-repro

@mykyta-shulipa
Copy link

Interesting...
No explicit dependency from 'history' in my project, and react-router v3.0.0 with react-router-redux v4.0.7 share the same lib version (history v3.2.1 if it will be helpful). But still have same issue - onEnter called twice for some reasons..
As I can understand it's rather related to react-router itself, see reference above.
It was closed when it was a question, but they think about reopening it as a bug.

@michaelBenin
Copy link

Also, something else I found to be very strange.

In src/sync, when calling:

history.getCurrentLocation()

This yields different results from client and server, hence invariant errors upgrading from 4.0.6 to 4.0.7.

On server:

{ pathname: '/repo/michaelBenin/react-ssr-spa',
  search: '',
  hash: '',
  state: undefined,
  action: 'POP',
  key: null,
  query: {} }

Where as in browser, key has a value of a string and not null.

@michaelBenin
Copy link

I currently have not been able to upgrade my app from 4.0.6, should we open a different issue for this on history? calling history.getCurrentLocation on server yields different values?

@timdorr
Copy link
Member

timdorr commented Jan 16, 2017

@michaelBenin That initial null key is described in the 3.0 changelog: https://github.com/mjackson/history/blob/v3.x/CHANGES.md#300-0

@michaelBenin
Copy link

Ok good to know that is expected in the history library.

Should this be a different issue for react-router-redux that 4.0.7 is breaking server rendering causing an invariant error because history.getCurrentLocation is giving different results in initial render in client and server?

Would you be open to reviewing a pr at attempted to fix this key having a different value?

Thank you.

@timdorr
Copy link
Member

timdorr commented Feb 8, 2017

BTW, I put the original gist into a jsfiddle here: https://jsfiddle.net/n4ovmzzf/ Just click Run at the top after it loads because the result iframe URL changes from the first load and the route won't match anything initially.

@timdorr
Copy link
Member

timdorr commented Feb 8, 2017

I think I finally figured it out. We were calling all listeners synchronously for history 3.x, which doesn't operate like that. I think b2c2259 fixes it, but can someone test that out?

@dritanlatifi
Copy link

dritanlatifi commented Feb 9, 2017

I want to test it, but I don't know how to install this version :-$
How can I install the version in my project? I tried with
npm install git+https://github.com/reactjs/react-router-redux.git
I seemed that it installed it. I checked in the code in node_modules. But when I run the application, I get the message:

Module not found: Error: Can't resolve 'react-router-redux' in XXX
 @ ./app/routes.js
 @ ./app/app.js
 @ multi babel-polyfill eventsource-polyfill webpack-hot-middleware/client bootstrap-loader ./app/app.js

@toomuchdesign
Copy link

Thanks @timdorr, the conditional statement added on line 130 fixed the duplicated onEnter call in my project.

@michaelBenin
Copy link

@timdorr can you create a branch with the built files?

npm installed the commit, didn't have the built files. cd'd into directory and looked at the package.json which pointed to lib/index, no lib folder existed, tried pointing to src/index, didn't work, then ran npm i && npm run build, no dice. I'll be happy to test again once the built files are available.

@semurgx
Copy link
Author

semurgx commented Feb 10, 2017

@timdorr Yes, I've checked your fix and it works. Waiting on 4.0.8

@rybon
Copy link

rybon commented Feb 10, 2017

@timdorr your commit b2c2259 indeed fixes it, could you publish this version to npm?

@timdorr
Copy link
Member

timdorr commented Feb 10, 2017

Pushed as 4.0.8 now.

@wphampton
Copy link

Sad to report this did not work in my case using hash history. As soon as I removed the debounce I got two calls again.

@timdorr
Copy link
Member

timdorr commented Feb 10, 2017

The problem with hash histories isn't in this library directly: remix-run/history#427

@smstamm
Copy link

smstamm commented Feb 21, 2017

The 4.0.8 fix worked for me while navigating through the app, but onEnter is still called 2x if the page is refreshed. Is anyone else seeing that, or is there another explanation for why that's happening?

@StevenKourepenos
Copy link

@smstamm I am also seeing this issue on refresh after updating to 4.0.8. The new release does not appear to have fixed the issue. This is pretty annoying as using browserHistory is not an option due to no back-end support for the route changes. I'm still tinkering with some of the options to see if I can figure something out. I have several timeouts in my code that are being thrown off by the additional route change. It would be nice to have this figured out so that the code doesn't need to be so fine tuned.

Can someone from the dev team clarify if this is issue with the history package itself or react-router-redux? It seems that this issue has been opened and closed on multiple repos but I am still seeing messages from folks within the past few weeks that confirm that this issue is still not resolved.

@timdorr
Copy link
Member

timdorr commented Feb 27, 2017

@smstamm and @StevenKourepenos As I already pointed out 2 comments up, the bug with hashHistory double-firing isn't in this library: remix-run/history#427

@smstamm
Copy link

smstamm commented Mar 2, 2017

@timdorr I am using browserHistory. onEnter only fires once when I change routes, but when I refresh the page onEnter fires 2x.

@andykenward
Copy link

Do you happen to have the chrome redux dev tools extension installed? Try disabling it

@lichenglu
Copy link

lichenglu commented Apr 2, 2017

@timdorr Same here, I am using browserHistory, and refreshing the page will fire onEnter twice.

@andykenward I disabled redux dev tool and tried again, but the issue is still there

@uynap
Copy link

uynap commented Apr 24, 2017

I'm using react-router-redux 4.0.8 with react-router 3.0.0.

When I access "/admin/test" the "onEnter" will be called once, but if I switch between "/admin/test" and "/admin/test2", onEnter will not be triggered. Is this what you are having?

const Root = ({ store, history }) => (
  <Provider store={store}>
      <Router history={history}>
        <Route path="/" component={App}>
          <Route path="login" component={Login} />
          <Route path="admin" component={Admin} onEnter={requireAuth(store)}>
            <Route path="test" component={Test} />
            <Route path="test2" component={Test2} />
          </Route>
        </Route>
       </Router>
  </Provider>
)

@bretthadley
Copy link

@uynap You need to declared the onEnter on child routes 'test & 'test2'

@uynap
Copy link

uynap commented Apr 24, 2017

@bretthadley I think "onEnter" is not designed to be used in that way. Without using Redux, "onEnter" will be triggered if i jumped between these 2 routes with the code above.

Also, if I declare the onEnter on all child routes, then the onEnter callback(requireAuth) will be called multiple times when I access the child route.

@reactjs reactjs locked and limited conversation to collaborators Apr 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests