Skip to content

Commit 212ebde

Browse files
committed
refactor(@angular/cli): parallelize MCP zoneless migration file discovery and improve error handling
This commit enhances the `onpush_zoneless_migration` MCP tool by improving its file discovery and error handling, with a primary goal of increasing performance on large projects. The `discoverAndCategorizeFiles` function is refactored to parallelize both file I/O and analysis. It now gathers file paths and processes them concurrently in batches, preventing system resource exhaustion while speeding up execution. The tool now continues processing even if some files fail to be analyzed. Any errors are collected and reported in the final output, providing the user with a complete summary of unanalyzable files.
1 parent 5ef8b99 commit 212ebde

File tree

1 file changed

+81
-39
lines changed

1 file changed

+81
-39
lines changed

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

Lines changed: 81 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,10 @@ export async function registerZonelessMigrationTool(
7070
fileOrDirPath: string,
7171
extras: RequestHandlerExtra<ServerRequest, ServerNotification>,
7272
) {
73-
let filesWithComponents, componentTestFiles, zoneFiles;
73+
let filesWithComponents, componentTestFiles, zoneFiles, categorizationErrors;
7474
try {
75-
({ filesWithComponents, componentTestFiles, zoneFiles } = await discoverAndCategorizeFiles(
76-
fileOrDirPath,
77-
extras,
78-
));
75+
({ filesWithComponents, componentTestFiles, zoneFiles, categorizationErrors } =
76+
await discoverAndCategorizeFiles(fileOrDirPath, extras));
7977
} catch (e) {
8078
return createResponse(
8179
`Error: Could not access the specified path. Please ensure the following path is correct ` +
@@ -113,17 +111,26 @@ export async function registerZonelessMigrationTool(
113111
}
114112
}
115113

114+
if (categorizationErrors.length > 0) {
115+
let errorMessage =
116+
'Migration analysis is complete for all actionable files. However, the following files could not be analyzed due to errors:\n';
117+
errorMessage += categorizationErrors.map((e) => `- ${e.filePath}: ${e.message}`).join('\n');
118+
119+
return createResponse(errorMessage);
120+
}
121+
116122
return createTestDebuggingGuideForNonActionableInput(fileOrDirPath);
117123
}
118124

119125
async function discoverAndCategorizeFiles(
120126
fileOrDirPath: string,
121127
extras: RequestHandlerExtra<ServerRequest, ServerNotification>,
122128
) {
123-
let files: SourceFile[] = [];
129+
const filePaths: string[] = [];
124130
const componentTestFiles = new Set<SourceFile>();
125131
const filesWithComponents = new Set<SourceFile>();
126132
const zoneFiles = new Set<SourceFile>();
133+
const categorizationErrors: { filePath: string; message: string }[] = [];
127134

128135
let isDirectory: boolean;
129136
try {
@@ -134,52 +141,87 @@ async function discoverAndCategorizeFiles(
134141
}
135142

136143
if (isDirectory) {
137-
const allFiles = glob(`${fileOrDirPath}/**/*.ts`);
138-
for await (const file of allFiles) {
139-
files.push(await createSourceFile(file));
144+
for await (const file of glob(`${fileOrDirPath}/**/*.ts`)) {
145+
filePaths.push(file);
140146
}
141147
} else {
142-
files = [await createSourceFile(fileOrDirPath)];
148+
filePaths.push(fileOrDirPath);
143149
const maybeTestFile = await getTestFilePath(fileOrDirPath);
144150
if (maybeTestFile) {
145-
componentTestFiles.add(await createSourceFile(maybeTestFile));
151+
// Eagerly add the test file path for categorization.
152+
filePaths.push(maybeTestFile);
146153
}
147154
}
148155

149-
for (const sourceFile of files) {
150-
const content = sourceFile.getFullText();
151-
const componentSpecifier = await getImportSpecifier(sourceFile, '@angular/core', 'Component');
152-
const zoneSpecifier = await getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
153-
const testBedSpecifier = await getImportSpecifier(
154-
sourceFile,
155-
/(@angular\/core)?\/testing/,
156-
'TestBed',
156+
const CONCURRENCY_LIMIT = 50;
157+
const filesToProcess = [...filePaths];
158+
while (filesToProcess.length > 0) {
159+
const batch = filesToProcess.splice(0, CONCURRENCY_LIMIT);
160+
const results = await Promise.allSettled(
161+
batch.map(async (filePath) => {
162+
const sourceFile = await createSourceFile(filePath);
163+
await categorizeFile(sourceFile, extras, {
164+
filesWithComponents,
165+
componentTestFiles,
166+
zoneFiles,
167+
});
168+
}),
157169
);
158-
if (testBedSpecifier) {
159-
componentTestFiles.add(sourceFile);
160-
} else if (componentSpecifier) {
161-
if (
162-
!content.includes('changeDetectionStrategy: ChangeDetectionStrategy.OnPush') &&
163-
!content.includes('changeDetectionStrategy: ChangeDetectionStrategy.Default')
164-
) {
165-
filesWithComponents.add(sourceFile);
166-
} else {
167-
sendDebugMessage(
168-
`Component file already has change detection strategy: ${sourceFile.fileName}. Skipping migration.`,
169-
extras,
170-
);
171-
}
172170

173-
const testFilePath = await getTestFilePath(sourceFile.fileName);
174-
if (testFilePath) {
175-
componentTestFiles.add(await createSourceFile(testFilePath));
171+
for (let i = 0; i < results.length; i++) {
172+
const result = results[i];
173+
if (result.status === 'rejected') {
174+
const failedFile = batch[i];
175+
const reason = result.reason instanceof Error ? result.reason.message : `${result.reason}`;
176+
categorizationErrors.push({ filePath: failedFile, message: reason });
176177
}
177-
} else if (zoneSpecifier) {
178-
zoneFiles.add(sourceFile);
179178
}
180179
}
181180

182-
return { filesWithComponents, componentTestFiles, zoneFiles };
181+
return { filesWithComponents, componentTestFiles, zoneFiles, categorizationErrors };
182+
}
183+
184+
async function categorizeFile(
185+
sourceFile: SourceFile,
186+
extras: RequestHandlerExtra<ServerRequest, ServerNotification>,
187+
categorizedFiles: {
188+
filesWithComponents: Set<SourceFile>;
189+
componentTestFiles: Set<SourceFile>;
190+
zoneFiles: Set<SourceFile>;
191+
},
192+
) {
193+
const { filesWithComponents, componentTestFiles, zoneFiles } = categorizedFiles;
194+
const content = sourceFile.getFullText();
195+
const componentSpecifier = await getImportSpecifier(sourceFile, '@angular/core', 'Component');
196+
const zoneSpecifier = await getImportSpecifier(sourceFile, '@angular/core', 'NgZone');
197+
const testBedSpecifier = await getImportSpecifier(
198+
sourceFile,
199+
/(@angular\/core)?\/testing/,
200+
'TestBed',
201+
);
202+
203+
if (testBedSpecifier) {
204+
componentTestFiles.add(sourceFile);
205+
} else if (componentSpecifier) {
206+
if (
207+
!content.includes('changeDetectionStrategy: ChangeDetectionStrategy.OnPush') &&
208+
!content.includes('changeDetectionStrategy: ChangeDetectionStrategy.Default')
209+
) {
210+
filesWithComponents.add(sourceFile);
211+
} else {
212+
sendDebugMessage(
213+
`Component file already has change detection strategy: ${sourceFile.fileName}. Skipping migration.`,
214+
extras,
215+
);
216+
}
217+
218+
const testFilePath = await getTestFilePath(sourceFile.fileName);
219+
if (testFilePath) {
220+
componentTestFiles.add(await createSourceFile(testFilePath));
221+
}
222+
} else if (zoneSpecifier) {
223+
zoneFiles.add(sourceFile);
224+
}
183225
}
184226

185227
async function rankComponentFilesForMigration(

0 commit comments

Comments
 (0)