Use forced REPLACE only when both location and state are the same #179
Conversation
|
Please add test coverage here. |
|
Yes, will add, if this is considered a useful change. |
| @@ -120,7 +120,7 @@ function createHistory(options={}) { | |||
| const prevPath = createPath(location) | |||
| const nextPath = createPath(nextLocation) | |||
|
|
|||
| if (nextPath === prevPath) | |||
| if (nextPath === prevPath && deepEqual(location.state, nextLocation.state)) | |||
There was a problem hiding this comment.
We should probably make this check w/something that shares code w/locationsAreEqual, possibly prior to calling createPath.
| @@ -7,6 +7,7 @@ | |||
| "docs", | |||
| "es6", | |||
| "lib", | |||
| "modules", | |||
|
Logically, it seems like this PR is on the right track and philosophically compatible with the fix that @taurose made in #43. The idea behind #43 is that when the exact same @Macroz Any reason you chose to modify package.json? Otherwise, I'd say this looks good. |
|
One question I have is where this "replace PUSH with REPLACE" logic should go. I think There's nothing in the HTML5 history API that strictly prevents users from re-pushing the exact same location. The "no extra push" behavior only comes from following Do you think a user might ever legitimately need to push the same location? Or should we just not bother supporting that? |
No, I don't think that's a legit use case. You shouldn't have the ability to PUSH to where you already are. |
|
Fair enough. |
- Update tests to include check for changing state - URL stays the same, but state changes.
|
@mjackson I added the modules directory to use the repo as a npm package directly from GitHub, while waiting for the discussion to finish. Now I've removed it, rebased to master and squashed the commits. Does this look good? I noticed that there are some tests failing in master at the moment. |
Use forced REPLACE only when both location and state are the same
|
Great! Thanks for the fix! |
Fixes #178