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

(preact-iso): Router: Fix change location after useLocation().route() #424

Merged
merged 2 commits into from
Mar 14, 2021

Conversation

piotr-cz
Copy link
Contributor

At this moment routers' reducer dispatcher behaves as follows:

  • User action (MouseEvent):
    window location is updated via history.pushState

  • User navigation via back/ forward (PopStateEvent):
    window location is left intact, as it's change is handled by browser

  • Manual route change via useLocation().route(path)
    window location is left intact

This PR

  • Fixes last case so history.pushState is used to add new entry to browsers history and in effect location changes.
  • Moves push from dispatcher argument to function code block, As reducer dispatchers consume only two arguments. it gave false indication that the argument may be chosen somehow.
  • Makes it clear that in reality there is no way to set push to false and in effect use history.replaceState

@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2021

🦋 Changeset detected

Latest commit: 0bbb8cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
preact-iso Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -1,16 +1,17 @@
import { h, createContext, cloneElement } from 'preact';
import { useContext, useMemo, useReducer, useEffect, useLayoutEffect, useRef } from 'preact/hooks';

const UPDATE = (state, url, push) => {
const UPDATE = (state, url) => {
let push = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we get to the else if (push === false) history.replaceState(null, '', url); now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot. however we never could (the UPDATE method is not exported, just the route(url) is). Now it's just obvious to notice.
Please read this PR description for explanation and try to get to evaluate history.replaceState on master branch if you can.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, #413 will provide a way to use push.

@developit developit merged commit edca6ce into preactjs:main Mar 14, 2021
@github-actions github-actions bot mentioned this pull request Mar 14, 2021
@piotr-cz piotr-cz deleted the bugfix/preact-iso/router/pushstate branch March 15, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants