-
Notifications
You must be signed in to change notification settings - Fork 26.7k
feat(migrations): Adds support for CommonModule to standalone migration #64138
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(migrations): Adds support for CommonModule to standalone migration #64138
Conversation
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.
Thanks for this.
Can you make sure to include the corresponding docs in the change, see adev/src/content/reference/migrations/overview.md
packages/core/schematics/test/common_to_standalone_migration_spec.ts
Outdated
Show resolved
Hide resolved
If I do it, I would first like to have your approval to be able to add it. I will do it on this same PR. |
|
Yes this is an optional migration that we'd like to have ! |
packages/core/schematics/test/common_to_standalone_migration_spec.ts
Outdated
Show resolved
Hide resolved
1827465 to
ae56ad6
Compare
|
Updated addressing the feedback, the only remaining question is whether to support NgModules or only standalone components. If it’s possible to consult this in some way, that would be great. |
5596c9b to
2f1954c
Compare
2f1954c to
4531859
Compare
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
4531859 to
477aac7
Compare
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.
Next round of comments. PTAL.
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Show resolved
Hide resolved
| private hasCommonModuleInImports(decorator: ts.Decorator): boolean { | ||
| if (!ts.isCallExpression(decorator.expression)) return false; | ||
|
|
||
| const config = decorator.expression.arguments[0]; |
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.
This is me thinking out loud:
I think this is unsafe if we hit a @Directive(), ie with 0 arguments. iirc ts.isObjectLiteralExpression(config) will throw if config is undefined.
But since we don't care about Directive decorator, we should only check component decorators.
Either way, we need to make the check more robust.
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.
Since we only process the Component decorator, this shouldn't happen. However, it could be "possible" that a user might have something strange with the decorator. I'm going to add a condition.
private analyzeClass(
node: ts.ClassDeclaration,
typeChecker: ts.TypeChecker,
): CommonModuleReference | null {
const nodeDecorators = ts.getDecorators(node);
if (!nodeDecorators) return null;
const decorators = getAngularDecorators(typeChecker, nodeDecorators);
// Only process Component decorators, not Directive or other Angular decorators
for (const decorator of decorators) {
if (decorator.name === 'Component') {
return this.analyzeComponentDecorator(node, decorator, typeChecker);
}
}
return null;
}
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
477aac7 to
12e59a5
Compare
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
12e59a5 to
7db8f18
Compare
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
7db8f18 to
fc8b738
Compare
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
fc8b738 to
c2e88ca
Compare
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/migration.ts
Outdated
Show resolved
Hide resolved
c2e88ca to
9abafde
Compare
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
9abafde to
93d7749
Compare
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
93d7749 to
8e76c83
Compare
packages/core/schematics/migrations/common-to-standalone-migration/util.ts
Outdated
Show resolved
Hide resolved
8e76c83 to
b0df11b
Compare
|
Can you please rebase. |
Introduces a migration that replaces CommonModule usage with individual imports from @angular/common, aligning with Angular's standalone component approach and improving module import clarity
b0df11b to
106c2d0
Compare
|
Updated |
|
This PR was merged into the repository. The changes were merged into the following branches:
|
|
Wait..? This is something else then EDIT: Ah yes so this will get rid of the old |
|
Those migrations are distinct and this one was introduced to help project follow the recommendations closely. |
Common to Standalone Migration
This migration replaces
CommonModuleimports with specific standalone imports from@angular/common.Usage
Example
Before:
After:
Benefits
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #60808
What is the new behavior?
Does this PR introduce a breaking change?
Other information