Skip to content

Commit 9b5e46a

Browse files
committed
fixup! refactor(@angular/cli): add a get Zoneless/OnPush MCP tool
1 parent f336ca0 commit 9b5e46a

5 files changed

Lines changed: 39 additions & 61 deletions

File tree

packages/angular/cli/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ ts_project(
7070
"//:node_modules/@types/semver",
7171
"//:node_modules/@types/yargs",
7272
"//:node_modules/@types/yarnpkg__lockfile",
73-
"//:node_modules/fast-glob",
7473
"//:node_modules/listr2",
7574
"//:node_modules/semver",
7675
"//:node_modules/typescript",

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/migrate_test_file.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { glob } from 'fast-glob';
10-
import * as fs from 'fs';
11-
import { dirname, join } from 'path';
9+
import { glob } from 'node:fs/promises';
10+
import * as fs from 'node:fs';
11+
import { dirname, join } from 'node:path';
1212
import ts from 'typescript';
1313
import { createFixResponseForZoneTests, createProvideZonelessForTestsSetupPrompt } from './prompts';
1414
import { MigrationResponse } from './types';
@@ -51,7 +51,7 @@ export async function searchForGlobalZoneless(startPath: string): Promise<boolea
5151

5252
try {
5353
const files = await glob(`${angularJsonDir}/**/*.ts`);
54-
for (const file of files) {
54+
for await (const file of files) {
5555
const content = fs.readFileSync(file, 'utf-8');
5656
if (
5757
content.includes('initTestEnvironment') &&

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/prompts.ts

Lines changed: 21 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,6 @@ export function createProvideZonelessForTestsSetupPrompt(testFilePath: string):
5454
});
5555
// ... rest of the test
5656
});
57-
58-
// Example for a group of failing tests
59-
describe('tests that require Zone.js', () => {
60-
beforeEach(() => {
61-
TestBed.configureTestingModule({
62-
providers: [provideZoneChangeDetection()]
63-
});
64-
});
65-
66-
// ... all failing tests go here
67-
});
6857
\`\`\`
6958
7059
### IMPORTANT: Rules and Constraints
@@ -74,6 +63,7 @@ export function createProvideZonelessForTestsSetupPrompt(testFilePath: string):
7463
2. **DO** add \`provideZoneChangeDetection()\` to the providers for each individual test or group of tests that fail after the change in Step 1.
7564
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.
7665
4. **DO NOT** make any other changes to the test file or any application code.
66+
5. **DO NOT** move tests around or create new describe blocks if it can be avoided. Changes are easier to review when code moves around less.
7767
7868
### Final Step
7969
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.`;
@@ -189,40 +179,16 @@ export function generateZonelessMigrationInstructionsForComponent(
189179
return createResponse(text);
190180
}
191181

192-
export function createTestDebuggingGuide(fileOrDirPath: string): MigrationResponse {
182+
export function createTestDebuggingGuideForNonActionableInput(
183+
fileOrDirPath: string,
184+
): MigrationResponse {
193185
const text = `You are an expert Angular developer assisting with a migration to zoneless.
194186
195187
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.
196188
197189
Your task is to investigate and fix any failing tests related to the code in \`${fileOrDirPath}\`.
198190
199-
### Test Debugging Guide
200-
201-
When running tests in a zoneless environment, you may encounter new types of failures. Here are common issues and how to resolve them:
202-
203-
1. **\`ExpressionChangedAfterItHasBeenCheckedError\`**:
204-
* **Cause**: This error indicates that a value in a component's template was updated, but Angular was not notified to run change detection.
205-
* **Solution**:
206-
* If the value is in a test-only wrapper component, update the property to be a signal.
207-
* For application components, either convert the property to a signal or call \`ChangeDetectorRef.markForCheck()\` immediately after the property is updated.
208-
209-
2. **Asynchronous Operations and Timing**:
210-
* **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\`).
211-
* **Solution**:
212-
* Avoid relying on synchronous change detection.
213-
* 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.
214-
215-
3. **Indirect Dependencies**:
216-
* **Cause**: The component itself might be zoneless-compatible, but it could be using a service or another dependency that is not.
217-
* **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.
218-
219-
### IMPORTANT: Rules and Constraints
220-
221-
You must follow these rules without exception:
222-
1. **DO** focus only on fixing the tests for the code in \`${fileOrDirPath}\`.
223-
2. **DO** apply the solutions described in the debugging guide to fix test failures.
224-
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\`).
225-
4. **DO NOT** re-introduce \`provideZoneChangeDetection()\` into tests that are already using \`provideZonelessChangeDetection()\`.
191+
${testDebuggingGuideText(fileOrDirPath)}
226192
`;
227193

228194
return createResponse(text);
@@ -255,7 +221,16 @@ export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): Migrat
255221
256222
The following usages of \`provideZoneChangeDetection\` must be removed:
257223
${locations.map((loc) => `- ${loc}`).join('\n')}
224+
${testDebuggingGuideText(sourceFile.fileName)}
225+
226+
### Final Step
227+
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.`;
228+
229+
return createResponse(text);
230+
}
258231

232+
function testDebuggingGuideText(fileName: string) {
233+
return `
259234
### Test Debugging Guide
260235
261236
After removing \`provideZoneChangeDetection\`, the tests will likely fail. Use this guide to diagnose and fix the failures.
@@ -279,15 +254,13 @@ export function createFixResponseForZoneTests(sourceFile: ts.SourceFile): Migrat
279254
### IMPORTANT: Rules and Constraints
280255
281256
You must follow these rules without exception:
282-
1. **DO** remove all usages of \`provideZoneChangeDetection\` from the test file.
283-
2. **DO** apply the solutions described in the debugging guide to fix any resulting test failures.
284-
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\`).
285-
4. **DO NOT** make any changes unrelated to fixing the failing tests in \`${sourceFile.fileName}\`.
286-
287-
### Final Step
288-
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.`;
289-
290-
return createResponse(text);
257+
1. **DO** focus only on fixing the tests for the code in \`${fileName}\`.
258+
2. **DO** remove all usages of \`provideZoneChangeDetection\` from the test file.
259+
3. **DO** apply the solutions described in the debugging guide to fix any resulting test failures.
260+
4. **DO** update properties of test components and directives to use signals. Tests often use plain objects and values and update the component state directly before calling \`fixture.detectChanges\`. This will not work and will result in \`ExpressionChangedAfterItHasBeenCheckedError\` because Angular was not notifed of the change.
261+
5. **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\`).
262+
6. **DO NOT** make any changes unrelated to fixing the failing tests in \`${fileName}\`.
263+
7. **DO NOT** re-introduce \`provideZoneChangeDetection()\` into tests that are already using \`provideZonelessChangeDetection()\`.`;
291264
}
292265

293266
/* eslint-enable max-len */

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/ts_utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import * as fs from 'fs';
9+
import * as fs from 'node:fs';
1010
import ts from 'typescript';
1111

1212
/**

packages/angular/cli/src/commands/mcp/tools/onpush-zoneless-migration/zoneless-migration.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@
88

99
import { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol';
1010
import { ServerNotification, ServerRequest } from '@modelcontextprotocol/sdk/types';
11-
import { glob } from 'fast-glob';
12-
import * as fs from 'fs';
11+
import { glob } from 'node:fs/promises';
12+
import * as fs from 'node:fs';
1313
import ts from 'typescript';
1414
import { z } from 'zod';
1515
import { declareTool } from '../tool-registry';
1616
import { analyzeForUnsupportedZoneUses } from './analyze_for_unsupported_zone_uses';
1717
import { migrateSingleFile } from './migrate_single_file';
1818
import { migrateTestFile } from './migrate_test_file';
19-
import { createTestDebuggingGuide } from './prompts';
19+
import { createTestDebuggingGuideForNonActionableInput } from './prompts';
2020
import { sendDebugMessage } from './send_debug_message';
2121
import { createSourceFile, getImportSpecifier } from './ts_utils';
2222

@@ -53,8 +53,10 @@ export async function registerZonelessMigrationTool(
5353
const zoneFiles = new Set<ts.SourceFile>();
5454

5555
if (fs.statSync(fileOrDirPath).isDirectory()) {
56-
const allFiles = await glob(`${fileOrDirPath}/**/*.ts`);
57-
files = allFiles.map(createSourceFile);
56+
const allFiles = glob(`${fileOrDirPath}/**/*.ts`);
57+
for await (const file of allFiles) {
58+
files.push(createSourceFile(file));
59+
}
5860
} else {
5961
files = [createSourceFile(fileOrDirPath)];
6062
const maybeTestFile = await getTestFilePath(fileOrDirPath);
@@ -67,7 +69,11 @@ export async function registerZonelessMigrationTool(
6769
const content = sourceFile.getFullText();
6870
const componentSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Component');
6971
const zoneSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
70-
const testBedSpecifier = getImportSpecifier(sourceFile, '@angular/core/testing', 'TestBed');
72+
const testBedSpecifier = getImportSpecifier(
73+
sourceFile,
74+
/(@angular\/core)?\/testing/,
75+
'TestBed',
76+
);
7177
if (testBedSpecifier) {
7278
componentTestFiles.add(sourceFile);
7379
} else if (componentSpecifier) {
@@ -122,7 +128,7 @@ export async function registerZonelessMigrationTool(
122128
}
123129
}
124130

125-
return createTestDebuggingGuide(fileOrDirPath);
131+
return createTestDebuggingGuideForNonActionableInput(fileOrDirPath);
126132
}
127133

128134
async function rankComponentFilesForMigration(

0 commit comments

Comments
 (0)