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): ensure URL is updated after second redirect with UrlUpda… #27523

Closed

Conversation

jasonaden
Copy link
Contributor

…teStrategy="eager"

Navigating to a route such as /users, you may get redirected to /login. Previously, if you go then route to /users again the URL will end up showing /users after the second redirect. This only happened in UrlUpdateStrategy="eager". This is now fixed so after the second redirect, the URL shows the correct page.

Fixes #27116

…teStrategy="eager"

Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes angular#27116
@mary-poppins
Copy link

You can preview 75b684d at https://pr27523-75b684d.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 78c9072 at https://pr27523-78c9072.ngbuilds.io/.

@jasonaden
Copy link
Contributor Author

Presubmit

@jasonaden jasonaden requested a review from alxhub December 6, 2018 23:23
@jasonaden jasonaden added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Dec 6, 2018
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed in person. LGTM!

@mhevery
Copy link
Contributor

mhevery commented Dec 13, 2018

Did not merge into patch. Need a separate PR

@mhevery mhevery closed this in ad26cd6 Dec 13, 2018
mhevery added a commit that referenced this pull request Dec 14, 2018
… UrlUpdateStrategy="eager" (#27523)"

This reverts commit ad26cd6.
@mhevery mhevery reopened this Dec 14, 2018
@mhevery mhevery added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 14, 2018
@mhevery
Copy link
Contributor

mhevery commented Dec 14, 2018

This PR needs to be fixed up as it breaks G3.

@jasonaden jasonaden added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Dec 18, 2018
@jasonaden
Copy link
Contributor Author

Presubmit after fix to g3 that should allow this to pass.

@jasonaden jasonaden added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Dec 19, 2018
@matsko matsko closed this in 07ada7f Dec 20, 2018
matsko added a commit to matsko/angular that referenced this pull request Dec 26, 2018
matsko added a commit that referenced this pull request Dec 26, 2018
jasonaden added a commit to jasonaden/angular that referenced this pull request Jan 22, 2019
…ect with UrlUpdateStrategy="eager" (angular#27523)"

This reverts commit eea2b0f.
alxhub pushed a commit that referenced this pull request Jan 22, 2019
…ect with UrlUpdateStrategy="eager" (#27523)" (#28300)

This reverts commit eea2b0f.

PR Close #28300
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
…teStrategy="eager" (angular#27523)

Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes angular#27116

PR Close angular#27523
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
…teStrategy="eager" (angular#27523)

Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes angular#27116

PR Close angular#27523
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
ngfelixl pushed a commit to ngfelixl/angular that referenced this pull request Jan 28, 2019
vetom pushed a commit to vetom/angular that referenced this pull request Jan 31, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Url is only updated the first time with UrlUpdateStrategy = "eager"
5 participants