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

fix(history): browser transitions being skipped for router.history navigation calls #2999

Closed

Conversation

scarabcoder
Copy link

@scarabcoder scarabcoder commented Dec 13, 2024

Fixes #2983

When using back(), forward() and go() on router.history for the browser history implementation, native transitions are not occurring.

This is caused by notify() being called twice, once for the back() from the default history implementation and again for when the window event listener is called by the browser for a navigation event.

This PR fixes it by moving calls for notify() to the implementation for the history API, so that the browser history implementation does not call notify() (instead relying on the event subscriber), while the memory implementation calls notify() on every history navigation call.

(fix suggested by @schiller-manuel )

@scarabcoder scarabcoder changed the title fix: browser transitions being skipped for router.history navigation calls fix(#2983): browser transitions being skipped for router.history navigation calls Dec 13, 2024
Copy link

nx-cloud bot commented Dec 13, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9155491. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Copy link

pkg-pr-new bot commented Dec 13, 2024

Open in Stackblitz

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@2999

@tanstack/create-router

npm i https://pkg.pr.new/@tanstack/create-router@2999

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@2999

@tanstack/react-cross-context

npm i https://pkg.pr.new/@tanstack/react-cross-context@2999

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@2999

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@2999

@tanstack/react-router-with-query

npm i https://pkg.pr.new/@tanstack/react-router-with-query@2999

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@2999

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@2999

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@2999

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@2999

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@2999

@tanstack/start

npm i https://pkg.pr.new/@tanstack/start@2999

@tanstack/start-vite-plugin

npm i https://pkg.pr.new/@tanstack/start-vite-plugin@2999

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@2999

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@2999

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@2999

commit: 9155491

@SeanCassiere SeanCassiere changed the title fix(#2983): browser transitions being skipped for router.history navigation calls fix(history): browser transitions being skipped for router.history navigation calls Dec 13, 2024
@schiller-manuel
Copy link
Contributor

we have another PR open that touches history

#1790

this has an ignoreNextPop flag we can use to solve this issue here.
so let's first merge #1790 and then fix the issue here

@ferretwithaberet
Copy link
Contributor

ferretwithaberet commented Dec 15, 2024

#3017 might fix the same issue, to be checked

Edit: Can you pull the PR above and check if the issue is fixed? @scarabcoder

@schiller-manuel
Copy link
Contributor

yes, #3017 will fix this

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.

View Transition API not working for router.history navigation calls
4 participants