-
Notifications
You must be signed in to change notification settings - Fork 7
fix: correctly handle cancellation of navigations #120
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
base: main
Are you sure you want to change the base?
fix: correctly handle cancellation of navigations #120
Conversation
🦋 Changeset detectedLatest commit: 59f8c22 The changes in this PR will be included in the next version bump. 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
50f231c to
ace70ee
Compare
@richardjelinek-fastest Curious to see your method of test, I didn’t see issues when testing this, but maybe I missed something |
I've created a min-repro demo: https://github.com/richardjelinek-fastest/sv-router-navigation-bug |
|
I don't see the issue in your repro 🤔 |
Ok I now see why. I also added a new condition that redirects from the homepage, so you don't have to go into the /nested page to check the possible bug. |
8e241af to
fe2fe0b
Compare
504929e to
042dae8
Compare
|
@colinlienard Do you plan to look into it? 👀 Just bumping, it's okay if you're busy :) |
|
I was still not able to reproduce the issue... |
3d00fee to
f399edc
Compare
|
@colinlienard Ok it's not that easily reproducible. Here's a video of the weird bug happening. I've also updated the repro demo once again... Working on an update to this PR. It's missing the change that seems to fix it. Screencast_20250819_212333.webm |
f399edc to
2a3a783
Compare
72d012c to
61c9085
Compare
|
Don't know why the test is failing in the actions. Locally it passes :'( |
61c9085 to
f0792b7
Compare
f0792b7 to
d732d56
Compare
|
@richardjelinek-fastest is attempting to deploy a commit to the Colin Lienard's projects Team on Vercel. A member of the Team first needs to authorize it. |
deddf78 to
ae0e0b6
Compare
ae0e0b6 to
59f8c22
Compare
Tries to fix #118
Passes initial URL parameters to onNavigate so we can catch them in beforeLoad at page load.
Hopefully cancel all navigations if they're not the most recent one. Old logic did not work from my tests.
Return Error in
navigateso linters (ESLint) don't complain about throwing void.