-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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(vue): incorrect view rendered when using router.go(-n) #29877
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
The main problem was that when using |
@brandyscarney just a heads up, this PR relates to one assigned to you here: #29877 |
@thetaPC are you able to check this one for me and let me know if any changes required on my end to get this one through? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for submitting a PR!
I noticed you removed the if statement from the code. I'd love to understand your reasoning behind that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @thetaPC thanks for the review!
The if
statement I've removed was originally added to handle the situation where delta
is less than -1
. In that situation what it does is return the location delta
many steps back from the end of the history stack.
if (delta < -1) {
return locationHistory[locationHistory.length - 1 + delta];
}
However, this wasn't correct behaviour. There is a routeInfo
passed into the function and the returned location should be delta
steps back from that. Not back from the end of locationHistory
.
This is why the bug reported in the issue was happening.
I've made it so it handles both situations using that same code. This is mostly reusing the code that was in the else
statement.
That code essentially searches backwards through the history stack until it finds routeInfo
and returns its parent (handling the delta
-1
situation). I realised I could use index of that location in the array + delta to return the correct location.
I've added tests to handle the situations and ensure the behaviour works as expected. Let me know if you need any more details :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
Gave @xxllxhdj co author credit as the team decided to go with this approach. Thank you everyone! |
resolves #28201
This PR fixes the navigation issue related to
router.go
that was identified in issue #28201. After working on this issue I realised that @xxllxhdj has already created a PR for this in #29847. While their fix is great, I have added tests to replicate the issue, reused existing code andundefined
will be returned in unexpected situations - which matches the existing functionality.What is the current behavior?
If a user navigates from
/home
->/pageA
->/pageB
->/pageC
-> back to/pageB
-> thenrouter.go(-2)
is called the URL will be updated to/home
correctly, but the app will try to render/pageA
.This happens for any delta < -1.
What is the new behavior?
The app will correctly render
/pageA
, which matches the URL.Does this introduce a breaking change?