-
Notifications
You must be signed in to change notification settings - Fork 27k
feat(migrations): add a combined migration for all signals APIs #58259
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
Conversation
Adds a combined `@angular/core:signals` migration that combines all of the signals-related migrations into one for the apps that want to do it all in one go. All of the heavy-lifting was already done by the individual migrations, these changes only chain them together for a more convenient developer experience.
| for (const migration of migrations) { | ||
| switch (migration) { | ||
| case SupportedMigrations.inputs: | ||
| rules.push(toSignalInputs(options)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the devkit has a RunSchematicTask which could've been used to execute the other schematics. I didn't use it, because:
- I couldn't actually get it to work. This might be a unit-test-only issue since we use that API in the CDK and it seems to work fine.
- This gives us better type safety in case the options for the other schematics change.
| "schema": "./ng-generate/signal-queries-migration/schema.json", | ||
| "aliases": ["signal-queries", "signal-query", "signal-query-migration"] | ||
| }, | ||
| "signals": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was one the fence whether to call it something like signals-combined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, if we include output at some point, should we name it something like function-apis and have signals as an alias?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good point. My concern would be discoverability since while we call the APIs "function" within the team, they're very much tied to signals in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, so maybe signals with the alias for function-api etc?
devversion
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it. Thank you!
|
This PR was merged into the repository by commit dff4de0. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds a combined
@angular/core:signalsmigration that combines all of the signals-related migrations into one for the apps that want to do it all in one go.All of the heavy-lifting was already done by the individual migrations, these changes only chain them together for a more convenient developer experience.