Skip to content

Add functional router guards and resolvers#24170

Merged
alan-agius4 merged 2 commits intoangular:mainfrom
atscott:functionalGuardsAndResolvers
Nov 9, 2022
Merged

Add functional router guards and resolvers#24170
alan-agius4 merged 2 commits intoangular:mainfrom
atscott:functionalGuardsAndResolvers

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Nov 2, 2022

Functional guards and resolvers were introduced in the Angular router in v14.2.

This commit adds the ability to generate functional router guards by specifying --guardType instead of --implements. These guards are also accompanied by a test that includes a helper function for executing the guard in the TestBed environment so that any inject calls in the guard will work properly. Functional resolvers are generated by adding the --functional flag.

@atscott atscott added the target: minor This PR is targeted for the next minor release label Nov 2, 2022
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Nov 2, 2022
@atscott atscott changed the title Functional guards and resolvers Add functional router guards Nov 2, 2022
@atscott atscott changed the title Add functional router guards Add functional router guards and resolvers Nov 2, 2022
@atscott atscott force-pushed the functionalGuardsAndResolvers branch 2 times, most recently from f3c8edf to 1a11ea1 Compare November 2, 2022 19:44
@atscott atscott requested a review from alan-agius4 November 2, 2022 19:54
Comment thread packages/schematics/angular/guard/schema.json
Comment thread packages/schematics/angular/guard/schema.json
Comment thread packages/schematics/angular/resolver/schema.json Outdated
Comment thread packages/schematics/angular/guard/files/__name@dasherize__.guard.spec.ts.template Outdated
Comment thread packages/schematics/angular/guard/schema.json Outdated
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

I am concerned that this is happing outside of a major version. As there are 2 changes in behaviour here which technically are breaking changes.

@atscott
Copy link
Copy Markdown
Contributor Author

atscott commented Nov 3, 2022

I am concerned that this is happing outside of a major version. As there are 2 changes in behaviour here which technically are breaking changes.

I don't know what the CLI team considers breaking, so you'll have to help determine what we're allowed to do here. I assumed, maybe incorrectly, that keeping all the options the same but changing the defaults would be fine.

@atscott atscott force-pushed the functionalGuardsAndResolvers branch 5 times, most recently from 4d8e2ed to 4c34666 Compare November 3, 2022 17:36
@angular-robot angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Nov 3, 2022
@atscott atscott force-pushed the functionalGuardsAndResolvers branch 2 times, most recently from 77c2a9c to 20b392d Compare November 3, 2022 20:18
@atscott atscott force-pushed the functionalGuardsAndResolvers branch from 20b392d to 78482b3 Compare November 3, 2022 20:38
Comment thread packages/schematics/angular/resolver/schema.json Outdated
Comment thread packages/schematics/angular/guard/index.ts Outdated
Comment thread packages/schematics/angular/guard/index.ts Outdated
Comment thread packages/schematics/angular/guard/index.ts Outdated
Comment thread packages/schematics/angular/guard/index.ts Outdated
"type": "string"
},
"default": ["CanActivate"],
"x-prompt": "Which interfaces would you like to implement?"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the prompt in this case should be removed. Since otherwise there wouldn't be a way to use guardType.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming there's not a way to make the prompt conditional? I feel like the lack of a prompt is a step down in UX.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Correct. we don't support conditional prompts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I tried this out and I don't think this is going to work. The experience now is that ng g guard myGuard will just generate a canActivate guard automatically. Most users will not know about the implements flag and will not know how to generate a guard of another type. We'll have to come up with another way to make this work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I updated the approach to use the --functional flag and throw an error if there is more than 1 value for implements when using it. The names of the existing implements options are the interfaces, and while this isn't ideal, I don't think it's really an issue.

PTAL

@alan-agius4 alan-agius4 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 Nov 7, 2022
@atscott atscott force-pushed the functionalGuardsAndResolvers branch 4 times, most recently from 45bc058 to 9589adb Compare November 7, 2022 19:43
@atscott atscott removed the detected: deprecation PR contains a commit with a deprecation label Nov 7, 2022
@atscott atscott force-pushed the functionalGuardsAndResolvers branch 4 times, most recently from 16bf2e3 to 4a6495b Compare November 7, 2022 20:41
Comment thread packages/schematics/angular/BUILD.bazel Outdated
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, one little nit.

…outer guards and resolvers

Functional guards and resolvers were introduced in the Angular router in v14.2.
This commit adds the ability to generate functional router guards by
specifying `--guardType` instead of `--implements`. These guards are also
accompanied by a test that includes a helper function for executing the
guard in the `TestBed` environment so that any `inject` calls in the
guard will work properly. Functional resolvers are generated by adding
the `--functional` flag.
@atscott atscott force-pushed the functionalGuardsAndResolvers branch from 4a6495b to 1a1dd53 Compare November 8, 2022 18:40
@atscott atscott removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Nov 8, 2022
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Nov 9, 2022
@alan-agius4 alan-agius4 merged commit b34c719 into angular:main Nov 9, 2022
@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 Dec 10, 2022
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 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.

3 participants