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

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

Closed
wants to merge 3 commits into from

Conversation

atscott
Copy link
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
Copy link
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
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
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
Contributor

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

@destus90
Copy link
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
Contributor Author

atscott commented Mar 1, 2023

@destus90 there will be a migration

@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 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.

None yet

6 participants