Skip to content

Commit 3e10340

Browse files
committed
refactor(@angular/cli): add a get Zoneless/OnPush MCP tool
This change adds a tool that identifies the next steps for migrating a project file or directory to Zoneless or OnPush. The priorities for migration are: 1. Find and report any unsupported uses of ZoneJS APIs. These are easily verifiable. If they exist, they need to be removed and the tool provides suggested replacements 2. Provide requirements for OnPush compatibility for any files with a Component. It is suggested to use an explicit `ChangeDetectionStrategy.Default` until it can be verified the migration is complete. The tool skips any components with explicit change detection strategy definitions. This is required since we have no way of statically verifying a component is compatible with OnPush, so we need some way to indicate the tool should move on from a component 3. When nothing is identified in the above two steps, move on to test files and suggest migrating those to use zoneless. This is the best method to verify that components are zoneless compatible.
1 parent eb42785 commit 3e10340

File tree

29 files changed

+1144
-41
lines changed

29 files changed

+1144
-41
lines changed

modules/testing/builder/src/test-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export async function browserBuild(
8181
scheduleOptions?: ScheduleOptions,
8282
): Promise<BrowserBuildOutput> {
8383
const run = await architect.scheduleTarget(target, overrides, scheduleOptions);
84-
const output = (await run.result) as BuilderOutput & { outputs: { path: string }[] };
84+
const output = await run.result;
8585
expect(output.success).toBe(true);
8686

8787
if (!output.success) {

packages/angular/cli/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ ts_project(
7070
"//:node_modules/@types/semver",
7171
"//:node_modules/@types/yargs",
7272
"//:node_modules/@types/yarnpkg__lockfile",
73+
"//:node_modules/fast-glob",
7374
"//:node_modules/listr2",
7475
"//:node_modules/semver",
76+
"//:node_modules/typescript",
7577
],
7678
)
7779

@@ -127,6 +129,7 @@ ts_project(
127129
":node_modules/yargs",
128130
"//:node_modules/@types/semver",
129131
"//:node_modules/@types/yargs",
132+
"//:node_modules/fast-glob",
130133
"//:node_modules/semver",
131134
],
132135
)

packages/angular/cli/src/commands/mcp/mcp-server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { BEST_PRACTICES_TOOL } from './tools/best-practices';
1515
import { DOC_SEARCH_TOOL } from './tools/doc-search';
1616
import { FIND_EXAMPLE_TOOL } from './tools/examples';
1717
import { MODERNIZE_TOOL } from './tools/modernize';
18+
import { ZONELESS_MIGRATION_TOOL } from './tools/onpush-zoneless-migration/zoneless-migration';
1819
import { LIST_PROJECTS_TOOL } from './tools/projects';
1920
import { AnyMcpToolDeclaration, registerTools } from './tools/tool-registry';
2021

@@ -28,7 +29,7 @@ const STABLE_TOOLS = [BEST_PRACTICES_TOOL, DOC_SEARCH_TOOL, LIST_PROJECTS_TOOL]
2829
* The set of tools that are available but not enabled by default.
2930
* These tools are considered experimental and may have limitations.
3031
*/
31-
const EXPERIMENTAL_TOOLS = [FIND_EXAMPLE_TOOL, MODERNIZE_TOOL] as const;
32+
const EXPERIMENTAL_TOOLS = [FIND_EXAMPLE_TOOL, MODERNIZE_TOOL, ZONELESS_MIGRATION_TOOL] as const;
3233

3334
export async function createMcpServer(
3435
options: {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import ts from 'typescript';
10+
import { createUnsupportedZoneUsagesMessage } from './prompts';
11+
import { getImportSpecifier } from './ts_utils';
12+
import { MigrationResponse } from './types';
13+
14+
export function analyzeForUnsupportedZoneUses(sourceFile: ts.SourceFile): MigrationResponse | null {
15+
const ngZoneImport = getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
16+
if (!ngZoneImport) {
17+
return null;
18+
}
19+
const unsupportedUsages = findUnsupportedZoneUsages(sourceFile, ngZoneImport);
20+
21+
if (unsupportedUsages.length === 0) {
22+
return null;
23+
}
24+
25+
const locations = unsupportedUsages.map((node: ts.Node) => {
26+
const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart());
27+
28+
return `line ${line + 1}, character ${character + 1}: ${node.getText()}`;
29+
});
30+
31+
return createUnsupportedZoneUsagesMessage(locations, sourceFile.fileName);
32+
}
33+
34+
/**
35+
* Finds usages of `NgZone` that are not supported in zoneless applications.
36+
* @param sourceFile The source file to check.
37+
* @param ngZoneImport The import specifier for `NgZone`.
38+
* @returns A list of nodes that are unsupported `NgZone` usages.
39+
*/
40+
export function findUnsupportedZoneUsages(
41+
sourceFile: ts.SourceFile,
42+
ngZoneImport: ts.ImportSpecifier,
43+
): ts.Node[] {
44+
const unsupportedUsages: ts.Node[] = [];
45+
const ngZoneClassName = ngZoneImport.name.text;
46+
47+
const staticMethods = new Set([
48+
'isInAngularZone',
49+
'assertInAngularZone',
50+
'assertNotInAngularZone',
51+
]);
52+
const instanceMethods = new Set(['onMicrotaskEmpty', 'onStable']);
53+
54+
ts.forEachChild(sourceFile, function visit(node) {
55+
if (ts.isPropertyAccessExpression(node)) {
56+
const propertyName = node.name.text;
57+
const expressionText = node.expression.getText(sourceFile);
58+
59+
// Static: NgZone.method()
60+
if (expressionText === ngZoneClassName && staticMethods.has(propertyName)) {
61+
unsupportedUsages.push(node);
62+
}
63+
64+
// Instance: zone.method() or this.zone.method()
65+
if (instanceMethods.has(propertyName)) {
66+
unsupportedUsages.push(node);
67+
}
68+
}
69+
ts.forEachChild(node, visit);
70+
});
71+
72+
return unsupportedUsages;
73+
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol';
10+
import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types';
11+
import ts from 'typescript';
12+
import { analyzeForUnsupportedZoneUses } from './analyze_for_unsupported_zone_uses';
13+
import { migrateTestFile } from './migrate_test_file';
14+
import { generateZonelessMigrationInstructionsForComponent } from './prompts';
15+
import { sendDebugMessage } from './send_debug_message';
16+
import { getImportSpecifier } from './ts_utils';
17+
import { MigrationResponse } from './types';
18+
19+
export async function migrateSingleFile(
20+
sourceFile: ts.SourceFile,
21+
extras: RequestHandlerExtra<ServerRequest, ServerNotification>,
22+
): Promise<MigrationResponse | null> {
23+
const testBedSpecifier = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed');
24+
const isTestFile = sourceFile.fileName.endsWith('.spec.ts') || !!testBedSpecifier;
25+
if (isTestFile) {
26+
return migrateTestFile(sourceFile);
27+
}
28+
29+
const unsupportedZoneUseResponse = analyzeForUnsupportedZoneUses(sourceFile);
30+
if (unsupportedZoneUseResponse) {
31+
return unsupportedZoneUseResponse;
32+
}
33+
34+
let detectedStrategy: 'OnPush' | 'Default' | undefined;
35+
let hasComponentDecorator = false;
36+
37+
const componentSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Component');
38+
if (!componentSpecifier) {
39+
sendDebugMessage(`No component decorator found in file: ${sourceFile.fileName}`, extras);
40+
41+
return null;
42+
}
43+
44+
ts.forEachChild(sourceFile, function visit(node) {
45+
if (detectedStrategy) {
46+
return; // Already found, no need to traverse further
47+
}
48+
49+
if (ts.isDecorator(node) && ts.isCallExpression(node.expression)) {
50+
const callExpr = node.expression;
51+
if (callExpr.expression.getText(sourceFile) === 'Component') {
52+
hasComponentDecorator = true;
53+
if (callExpr.arguments.length > 0 && ts.isObjectLiteralExpression(callExpr.arguments[0])) {
54+
const componentMetadata = callExpr.arguments[0];
55+
for (const prop of componentMetadata.properties) {
56+
if (
57+
ts.isPropertyAssignment(prop) &&
58+
prop.name.getText(sourceFile) === 'changeDetection'
59+
) {
60+
if (
61+
ts.isPropertyAccessExpression(prop.initializer) &&
62+
prop.initializer.expression.getText(sourceFile) === 'ChangeDetectionStrategy'
63+
) {
64+
const strategy = prop.initializer.name.text;
65+
if (strategy === 'OnPush' || strategy === 'Default') {
66+
detectedStrategy = strategy;
67+
68+
return;
69+
}
70+
}
71+
}
72+
}
73+
}
74+
}
75+
}
76+
ts.forEachChild(node, visit);
77+
});
78+
79+
if (
80+
!hasComponentDecorator ||
81+
// component uses OnPush. We don't have anything more to do here.
82+
detectedStrategy === 'OnPush' ||
83+
// Explicit default strategy, assume there's a reason for it (already migrated, or is a library that hosts Default components) and skip.
84+
detectedStrategy === 'Default'
85+
) {
86+
sendDebugMessage(
87+
`Component decorator found with strategy: ${detectedStrategy} in file: ${sourceFile.fileName}. Skipping migration for file.`,
88+
extras,
89+
);
90+
91+
return null;
92+
}
93+
94+
// Component decorator found, but no change detection strategy.
95+
return generateZonelessMigrationInstructionsForComponent(sourceFile.fileName);
96+
}
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol';
10+
import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types';
11+
import ts from 'typescript';
12+
import { migrateSingleFile } from './migrate_single_file';
13+
14+
const fakeExtras = {
15+
sendDebugMessage: jasmine.createSpy(),
16+
sendNotification: jasmine.createSpy(),
17+
} as unknown as RequestHandlerExtra<ServerRequest, ServerNotification>;
18+
19+
describe('migrateSingleFile', () => {
20+
it('should identify test files by extension', async () => {
21+
const fileName = 'test.spec.ts';
22+
const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ESNext, true);
23+
24+
const result = await migrateSingleFile(sourceFile, fakeExtras);
25+
26+
expect(result?.content[0].text).toContain(
27+
'The test file `test.spec.ts` is not yet configured for zoneless change detection.' +
28+
' You need to enable it for the entire test suite and then identify which specific tests fail.',
29+
);
30+
});
31+
32+
it('should identify test files by TestBed import', async () => {
33+
const fileName = 'test.ts';
34+
const content = `import { TestBed } from '@angular/core/testing';`;
35+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
36+
37+
const result = await migrateSingleFile(sourceFile, fakeExtras);
38+
39+
expect(result?.content[0].text).toContain(
40+
'The test file `test.ts` is not yet configured for zoneless change detection.' +
41+
' You need to enable it for the entire test suite and then identify which specific tests fail.',
42+
);
43+
});
44+
45+
it('should return unsupported zone usages message if NgZone is used', async () => {
46+
const fileName = 'app.component.ts';
47+
const content = `
48+
import { Component, NgZone } from '@angular/core';
49+
50+
@Component({
51+
selector: 'app-root',
52+
template: 'Hello',
53+
})
54+
export class AppComponent {
55+
constructor(private zone: NgZone) {
56+
this.zone.onMicrotaskEmpty(() => {});
57+
}
58+
}
59+
`;
60+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
61+
62+
const result = await migrateSingleFile(sourceFile, fakeExtras);
63+
64+
expect(result?.content[0].text).toContain(
65+
'The component uses NgZone APIs that are incompatible with zoneless applications',
66+
);
67+
});
68+
69+
it('should return null if component already has ChangeDetectionStrategy.OnPush', async () => {
70+
const fileName = 'app.component.ts';
71+
const content = `
72+
import { Component, ChangeDetectionStrategy } from '@angular/core';
73+
74+
@Component({
75+
selector: 'app-root',
76+
template: 'Hello',
77+
changeDetection: ChangeDetectionStrategy.OnPush,
78+
})
79+
export class AppComponent {}
80+
`;
81+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
82+
83+
const result = await migrateSingleFile(sourceFile, fakeExtras);
84+
85+
expect(result).toBeNull();
86+
});
87+
88+
it('should return null if component has ChangeDetectionStrategy.Default', async () => {
89+
const fileName = 'app.component.ts';
90+
const content = `
91+
import { Component, ChangeDetectionStrategy } from '@angular/core';
92+
93+
@Component({
94+
selector: 'app-root',
95+
template: 'Hello',
96+
changeDetection: ChangeDetectionStrategy.Default,
97+
})
98+
export class AppComponent {}
99+
`;
100+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
101+
102+
const result = await migrateSingleFile(sourceFile, fakeExtras);
103+
104+
expect(result).toBeNull();
105+
});
106+
107+
it('should return migration instructions for a component without a change detection strategy', async () => {
108+
const fileName = 'app.component.ts';
109+
const content = `
110+
import { Component } from '@angular/core';
111+
112+
@Component({
113+
selector: 'app-root',
114+
template: 'Hello',
115+
})
116+
export class AppComponent {}
117+
`;
118+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
119+
120+
const result = await migrateSingleFile(sourceFile, fakeExtras);
121+
122+
expect(result?.content[0].text).toContain(
123+
'The component does not currently use a change detection strategy, which means it may rely on Zone.js',
124+
);
125+
});
126+
127+
it('should return null for a file that is not a component', async () => {
128+
const fileName = 'some.service.ts';
129+
const content = `
130+
import { Injectable } from '@angular/core';
131+
132+
@Injectable({ providedIn: 'root' })
133+
export class SomeService {}
134+
`;
135+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
136+
137+
const result = await migrateSingleFile(sourceFile, fakeExtras);
138+
139+
expect(result).toBeNull();
140+
});
141+
142+
it('should return null for an empty file', async () => {
143+
const fileName = 'empty.ts';
144+
const content = ``;
145+
const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true);
146+
147+
const result = await migrateSingleFile(sourceFile, fakeExtras);
148+
149+
expect(result).toBeNull();
150+
});
151+
});

0 commit comments

Comments
 (0)