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

client: Fix navigation regressions #1515

Merged
merged 3 commits into from
Jan 9, 2025
Merged

client: Fix navigation regressions #1515

merged 3 commits into from
Jan 9, 2025

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Jan 9, 2025

Regressions from 88deede

Fixes: #1513

`state` belongs to the second argument.

And we do not really need to spread the original location – `pathname` and `search` will be updated as needed, and we do not care about `hash`.

This got broken in 88deede.
Avoid pushing an extra item to history so that back button works.

This got broken in 88deede.
When an article is activated, its `id` will be appended to URL.
`NavigateFunction` depends on `Location` so when the URL was updated,
the `useEffect` callback would be triggered due to the dependency change.
The callback would then reload the entries list. Even worse, when using
the unread filter, the just activated entry would disappear.

However, we should not actually need to pass the `navigate` function:
The `Location` is really only a dependency so that `pathname: null`
can use the current location,
https://github.com/remix-run/react-router/blob/a3e4b8ed875611637357647fcf862c2bc61f4e11/packages/react-router/lib/router/utils.ts#L1267
and we always do set a string `pathname`.
And the other dependencies appear to be static:
https://github.com/remix-run/react-router/blob/a3e4b8ed875611637357647fcf862c2bc61f4e11/packages/react-router/lib/hooks.tsx#L292-L296

This got broken in 88deede.
@jtojnar jtojnar added this to the 2.20 milestone Jan 9, 2025
@jtojnar jtojnar added the bug label Jan 9, 2025
Copy link

netlify bot commented Jan 9, 2025

Deploy Preview for selfoss canceled.

Name Link
🔨 Latest commit 9bcd66a
🔍 Latest deploy log https://app.netlify.com/sites/selfoss/deploys/67805c402a38440008902aa1

@jtojnar jtojnar merged commit 9bcd66a into master Jan 9, 2025
15 checks passed
@jtojnar jtojnar deleted the fix-navigation branch January 9, 2025 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2.20-e88f0b8: items don't open
1 participant