history.listen doesn't fire when route changes via history.push

Created on 18 Jun 2019  路  6Comments  路  Source: ReactTraining/history

Version

5.0.0

Steps to reproduce

  1. import { withRouter, Link } from 'react-router-dom';
  2. componentDidMount() {this.unlisten = this.props.history.listen(() => console.log('route changed')); }
  3. componentWillUnmount() {this.unlisten();}
  4. this.props.history.push('url'); _in my case this line executed after ajax req_
  5. export default connect(mapStateToProps, mapDispatchToProps)(withRouter(MyCom));

Testcase: https://github.com/dehghani-mehdi/history-bug-testcase

Expected Behavior

console.log('route changed') should run after step 4. but it doesn't.

Actual Behavior

history.listen doesn't fire.

BTW when if route changes via <Link>, everything is fine.

Most helpful comment

My apologies for my brusqueness; I wasn't paying close enough attention and thought that this issue was in the React Router repo. However, this also isn't a bug with history.

Each history listener is wrapped with a function that will only call the listener when it is "active". Removing the listener sets the active value to false and the listener will not be run.

In your code, the history listener is removed when a component unmounts.

Combining the above two statements, it is a pretty safe guess that when you click the button, the history listener is not called until after the component has unmounted.

Of course, this is not happening when you click a link, so what is the difference between clicking a link and clicking a button? The button is making the navigation from within a callback to the getAll API call; if we remove the getAll call from the event handler and only call history.push, we will see the home: route changed! message displayed.

Why? React batches updates that happen in events by default. However, when the update gets scheduled asynchronously (even if the asynchronous scheduling happens from within an event handler), updates are handled individually.

What this means is when the history.push happens asynchronously, history calls its first listener (the one added by the router), which sets the new location and triggers a re-render. The re-render finishes and the Home component is unmounted, which means that history sets the listener registered by the Home component to not "active". Finally, the listener registered by the Home component is called, but it isn't active so it doesn't do anything.

The react-dom package currently provides an unstable_batchedUpdates function which you _could_ use to batch the updates, but the unstable prefix is there for a reason. Eventually React will ship a "scheduler" package which will include supported API.

All 6 comments

Please stop posting the same issue; while you very like _are_ dealing with a bug, it is not with React Router.

Additionally, we provide the CodeSandbox template to make it as simple as possible to create a reproducible demonstration of an issue. Even if we did provide support help, an issue that provides no reproducible code makes it incredibly difficult to deduce what the problem is.

As it is, I threw together this quick sandbox to demonstrate that history.listen does indeed work: https://codesandbox.io/s/peaceful-snow-6dchl

@pshrmn in this repo this is my very first reported issue, I opened an issue on react-router and they told me the issue is belongs to history, so I opened the issue here.

provides no reproducible code

If what I mentioned in Steps to reproduce section in my post is not code, what is that?

About your demo code, it's not same as mine, which already mentioned in my first post.

I'm wondering why you like to close the issue without any reason, obviously your demo code doesn't meet the situation and closing the issue based on that is not pro behavior.

@dehghani-mehdi The steps to reproduce you provided obviously aren't detailed enough for us to use to reproduce the bug. If you can provide a codesandbox or a github repo that already and clearly reproduces the issue, that will greatly improve our ability to help you.

(But I understand why you might be a little impatient by now.)

@pshrmn Well, they were told by @MeiKatz that this is a history issue. It's not unreasonable to then open the issue here too, after having it closed twice on the other repo. Especially since that comment was the only reasonable "answer" as to why their original was closed.

@pshrmn, @StringEpsilon here you go:
https://github.com/dehghani-mehdi/history-bug-testcase
More info can be found on repo's readme

@pshrmn still is not a bug and this issue should stay closed?

My apologies for my brusqueness; I wasn't paying close enough attention and thought that this issue was in the React Router repo. However, this also isn't a bug with history.

Each history listener is wrapped with a function that will only call the listener when it is "active". Removing the listener sets the active value to false and the listener will not be run.

In your code, the history listener is removed when a component unmounts.

Combining the above two statements, it is a pretty safe guess that when you click the button, the history listener is not called until after the component has unmounted.

Of course, this is not happening when you click a link, so what is the difference between clicking a link and clicking a button? The button is making the navigation from within a callback to the getAll API call; if we remove the getAll call from the event handler and only call history.push, we will see the home: route changed! message displayed.

Why? React batches updates that happen in events by default. However, when the update gets scheduled asynchronously (even if the asynchronous scheduling happens from within an event handler), updates are handled individually.

What this means is when the history.push happens asynchronously, history calls its first listener (the one added by the router), which sets the new location and triggers a re-render. The re-render finishes and the Home component is unmounted, which means that history sets the listener registered by the Home component to not "active". Finally, the listener registered by the Home component is called, but it isn't active so it doesn't do anything.

The react-dom package currently provides an unstable_batchedUpdates function which you _could_ use to batch the updates, but the unstable prefix is there for a reason. Eventually React will ship a "scheduler" package which will include supported API.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

AndreasHogstrom picture AndreasHogstrom  路  4Comments

richardgorman picture richardgorman  路  5Comments

andi5 picture andi5  路  6Comments

vchibilisco-convey picture vchibilisco-convey  路  3Comments

vass-david picture vass-david  路  8Comments