Skip to content

feat(router): helper functions to convert class guards to functional#48709

Closed
atscott wants to merge 3 commits intoangular:mainfrom
atscott:fromTokensHelper
Closed

feat(router): helper functions to convert class guards to functional#48709
atscott wants to merge 3 commits intoangular:mainfrom
atscott:fromTokensHelper

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Jan 12, 2023

This commit introduces helper functions to easily convert Injectables with functions compatible with Route guards to the corresponding guard functions. These functions will serve to aid in migrating off of the now deprecated class-based guards, but also provide an easy avenue to still defining guards as Injectable classes if that is desired.

Reviewer notes: Open to bikeshedding the API here. We could just have separate functions for each type of guard. I thought grouping them would help in discovery but maybe it's more confusing than the alternative.

@atscott atscott added area: router target: minor This PR is targeted for the next minor release labels Jan 12, 2023
@ngbot ngbot bot added this to the Backlog milestone Jan 12, 2023
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jan 12, 2023
@atscott atscott force-pushed the fromTokensHelper branch 3 times, most recently from 2f3f1b3 to 687d916 Compare January 12, 2023 01:43
@atscott atscott force-pushed the fromTokensHelper branch 3 times, most recently from 8ab452c to 9dd2717 Compare January 26, 2023 20:32
@pullapprove pullapprove bot requested a review from josephperrott January 26, 2023 20:32
Copy link
Copy Markdown
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: dev-infra

Copy link
Copy Markdown
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api, global-docs-approvers

Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

This commit introduces helper functions to easily convert `Injectable`s with
functions compatible with `Route` guards to the corresponding guard functions.
These functions will serve to aid in migrating off of the now deprecated
class-based guards, but also provide an easy avenue to still defining
guards as `Injectable` classes if that is desired.
@atscott atscott force-pushed the fromTokensHelper branch 2 times, most recently from 3575388 to d101d04 Compare February 15, 2023 22:16
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Feb 27, 2023
@AndrewKushnir
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 455c728.

@destus90
Copy link
Copy Markdown
Contributor

destus90 commented Mar 1, 2023

@atscott
Does the Angular team plan to migrate existing code in favor to use these functions or should we do it ourselves?

@atscott
Copy link
Copy Markdown
Contributor Author

atscott commented Mar 1, 2023

@destus90 there will be a migration

@angular-automatic-lock-bot
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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 1, 2023
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 area: router detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants