remix-run / history

Manage session history with JavaScript
MIT License
8.29k stars 960 forks source link

hashchange double-fires listeners #427

Closed timdorr closed 7 years ago

timdorr commented 7 years ago

As discussed in ReactTraining/react-router#4266. Here's my comment summing it up:

Looks like the problem is the hashchange listener is calling the history listeners directly instead of using transitionTo like 2.x did. While that's fine for the browser history, because popstate only fires on browser-initiated transitions, the hashchange event fires for all mutations of the hash, including the transition itself! That causes it to double up the listener calls.

So, I'm not sure of the best fix for the HashProtocol. Perhaps switch it back to transitionTo so that it can filter out duplicate transitions? Or maybe there is some other way to distinguish hashchange event sources?

mjackson commented 7 years ago

FWIW I simplified the way this works in v4. hashchange is tricky. Browsers actually fire the event multiple times so we have to try and make sure we don't call it twice for the same transition. It's a real mess. Good luck!

nippur72 commented 7 years ago

will this be ever fixed for v3 ?

Because of it I had to revert to old react-router v2.8 😞

mjackson commented 7 years ago

Probably not. I already fixed it once in v4. I'm not keen on fixing the same bug twice!

mjackson commented 7 years ago

I'm not supporting version 3 any more. Please send a PR if you'd like to get this fixed.

iyobo commented 7 years ago

And so for that PR:

hashHistory.listen( ( evt ) = > {
    console.log('LISTENER:', evt)
});

This event handler gets called twice per route visit. But evt is different each time:

//log upon visiting /models/Testy

LISTENER: 
{"pathname":"/models/Testy","search":"","hash":"","action":"PUSH","key":"1myy9b","query":{}}

LISTENER: 
{"pathname":"/models/Testy","search":"","hash":"","action":"POP","key":null,"query":{}}

It's basically as a result of that annoying evt.key url replacement thing they do in hashHistory. It just means, when responding to Link clicks for the hashhistory alone(Not sure if browserHistory does that seemingly useless key stuff) , react-router should only act on events with:

evt.action=== 'PUSH' && evt.key

Would there be any other consideration?

arfa commented 7 years ago

@iyobo

router.goBack() will trigger {"pathname":"/models/Testy","search":"","hash":"","action":"POP","key":null,"query":{}}

I am using this fix until we update our router to V4

let lastLocation = '';

hashHistory.listen(location => {
  if (lastLocation !== location.pathname) {
    lastLocation = location.pathname;
    store.dispatch(changeLocation(lastLocation));
  }
});