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(router): fix navigation ignoring logic to compare to the browser url #37716

Draft
wants to merge 1 commit into
base: master
from

Conversation

@atscott
Copy link
Contributor

atscott commented Jun 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.

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.
@atscott atscott force-pushed the atscott:imperativenavtest-resubmit branch from a72e895 to 2368c37 Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.