Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upfix(router): fix navigation ignoring logic to compare to the browser url #37716
+203
−10
Conversation
This PR changes the logic for determining when to skip route processing from using the URL of the last attempted navigation to the actual resulting URL after that transition. Because guards may prevent navigation and reset the browser URL, the raw URL of the previous transition may not match the actual URL of the browser at the end of the navigation process. For that reason, we need to use `urlAfterRedirects` instead. Other notes: These checks in scheduleNavigation were added in eb2ceff The test still passes and, more surprisingly, passes if the checks are removed completely. There have likely been changes to the navigation handling that handle the test in a different way. That said, it still appears to be important to keep the checks there in some capacity because it does affect how many navigation events occur. This addresses an issue that came up in #16710: #16710 (comment) This also partially addresses #13586 in fixing history for imperative navigations that are cancelled by guards.
a72e895
to
2368c37
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
atscott commentedJun 24, 2020
This PR changes the logic for determining when to skip route processing from
using the URL of the last attempted navigation to the actual resulting URL after
that transition.
Because guards may prevent navigation and reset the browser URL, the raw
URL of the previous transition may not match the actual URL of the
browser at the end of the navigation process. For that reason, we need to use
urlAfterRedirects
if the previous navigation failed.Note that this is a resubmit of d3a8175 which caused a failure in Google. I've verified that this change does not result in the same failure but this PR should still be merged and synced separately.