Skip to content

feat(router): Add ability to return UrlTree with NavigationBehaviorOptions from guards - #45023

Closed
atscott wants to merge 1 commit into
angular:mainfrom
atscott:urltreeguardoptions
Closed

feat(router): Add ability to return UrlTree with NavigationBehaviorOptions from guards#45023
atscott wants to merge 1 commit into
angular:mainfrom
atscott:urltreeguardoptions

Conversation

@atscott

@atscott atscott commented Feb 8, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Returning UrlTree from a guard was a convenient new feature added to
the Router. However, it does not have feature-parity with the old
router.navigate(...); return false; pattern. The most common use-case
for this feature is to redirect to a new page without updating the URL
from the initially attempted navigation. For example, rendering a 404
page when the user does not have access privelages to a route.

Fixes #27148

@ngbot ngbot Bot modified the milestone: Backlog Feb 8, 2022
@atscott atscott added the target: minor This PR is targeted for the next minor release label Feb 8, 2022
@atscott
atscott force-pushed the urltreeguardoptions branch 7 times, most recently from 3ac4a45 to 69e0965 Compare February 9, 2022 01:32
@atscott
atscott force-pushed the urltreeguardoptions branch from 69e0965 to 026813a Compare February 9, 2022 17:17
@mary-poppins

This comment was marked as outdated.

Sign in to view
@atscott

atscott commented Feb 9, 2022

Copy link
Copy Markdown
Contributor Author

Results from running migration the migration internally: cl/427481595

@atscott
atscott force-pushed the urltreeguardoptions branch from 026813a to bfca0cf Compare February 9, 2022 18:22
@atscott
atscott requested a review from AndrewKushnir February 9, 2022 18:54
@atscott
atscott marked this pull request as ready for review February 9, 2022 18:54
@pullapprove
pullapprove Bot requested review from alxhub and crisbeto February 9, 2022 18:54
@atscott atscott added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Feb 9, 2022
@atscott

atscott commented Feb 9, 2022

Copy link
Copy Markdown
Contributor Author

presubmit

atscott
atscott commented Feb 9, 2022
View reviewed changes
Comment thread goldens/public-api/router/router.md Outdated
AndrewKushnir
AndrewKushnir reviewed Feb 14, 2022
View reviewed changes

@AndrewKushnir AndrewKushnir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, just added a few comments.

Comment thread goldens/public-api/router/router.md Outdated
Comment thread packages/core/schematics/migrations.json Outdated
Comment thread packages/router/src/router.ts Outdated
Comment thread packages/router/src/apply_redirects.ts Outdated
Comment thread packages/router/src/models.ts Outdated
@atscott
atscott force-pushed the urltreeguardoptions branch 3 times, most recently from e19d04e to 78ca5eb Compare February 15, 2022 00:56
@atscott
atscott marked this pull request as draft November 17, 2022 17:31
@rhutchison

rhutchison commented Feb 2, 2023

Copy link
Copy Markdown

@atscott thanks for your work on this. Has there been any further discussions or change in direction?

@atscott

atscott commented Feb 2, 2023

Copy link
Copy Markdown
Contributor Author

@rhutchison I do still see value in this but work on it has dropped pretty significantly off my priority list. I would say the deprecation and migration of returning just UrlTree was/is the biggest blocker due to the time consumption and churn it would create. I might pick this up again as a feature-only change that removes the deprecation, at least for the time-being.

@Majies

Majies commented Jul 17, 2023

Copy link
Copy Markdown

@atscott I know this isn't the top of the priority list at the moment, but do we have any update? also are you still recommending the work around stated here - #27148 (comment)

@jessicajaniuk
jessicajaniuk removed their request for review August 31, 2023 18:38
@atscott
atscott force-pushed the urltreeguardoptions branch from c1f9ec9 to 69ce3f3 Compare November 16, 2023 15:57
@angular-robot angular-robot Bot added the detected: feature PR contains a feature commit label Nov 16, 2023
@atscott atscott mentioned this pull request Dec 4, 2023
Closed
@atscott
atscott force-pushed the urltreeguardoptions branch from 69ce3f3 to bcae363 Compare February 20, 2024 17:43
@angular-robot angular-robot Bot added area: docs Related to the documentation area: build & ci Related the build and CI infrastructure of the project labels Feb 20, 2024
@atscott
atscott force-pushed the urltreeguardoptions branch from bcae363 to e4c7f53 Compare February 20, 2024 17:48
@angular-robot angular-robot Bot removed area: docs Related to the documentation area: build & ci Related the build and CI infrastructure of the project labels Feb 20, 2024
@atscott
atscott force-pushed the urltreeguardoptions branch 2 times, most recently from aaab2ac to c7a13d5 Compare February 20, 2024 17:52
@angular-robot angular-robot Bot added the detected: breaking change PR contains a commit with a breaking change label Feb 20, 2024
@atscott
atscott force-pushed the urltreeguardoptions branch from c7a13d5 to 94aca9f Compare February 20, 2024 17:55
@atscott
atscott marked this pull request as ready for review February 20, 2024 18:11
@atscott
atscott force-pushed the urltreeguardoptions branch from 94aca9f to 42d58d3 Compare February 22, 2024 01:08
@atscott atscott added target: major This PR is targeted for the next major release and removed target: minor This PR is targeted for the next minor release labels Feb 22, 2024
@atscott atscott mentioned this pull request Feb 22, 2024
Closed
@atscott
feat(router): Add ability to return UrlTree with `NavigationBehavio…
850c3b8 
…rOptions` from guards

Returning `UrlTree` from a guard was a convenient new feature added to
the `Router`. However, it does not have feature-parity with the old
`router.navigate(...); return false;` pattern. The most common use-case
for this feature is to redirect to a new page _without_ updating the URL
from the initially attempted navigation. For example, rendering a 404
page when the user does not have access privelages to a route.

Fixes angular#17004
Fixes angular#27148

BREAKING CHANGE: Guards can now return `RedirectCommand` for redirects
in addition to `UrlTree`. Code which expects only `boolean` or `UrlTree`
values in `Route` types will need to be adjusted.
@atscott
atscott force-pushed the urltreeguardoptions branch from 42d58d3 to 850c3b8 Compare February 28, 2024 22:13
alxhub
alxhub approved these changes Mar 11, 2024
View reviewed changes

@alxhub alxhub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed-for: public-api

jessicajaniuk
jessicajaniuk approved these changes Mar 11, 2024
View reviewed changes

@jessicajaniuk jessicajaniuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed-for: public-api

@atscott

atscott commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

TGP=https://fusion2.corp.google.com/presubmit/583045890/OCL:583045890:BASE:614718613:1710212556271:4b5e09e3/targets

@atscott

atscott commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

This PR was merged into the repository by commit 8735af0.

@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Apr 12, 2024

Copy link
Copy Markdown

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alxhub alxhub alxhub approved these changes
@crisbeto crisbeto crisbeto left review comments
@AndrewKushnir AndrewKushnir Awaiting requested review from AndrewKushnir
@dylhunn dylhunn Awaiting requested review from dylhunn
+1 more reviewer
@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: router detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit target: major This PR is targeted for the next major release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

NavigationExtras when returning URLTree from guard

8 participants

@atscott @mary-poppins @rhutchison @Majies @alxhub @crisbeto @AndrewKushnir @jessicajaniuk