From f336ca0e0890a95bc2be547a627b26956b1fc2c1 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 13 Aug 2025 15:45:31 -0700 Subject: [PATCH] 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. --- packages/angular/cli/BUILD.bazel | 3 + .../cli/src/commands/mcp/mcp-server.ts | 3 +- .../analyze_for_unsupported_zone_uses.ts | 73 +++++ .../migrate_single_file.ts | 96 ++++++ .../migrate_single_file_spec.ts | 151 +++++++++ .../migrate_test_file.ts | 82 +++++ .../migrate_test_file_spec.ts | 69 ++++ .../onpush-zoneless-migration/prompts.ts | 299 ++++++++++++++++++ .../send_debug_message.ts | 23 ++ .../onpush-zoneless-migration/ts_utils.ts | 114 +++++++ .../tools/onpush-zoneless-migration/types.ts | 14 + .../zoneless-migration.ts | 180 +++++++++++ 12 files changed, 1106 insertions(+), 1 deletion(-) create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/analyze_for_unsupported_zone_uses.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file_spec.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file_spec.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/send_debug_message.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts_utils.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/types.ts create mode 100644 packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts diff --git a/packages/angular/cli/BUILD.bazel b/packages/angular/cli/BUILD.bazel index 409dbcd14000..b9cb67db331c 100644 --- a/packages/angular/cli/BUILD.bazel +++ b/packages/angular/cli/BUILD.bazel @@ -70,8 +70,10 @@ ts_project( "//:node_modules/@types/semver", "//:node_modules/@types/yargs", "//:node_modules/@types/yarnpkg__lockfile", + "//:node_modules/fast-glob", "//:node_modules/listr2", "//:node_modules/semver", + "//:node_modules/typescript", ], ) @@ -127,6 +129,7 @@ ts_project( ":node_modules/yargs", "//:node_modules/@types/semver", "//:node_modules/@types/yargs", + "//:node_modules/fast-glob", "//:node_modules/semver", ], ) diff --git a/packages/angular/cli/src/commands/mcp/mcp-server.ts b/packages/angular/cli/src/commands/mcp/mcp-server.ts index ceedc6374ad6..232a9f6fc2be 100644 --- a/packages/angular/cli/src/commands/mcp/mcp-server.ts +++ b/packages/angular/cli/src/commands/mcp/mcp-server.ts @@ -15,6 +15,7 @@ import { BEST_PRACTICES_TOOL } from './tools/best-practices'; import { DOC_SEARCH_TOOL } from './tools/doc-search'; import { FIND_EXAMPLE_TOOL } from './tools/examples'; import { MODERNIZE_TOOL } from './tools/modernize'; +import { ZONELESS_MIGRATION_TOOL } from './tools/onpush-zoneless-migration/zoneless-migration'; import { LIST_PROJECTS_TOOL } from './tools/projects'; import { AnyMcpToolDeclaration, registerTools } from './tools/tool-registry'; @@ -28,7 +29,7 @@ const STABLE_TOOLS = [BEST_PRACTICES_TOOL, DOC_SEARCH_TOOL, LIST_PROJECTS_TOOL] * The set of tools that are available but not enabled by default. * These tools are considered experimental and may have limitations. */ -const EXPERIMENTAL_TOOLS = [FIND_EXAMPLE_TOOL, MODERNIZE_TOOL] as const; +const EXPERIMENTAL_TOOLS = [FIND_EXAMPLE_TOOL, MODERNIZE_TOOL, ZONELESS_MIGRATION_TOOL] as const; export async function createMcpServer( options: { diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/analyze_for_unsupported_zone_uses.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/analyze_for_unsupported_zone_uses.ts new file mode 100644 index 000000000000..8322c622620b --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/analyze_for_unsupported_zone_uses.ts @@ -0,0 +1,73 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from 'typescript'; +import { createUnsupportedZoneUsagesMessage } from './prompts'; +import { getImportSpecifier } from './ts_utils'; +import { MigrationResponse } from './types'; + +export function analyzeForUnsupportedZoneUses(sourceFile: ts.SourceFile): MigrationResponse | null { + const ngZoneImport = getImportSpecifier(sourceFile, '@angular/core', 'NgZone'); + if (!ngZoneImport) { + return null; + } + const unsupportedUsages = findUnsupportedZoneUsages(sourceFile, ngZoneImport); + + if (unsupportedUsages.length === 0) { + return null; + } + + const locations = unsupportedUsages.map((node: ts.Node) => { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + + return `line ${line + 1}, character ${character + 1}: ${node.getText()}`; + }); + + return createUnsupportedZoneUsagesMessage(locations, sourceFile.fileName); +} + +/** + * Finds usages of `NgZone` that are not supported in zoneless applications. + * @param sourceFile The source file to check. + * @param ngZoneImport The import specifier for `NgZone`. + * @returns A list of nodes that are unsupported `NgZone` usages. + */ +export function findUnsupportedZoneUsages( + sourceFile: ts.SourceFile, + ngZoneImport: ts.ImportSpecifier, +): ts.Node[] { + const unsupportedUsages: ts.Node[] = []; + const ngZoneClassName = ngZoneImport.name.text; + + const staticMethods = new Set([ + 'isInAngularZone', + 'assertInAngularZone', + 'assertNotInAngularZone', + ]); + const instanceMethods = new Set(['onMicrotaskEmpty', 'onStable']); + + ts.forEachChild(sourceFile, function visit(node) { + if (ts.isPropertyAccessExpression(node)) { + const propertyName = node.name.text; + const expressionText = node.expression.getText(sourceFile); + + // Static: NgZone.method() + if (expressionText === ngZoneClassName && staticMethods.has(propertyName)) { + unsupportedUsages.push(node); + } + + // Instance: zone.method() or this.zone.method() + if (instanceMethods.has(propertyName)) { + unsupportedUsages.push(node); + } + } + ts.forEachChild(node, visit); + }); + + return unsupportedUsages; +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file.ts new file mode 100644 index 000000000000..9330fca69026 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file.ts @@ -0,0 +1,96 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; +import ts from 'typescript'; +import { analyzeForUnsupportedZoneUses } from './analyze_for_unsupported_zone_uses'; +import { migrateTestFile } from './migrate_test_file'; +import { generateZonelessMigrationInstructionsForComponent } from './prompts'; +import { sendDebugMessage } from './send_debug_message'; +import { getImportSpecifier } from './ts_utils'; +import { MigrationResponse } from './types'; + +export async function migrateSingleFile( + sourceFile: ts.SourceFile, + extras: RequestHandlerExtra, +): Promise { + const testBedSpecifier = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed'); + const isTestFile = sourceFile.fileName.endsWith('.spec.ts') || !!testBedSpecifier; + if (isTestFile) { + return migrateTestFile(sourceFile); + } + + const unsupportedZoneUseResponse = analyzeForUnsupportedZoneUses(sourceFile); + if (unsupportedZoneUseResponse) { + return unsupportedZoneUseResponse; + } + + let detectedStrategy: 'OnPush' | 'Default' | undefined; + let hasComponentDecorator = false; + + const componentSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Component'); + if (!componentSpecifier) { + sendDebugMessage(`No component decorator found in file: ${sourceFile.fileName}`, extras); + + return null; + } + + ts.forEachChild(sourceFile, function visit(node) { + if (detectedStrategy) { + return; // Already found, no need to traverse further + } + + if (ts.isDecorator(node) && ts.isCallExpression(node.expression)) { + const callExpr = node.expression; + if (callExpr.expression.getText(sourceFile) === 'Component') { + hasComponentDecorator = true; + if (callExpr.arguments.length > 0 && ts.isObjectLiteralExpression(callExpr.arguments[0])) { + const componentMetadata = callExpr.arguments[0]; + for (const prop of componentMetadata.properties) { + if ( + ts.isPropertyAssignment(prop) && + prop.name.getText(sourceFile) === 'changeDetection' + ) { + if ( + ts.isPropertyAccessExpression(prop.initializer) && + prop.initializer.expression.getText(sourceFile) === 'ChangeDetectionStrategy' + ) { + const strategy = prop.initializer.name.text; + if (strategy === 'OnPush' || strategy === 'Default') { + detectedStrategy = strategy; + + return; + } + } + } + } + } + } + } + ts.forEachChild(node, visit); + }); + + if ( + !hasComponentDecorator || + // component uses OnPush. We don't have anything more to do here. + detectedStrategy === 'OnPush' || + // Explicit default strategy, assume there's a reason for it (already migrated, or is a library that hosts Default components) and skip. + detectedStrategy === 'Default' + ) { + sendDebugMessage( + `Component decorator found with strategy: ${detectedStrategy} in file: ${sourceFile.fileName}. Skipping migration for file.`, + extras, + ); + + return null; + } + + // Component decorator found, but no change detection strategy. + return generateZonelessMigrationInstructionsForComponent(sourceFile.fileName); +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file_spec.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file_spec.ts new file mode 100644 index 000000000000..da2f59db0182 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_single_file_spec.ts @@ -0,0 +1,151 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; +import ts from 'typescript'; +import { migrateSingleFile } from './migrate_single_file'; + +const fakeExtras = { + sendDebugMessage: jasmine.createSpy(), + sendNotification: jasmine.createSpy(), +} as unknown as RequestHandlerExtra; + +describe('migrateSingleFile', () => { + it('should identify test files by extension', async () => { + const fileName = 'test.spec.ts'; + const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result?.content[0].text).toContain( + 'The test file `test.spec.ts` is not yet configured for zoneless change detection.' + + ' You need to enable it for the entire test suite and then identify which specific tests fail.', + ); + }); + + it('should identify test files by TestBed import', async () => { + const fileName = 'test.ts'; + const content = `import { TestBed } from '@angular/core/testing';`; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result?.content[0].text).toContain( + 'The test file `test.ts` is not yet configured for zoneless change detection.' + + ' You need to enable it for the entire test suite and then identify which specific tests fail.', + ); + }); + + it('should return unsupported zone usages message if NgZone is used', async () => { + const fileName = 'app.component.ts'; + const content = ` + import { Component, NgZone } from '@angular/core'; + + @Component({ + selector: 'app-root', + template: 'Hello', + }) + export class AppComponent { + constructor(private zone: NgZone) { + this.zone.onMicrotaskEmpty(() => {}); + } + } + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result?.content[0].text).toContain( + 'The component uses NgZone APIs that are incompatible with zoneless applications', + ); + }); + + it('should return null if component already has ChangeDetectionStrategy.OnPush', async () => { + const fileName = 'app.component.ts'; + const content = ` + import { Component, ChangeDetectionStrategy } from '@angular/core'; + + @Component({ + selector: 'app-root', + template: 'Hello', + changeDetection: ChangeDetectionStrategy.OnPush, + }) + export class AppComponent {} + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result).toBeNull(); + }); + + it('should return null if component has ChangeDetectionStrategy.Default', async () => { + const fileName = 'app.component.ts'; + const content = ` + import { Component, ChangeDetectionStrategy } from '@angular/core'; + + @Component({ + selector: 'app-root', + template: 'Hello', + changeDetection: ChangeDetectionStrategy.Default, + }) + export class AppComponent {} + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result).toBeNull(); + }); + + it('should return migration instructions for a component without a change detection strategy', async () => { + const fileName = 'app.component.ts'; + const content = ` + import { Component } from '@angular/core'; + + @Component({ + selector: 'app-root', + template: 'Hello', + }) + export class AppComponent {} + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result?.content[0].text).toContain( + 'The component does not currently use a change detection strategy, which means it may rely on Zone.js', + ); + }); + + it('should return null for a file that is not a component', async () => { + const fileName = 'some.service.ts'; + const content = ` + import { Injectable } from '@angular/core'; + + @Injectable({ providedIn: 'root' }) + export class SomeService {} + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result).toBeNull(); + }); + + it('should return null for an empty file', async () => { + const fileName = 'empty.ts'; + const content = ``; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateSingleFile(sourceFile, fakeExtras); + + expect(result).toBeNull(); + }); +}); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts new file mode 100644 index 000000000000..67b7480f73f9 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { glob } from 'fast-glob'; +import * as fs from 'fs'; +import { dirname, join } from 'path'; +import ts from 'typescript'; +import { createFixResponseForZoneTests, createProvideZonelessForTestsSetupPrompt } from './prompts'; +import { MigrationResponse } from './types'; + +export async function migrateTestFile( + sourceFile: ts.SourceFile, +): Promise { + // Check if tests use zoneless either by default through `initTestEnvironment` or by explicitly calling `provideZonelessChangeDetection`. + let testsUseZonelessChangeDetection = await searchForGlobalZoneless(sourceFile.fileName); + if (!testsUseZonelessChangeDetection) { + ts.forEachChild(sourceFile, function visit(node) { + if ( + ts.isCallExpression(node) && + node.expression.getText(sourceFile) === 'provideZonelessChangeDetection' + ) { + testsUseZonelessChangeDetection = true; + + return; + } + ts.forEachChild(node, visit); + }); + } + + if (!testsUseZonelessChangeDetection) { + // Tests do not use zoneless, so we provide instructions to set it up. + return createProvideZonelessForTestsSetupPrompt(sourceFile.fileName); + } + + // At this point, tests are using zoneless, so we look for any explicit uses of `provideZoneChangeDetection` that need to be fixed. + return createFixResponseForZoneTests(sourceFile); +} + +export async function searchForGlobalZoneless(startPath: string): Promise { + const angularJsonDir = findAngularJsonDir(startPath); + if (!angularJsonDir) { + // Cannot determine project root, fallback to original behavior or assume false. + // For now, let's assume no global setup if angular.json is not found. + return false; + } + + try { + const files = await glob(`${angularJsonDir}/**/*.ts`); + for (const file of files) { + const content = fs.readFileSync(file, 'utf-8'); + if ( + content.includes('initTestEnvironment') && + content.includes('provideZonelessChangeDetection') + ) { + return true; + } + } + } catch (e) { + return false; + } + + return false; +} + +function findAngularJsonDir(startDir: string): string | null { + let currentDir = startDir; + while (true) { + if (fs.existsSync(join(currentDir, 'angular.json'))) { + return currentDir; + } + const parentDir = dirname(currentDir); + if (parentDir === currentDir) { + return null; + } + currentDir = parentDir; + } +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file_spec.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file_spec.ts new file mode 100644 index 000000000000..268561d176b0 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file_spec.ts @@ -0,0 +1,69 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from 'typescript'; +import { migrateTestFile } from './migrate_test_file'; + +describe('migrateTestFile', () => { + it('should return setup prompt when zoneless is not detected', async () => { + const fileName = 'test.spec.ts'; + const sourceFile = ts.createSourceFile(fileName, '', ts.ScriptTarget.ESNext, true); + + const result = await migrateTestFile(sourceFile); + + expect(result?.content[0].text).toContain( + 'The test file `test.spec.ts` is not yet configured for zoneless change detection.', + ); + }); + + it('should return null when zoneless is enabled and there are no zonejs apis used', async () => { + const fileName = 'test.spec.ts'; + const content = ` + import { provideZonelessChangeDetection } from '@angular/core'; + import { TestBed } from '@angular/core/testing'; + + TestBed.configureTestingModule({ + providers: [provideZonelessChangeDetection()], + }); + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateTestFile(sourceFile); + + expect(result).toBeNull(); + }); + + it('should return fix prompt when zoneless is enabled and provideZoneChangeDetection is used', async () => { + const fileName = 'test.spec.ts'; + const content = ` + import { provideZonelessChangeDetection, provideZoneChangeDetection } from '@angular/core'; + import { TestBed } from '@angular/core/testing'; + + describe('suite', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideZonelessChangeDetection()], + }); + }); + + it('zone test', () => { + TestBed.configureTestingModule({ + providers: [provideZoneChangeDetection()], + }); + }); + }); + `; + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.ESNext, true); + + const result = await migrateTestFile(sourceFile); + + expect(result?.content[0].text).toContain( + 'You must refactor these tests to work in a zoneless environment and remove the `provideZoneChangeDetection` calls.', + ); + }); +}); diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts new file mode 100644 index 000000000000..1d063e1bcfba --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts @@ -0,0 +1,299 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from 'typescript'; +import { MigrationResponse } from './types'; + +/* eslint-disable max-len */ + +export function createProvideZonelessForTestsSetupPrompt(testFilePath: string): MigrationResponse { + const text = `You are an expert Angular developer assisting with a migration to zoneless. Your task is to update the test file at \`${testFilePath}\` to enable zoneless change detection and identify tests that are not yet compatible. + + Follow these instructions precisely. + + ### Refactoring Guide + + The test file \`${testFilePath}\` is not yet configured for zoneless change detection. You need to enable it for the entire test suite and then identify which specific tests fail. + + #### Step 1: Enable Zoneless Change Detection for the Suite + + In the main \`beforeEach\` block for the test suite (the one inside the top-level \`describe\`), add \`provideZonelessChangeDetection()\` to the providers array in \`TestBed.configureTestingModule\`. + + \`\`\`diff + + import { provideZonelessChangeDetection } from '@angular/core'; + + describe('MyComponent', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + // ... other configuration + providers: [ + + provideZonelessChangeDetection(), + // ... other providers + ] + }); + }); + }); + \`\`\` + + #### Step 2: Identify and Isolate Failing Tests + + After enabling zoneless detection for the suite, some tests will likely fail. Your next task is to identify these failing tests and temporarily revert them to use Zone.js-based change detection. This isolates them for a future fix. + + For each test (\`it\` block) or group of tests (\`describe\` block) that fails, override the testbed configuration to use \`provideZoneChangeDetection\`. + + \`\`\`typescript + // Example for a single failing test + it('should do something that fails in zoneless', () => { + TestBed.configureTestingModule({ + providers: [provideZoneChangeDetection()] + }); + // ... rest of the test + }); + + // Example for a group of failing tests + describe('tests that require Zone.js', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideZoneChangeDetection()] + }); + }); + + // ... all failing tests go here + }); + \`\`\` + + ### IMPORTANT: Rules and Constraints + + You must follow these rules without exception: + 1. **DO** add \`provideZonelessChangeDetection()\` to the top-level \`beforeEach\` block as instructed in Step 1. + 2. **DO** add \`provideZoneChangeDetection()\` to the providers for each individual test or group of tests that fail after the change in Step 1. + 3. **DO NOT** attempt to fix the logic of the failing tests at this stage. The goal is only to enable zoneless for the suite and isolate the incompatible tests. + 4. **DO NOT** make any other changes to the test file or any application code. + + ### Final Step + After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`; + + return createResponse(text); +} + +export function createUnsupportedZoneUsagesMessage( + usages: string[], + filePath: string, +): MigrationResponse { + const text = `You are an expert Angular developer assisting with a migration to zoneless. Your task is to refactor the component in ${filePath} to remove unsupported NgZone APIs. + +The component uses NgZone APIs that are incompatible with zoneless applications. The only permitted NgZone APIs are \`NgZone.run\` and \`NgZone.runOutsideAngular\`. + +The following usages are unsupported and must be fixed: +${usages.map((usage) => `- ${usage}`).join('\n')} + +Follow these instructions precisely to refactor the code. + +### Refactoring Guide + +#### 1. APIs to Remove (No Replacement) +The following methods have no replacement in a zoneless context and must be removed entirely: +- \`NgZone.assertInAngularZone\` +- \`NgZone.assertNotInAngularZone\` +- \`NgZone.isInAngularZone\` + +#### 2. APIs to Replace +The \`onMicrotaskEmpty\` and \`onStable\` observables must be replaced with modern Angular APIs. + +- **For single-event subscriptions** (e.g., using \`.pipe(take(1))\` or \`.pipe(first())\`), use \`afterNextRender\` from \`@angular/core\`. + + \`\`\`diff + - this.zone.onMicrotaskEmpty.pipe(take(1)).subscribe(() => {}); + - this.zone.onStable.pipe(take(1)).subscribe(() => {}); + + import { afterNextRender, Injector } from '@angular/core'; + + afterNextRender(() => {}, {injector: this.injector}); + \`\`\` + +- **For continuous subscriptions**, use \`afterEveryRender\` from \`@angular/core\`. + + \`\`\`diff + - this.zone.onMicrotaskEmpty.subscribe(() => {}); + - this.zone.onStable.subscribe(() => {}); + + import { afterEveryRender, Injector } from '@angular/core'; + + afterEveryRender(() => {}, {injector: this.injector}); + \`\`\` + +- If the code checks \`this.zone.isStable\` before subscribing, you can remove the \`isStable\` check. \`afterNextRender\` handles this case correctly. + +### IMPORTANT: Rules and Constraints +You must follow these rules without exception: +1. **DO NOT** make any changes to the component that are unrelated to removing the unsupported NgZone APIs listed above. +2. **DO NOT** remove or modify usages of \`NgZone.run\` or \`NgZone.runOutsideAngular\`. These are still required. +3. **DO** ensure that you replace \`onMicrotaskEmpty\` and \`onStable\` with the correct replacements (\`afterNextRender\` or \`afterEveryRender\`) as described in the guide. +4. **DO** add the necessary imports for \`afterNextRender\`, \`afterEveryRender\`, and \`Injector\` when you use them. + +### Final Step +After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process. +`; + + return createResponse(text); +} + +export function generateZonelessMigrationInstructionsForComponent( + filePath: string, +): MigrationResponse { + const text = `You are an expert Angular developer assisting with a migration to zoneless. Your task is to refactor the component in \`${filePath}\` to be compatible with zoneless change detection by ensuring Angular is notified of all state changes that affect the view. + + The component does not currently use a change detection strategy, which means it may rely on Zone.js. To prepare it for zoneless, you must manually trigger change detection when its state changes. + + Follow these instructions precisely. + + ### Refactoring Guide + + #### Step 1: Identify and Refactor State + Your primary goal is to ensure that every time a component property used in the template is updated, Angular knows it needs to run change detection. + + 1. **Identify Properties**: Find all component properties that are read by the template. + 2. **Choose a Strategy**: For each property identified, choose one of the following refactoring strategies: + * **(Preferred) Convert to Signal**: The best approach is to convert the property to an Angular Signal. This is the most idiomatic and future-proof way to handle state in zoneless applications. + * **(Alternative) Use \`markForCheck()\`**: If converting to a signal is too complex or would require extensive refactoring, you can instead inject \`ChangeDetectorRef\` and call \`this.cdr.markForCheck()\` immediately after the property is updated. + + #### Step 2: Add \`ChangeDetectionStrategy.Default\` + After you have refactored all necessary properties, you must update the component's decorator to explicitly set the change detection strategy. + + 1. Add \`ChangeDetectionStrategy\` to the import from \`@angular/core\`. + 2. In the \`@Component\` decorator, add the property \`changeDetection: ChangeDetectionStrategy.Default\`. + 3. Add a \`// TODO\` comment above this line explaining that the component should be fully migrated to \`OnPush\` after the application has been tested with these changes. + + Example: + \`\`\`typescript + @Component({ + ... + // TODO: This component has been partially migrated to be zoneless-compatible. + // After testing, this should be updated to ChangeDetectionStrategy.OnPush. + changeDetection: ChangeDetectionStrategy.Default, + }) + \`\`\` + + ### IMPORTANT: Rules and Constraints + You must follow these rules without exception: + 1. **DO** apply one of the two refactoring strategies (signals or \`markForCheck()\`) for all relevant component properties. + 2. **DO** add \`changeDetection: ChangeDetectionStrategy.Default\` with the specified TODO comment as the final code change. + 3. **DO NOT** use \`ChangeDetectionStrategy.OnPush\`. This will be the next step in the migration, but it is not part of this task. + 4. **DO NOT** modify properties that are already signals or are used with the \`async\` pipe in the template, as they are already zoneless-compatible. + 5. **DO NOT** make any changes to files other than the component file at \`${filePath}\` and its direct template/style files if necessary. + + ### Final Step + After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`; + + return createResponse(text); +} + +export function createTestDebuggingGuide(fileOrDirPath: string): MigrationResponse { + const text = `You are an expert Angular developer assisting with a migration to zoneless. + +No actionable migration steps were found in the application code for \`${fileOrDirPath}\`. However, if the tests for this code are failing with zoneless enabled, the tests themselves likely need to be updated. + +Your task is to investigate and fix any failing tests related to the code in \`${fileOrDirPath}\`. + +### Test Debugging Guide + +When running tests in a zoneless environment, you may encounter new types of failures. Here are common issues and how to resolve them: + +1. **\`ExpressionChangedAfterItHasBeenCheckedError\`**: + * **Cause**: This error indicates that a value in a component's template was updated, but Angular was not notified to run change detection. + * **Solution**: + * If the value is in a test-only wrapper component, update the property to be a signal. + * For application components, either convert the property to a signal or call \`ChangeDetectorRef.markForCheck()\` immediately after the property is updated. + +2. **Asynchronous Operations and Timing**: + * **Cause**: Without Zone.js, change detection is always scheduled asynchronously. Tests that previously relied on synchronous updates might now fail. The \`fixture.whenStable()\` utility also no longer waits for timers (like \`setTimeout\` or \`setInterval\`). + * **Solution**: + * Avoid relying on synchronous change detection. + * To wait for asynchronous operations to complete, you may need to poll for an expected state, use \`fakeAsync\` with \`tick()\`, or use a mock clock to flush timers. + +3. **Indirect Dependencies**: + * **Cause**: The component itself might be zoneless-compatible, but it could be using a service or another dependency that is not. + * **Solution**: Investigate the services and dependencies used by the component and its tests. Run this tool on those dependencies to identify and fix any issues. + +### IMPORTANT: Rules and Constraints + +You must follow these rules without exception: +1. **DO** focus only on fixing the tests for the code in \`${fileOrDirPath}\`. +2. **DO** apply the solutions described in the debugging guide to fix test failures. +3. **DO NOT** make changes to application code unless it is to fix a bug revealed by the zoneless migration (e.g., converting a property to a signal to fix an \`ExpressionChangedAfterItHasBeenCheckedError\`). +4. **DO NOT** re-introduce \`provideZoneChangeDetection()\` into tests that are already using \`provideZonelessChangeDetection()\`. +`; + + return createResponse(text); +} + +export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): MigrationResponse | null { + const usages: ts.Node[] = []; + ts.forEachChild(sourceFile, function visit(node) { + if ( + ts.isCallExpression(node) && + node.expression.getText(sourceFile) === 'provideZoneChangeDetection' + ) { + usages.push(node); + } + ts.forEachChild(node, visit); + }); + if (usages.length === 0) { + // No usages of provideZoneChangeDetection found, so no fix needed. + return null; + } + + const locations = usages.map((node) => { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + + return `line ${line + 1}, character ${character + 1}`; + }); + const text = `You are an expert Angular developer assisting with a migration to zoneless. Your task is to update the test file at \`${sourceFile.fileName}\` to be fully zoneless-compatible. + + The test suite has been partially migrated, but some tests were incompatible and are still using Zone.js-based change detection via \`provideZoneChangeDetection\`. You must refactor these tests to work in a zoneless environment and remove the \`provideZoneChangeDetection\` calls. + + The following usages of \`provideZoneChangeDetection\` must be removed: + ${locations.map((loc) => `- ${loc}`).join('\n')} + + ### Test Debugging Guide + + After removing \`provideZoneChangeDetection\`, the tests will likely fail. Use this guide to diagnose and fix the failures. + + 1. **\`ExpressionChangedAfterItHasBeenCheckedError\`**: + * **Cause**: This error indicates that a value in a component's template was updated, but Angular was not notified to run change detection. + * **Solution**: + * If the value is in a test-only wrapper component, update the property to be a signal. + * For application components, either convert the property to a signal or call \`ChangeDetectorRef.markForCheck()\` immediately after the property is updated. + + 2. **Asynchronous Operations and Timing**: + * **Cause**: Without Zone.js, change detection is always scheduled asynchronously. Tests that previously relied on synchronous updates might now fail. The \`fixture.whenStable()\` utility also no longer waits for timers (like \`setTimeout\` or \`setInterval\`). + * **Solution**: + * Avoid relying on synchronous change detection. + * To wait for asynchronous operations to complete, you may need to poll for an expected state, use \`fakeAsync\` with \`tick()\`, or use a mock clock to flush timers. + + 3. **Indirect Dependencies**: + * **Cause**: The component itself might be zoneless-compatible, but it could be using a service or another dependency that is not. + * **Solution**: Investigate the services and dependencies used by the component and its tests. Run this tool on those dependencies to identify and fix any issues. + + ### IMPORTANT: Rules and Constraints + + You must follow these rules without exception: + 1. **DO** remove all usages of \`provideZoneChangeDetection\` from the test file. + 2. **DO** apply the solutions described in the debugging guide to fix any resulting test failures. + 3. **DO NOT** make changes to application code unless it is to fix a bug revealed by the zoneless migration (e.g., converting a property to a signal to fix an \`ExpressionChangedAfterItHasBeenCheckedError\`). + 4. **DO NOT** make any changes unrelated to fixing the failing tests in \`${sourceFile.fileName}\`. + + ### Final Step + After you have applied all the required changes and followed all the rules, consult this tool again for the next steps in the migration process.`; + + return createResponse(text); +} + +/* eslint-enable max-len */ + +export function createResponse(text: string): MigrationResponse { + return { + content: [{ type: 'text', text }], + }; +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/send_debug_message.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/send_debug_message.ts new file mode 100644 index 000000000000..73a1b068a698 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/send_debug_message.ts @@ -0,0 +1,23 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; + +export function sendDebugMessage( + message: string, + { sendNotification }: RequestHandlerExtra, +): void { + void sendNotification({ + method: 'notifications/message', + params: { + level: 'debug', + data: message, + }, + }); +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts_utils.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts_utils.ts new file mode 100644 index 000000000000..958a8b86b194 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts_utils.ts @@ -0,0 +1,114 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import * as fs from 'fs'; +import ts from 'typescript'; + +/** + * Gets a top-level import specifier with a specific name that is imported from a particular module. + * E.g. given a file that looks like: + * + * ```ts + * import { Component, Directive } from '@angular/core'; + * import { Foo } from './foo'; + * ``` + * + * Calling `getImportSpecifier(sourceFile, '@angular/core', 'Directive')` will yield the node + * referring to `Directive` in the top import. + * + * @param sourceFile File in which to look for imports. + * @param moduleName Name of the import's module. + * @param specifierName Original name of the specifier to look for. Aliases will be resolved to + * their original name. + */ +export function getImportSpecifier( + sourceFile: ts.SourceFile, + moduleName: string | RegExp, + specifierName: string, +): ts.ImportSpecifier | null { + return getImportSpecifiers(sourceFile, moduleName, specifierName)[0] ?? null; +} + +/** + * Gets top-level import specifiers with specific names that are imported from a particular module. + * E.g. given a file that looks like: + * + * ```ts + * import { Component, Directive } from '@angular/core'; + * import { Foo } from './foo'; + * ``` + * + * Calling `getImportSpecifiers(sourceFile, '@angular/core', ['Directive', 'Component'])` will + * yield the nodes referring to `Directive` and `Component` in the top import. + * + * @param sourceFile File in which to look for imports. + * @param moduleName Name of the import's module. + * @param specifierOrSpecifiers Original name of the specifier to look for, or an array of such + * names. Aliases will be resolved to their original name. + */ +export function getImportSpecifiers( + sourceFile: ts.SourceFile, + moduleName: string | RegExp, + specifierOrSpecifiers: string | string[], +): ts.ImportSpecifier[] { + const matches: ts.ImportSpecifier[] = []; + for (const node of sourceFile.statements) { + if (!ts.isImportDeclaration(node) || !ts.isStringLiteral(node.moduleSpecifier)) { + continue; + } + + const namedBindings = node.importClause?.namedBindings; + const isMatch = + typeof moduleName === 'string' + ? node.moduleSpecifier.text === moduleName + : moduleName.test(node.moduleSpecifier.text); + + if (!isMatch || !namedBindings || !ts.isNamedImports(namedBindings)) { + continue; + } + + if (typeof specifierOrSpecifiers === 'string') { + const match = findImportSpecifier(namedBindings.elements, specifierOrSpecifiers); + if (match) { + matches.push(match); + } + } else { + for (const specifierName of specifierOrSpecifiers) { + const match = findImportSpecifier(namedBindings.elements, specifierName); + if (match) { + matches.push(match); + } + } + } + } + + return matches; +} + +/** + * Finds an import specifier with a particular name. + * @param nodes Array of import specifiers to search through. + * @param specifierName Name of the specifier to look for. + */ +export function findImportSpecifier( + nodes: ts.NodeArray, + specifierName: string, +): ts.ImportSpecifier | undefined { + return nodes.find((element) => { + const { name, propertyName } = element; + + return propertyName ? propertyName.text === specifierName : name.text === specifierName; + }); +} + +/** Creates a TypeScript source file from a file path. */ +export function createSourceFile(file: string) { + const content = fs.readFileSync(file, 'utf8'); + + return ts.createSourceFile(file, content, ts.ScriptTarget.Latest, true); +} diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/types.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/types.ts new file mode 100644 index 000000000000..e1619f83edb2 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/types.ts @@ -0,0 +1,14 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +export type MigrationResponse = { + content: { + type: 'text'; + text: string; + }[]; +}; diff --git a/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts new file mode 100644 index 000000000000..651c576ec672 --- /dev/null +++ b/packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts @@ -0,0 +1,180 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol'; +import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types'; +import { glob } from 'fast-glob'; +import * as fs from 'fs'; +import ts from 'typescript'; +import { z } from 'zod'; +import { declareTool } from '../tool-registry'; +import { analyzeForUnsupportedZoneUses } from './analyze_for_unsupported_zone_uses'; +import { migrateSingleFile } from './migrate_single_file'; +import { migrateTestFile } from './migrate_test_file'; +import { createTestDebuggingGuide } from './prompts'; +import { sendDebugMessage } from './send_debug_message'; +import { createSourceFile, getImportSpecifier } from './ts_utils'; + +export const ZONELESS_MIGRATION_TOOL = declareTool({ + name: 'onpush-zoneless-migration', + title: 'Plan migration to OnPush and/or zoneless', + description: + '**Required tool for migrating Angular components to OnPush change detection or zoneless.**' + + ' This tool orchestrates the entire migration process, including running prerequisite migrations' + + ' for signal inputs and queries. Use this tool as the first step before making any manual changes' + + ' to adopt `ChangeDetectionStrategy.OnPush` or `provideZonelessChangeDetection`.', + isReadOnly: true, + isLocalOnly: true, + inputSchema: { + fileOrDirPath: z + .string() + .describe( + 'The absolute path of the directory or file with the component(s), directive(s), or service(s) to migrate.' + + ' The contents are read with fs.readFileSync.', + ), + }, + factory: + () => + ({ fileOrDirPath }, requestHandlerExtra) => + registerZonelessMigrationTool(fileOrDirPath, requestHandlerExtra), +}); +export async function registerZonelessMigrationTool( + fileOrDirPath: string, + extras: RequestHandlerExtra, +) { + let files: ts.SourceFile[] = []; + const componentTestFiles = new Set(); + const filesWithComponents = new Set(); + const zoneFiles = new Set(); + + if (fs.statSync(fileOrDirPath).isDirectory()) { + const allFiles = await glob(`${fileOrDirPath}/**/*.ts`); + files = allFiles.map(createSourceFile); + } else { + files = [createSourceFile(fileOrDirPath)]; + const maybeTestFile = await getTestFilePath(fileOrDirPath); + if (maybeTestFile) { + componentTestFiles.add(createSourceFile(maybeTestFile)); + } + } + + for (const sourceFile of files) { + const content = sourceFile.getFullText(); + const componentSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Component'); + const zoneSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'NgZone'); + const testBedSpecifier = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed'); + if (testBedSpecifier) { + componentTestFiles.add(sourceFile); + } else if (componentSpecifier) { + if ( + !content.includes('changeDetectionStrategy: ChangeDetectionStrategy.OnPush') && + !content.includes('changeDetectionStrategy: ChangeDetectionStrategy.Default') + ) { + filesWithComponents.add(sourceFile); + } else { + sendDebugMessage( + `Component file already has change detection strategy: ${sourceFile.fileName}. Skipping migration.`, + extras, + ); + } + + const testFilePath = await getTestFilePath(sourceFile.fileName); + if (testFilePath) { + componentTestFiles.add(createSourceFile(testFilePath)); + } + } else if (zoneSpecifier) { + zoneFiles.add(sourceFile); + } + } + + if (zoneFiles.size > 0) { + for (const file of zoneFiles) { + const result = analyzeForUnsupportedZoneUses(file); + if (result !== null) { + return result; + } + } + } + + if (filesWithComponents.size > 0) { + const rankedFiles = + filesWithComponents.size > 1 + ? await rankComponentFilesForMigration(extras, Array.from(filesWithComponents)) + : Array.from(filesWithComponents); + + for (const file of rankedFiles) { + const result = await migrateSingleFile(file, extras); + if (result !== null) { + return result; + } + } + } + + for (const file of componentTestFiles) { + const result = await migrateTestFile(file); + if (result !== null) { + return result; + } + } + + return createTestDebuggingGuide(fileOrDirPath); +} + +async function rankComponentFilesForMigration( + { sendRequest }: RequestHandlerExtra, + componentFiles: ts.SourceFile[], +): Promise { + try { + const response = await sendRequest( + { + method: 'sampling/createMessage', + params: { + messages: [ + { + role: 'user', + content: { + type: 'text', + text: + `The following files are components that need to be migrated to OnPush change detection.` + + ` Please rank them based on which ones are most likely to be shared or common components.` + + ` The most likely shared component should be first. + ${componentFiles.map((f) => f.fileName).join('\n ')} + Respond ONLY with the ranked list of files, one file per line.`, + }, + }, + ], + systemPrompt: + 'You are a helpful assistant that helps migrate identify shared Angular components.', + maxTokens: 2000, + }, + }, + z.object({ sortedFiles: z.array(z.string()) }), + ); + + const rankedFiles = response.sortedFiles + .map((line) => line.trim()) + .map((fileName) => componentFiles.find((f) => f.fileName === fileName)) + .filter((f) => !!f); + + // Ensure the ranking didn't mess up the list of files + if (rankedFiles.length === componentFiles.length) { + return rankedFiles; + } + } catch {} + + return componentFiles; // Fallback to original order if the response fails +} + +async function getTestFilePath(filePath: string): Promise { + const testFilePath = filePath.replace(/\.ts$/, '.spec.ts'); + if (fs.existsSync(testFilePath)) { + return testFilePath; + } + + return undefined; +}