Skip to content

Commit 9457713

Browse files
ZainnQureshiidgp1130
authored andcommitted
fix(@angular/cli): skip components with a change detection strategy in the zoneless migration tool
The pre-filter in the onpush_zoneless_migration MCP tool looked for a `changeDetectionStrategy:` property, but the @component metadata key is `changeDetection`, so it never matched. Every component was queued for migration, and with more than one the tool sent a sampling request to rank files that migrateSingleFile then skipped anyway. Components that are skipped but import NgZone are still analyzed for unsupported zone usages, so the tool keeps reporting those blockers.
1 parent 47745c0 commit 9457713

2 files changed

Lines changed: 122 additions & 3 deletions

File tree

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,13 @@ async function categorizeFile(
194194
if (testBedSpecifier) {
195195
componentTestFiles.add(sourceFile);
196196
} else if (componentSpecifier) {
197-
if (
198-
!/changeDetectionStrategy:\s*ChangeDetectionStrategy\.(?:OnPush|Default|Eager)/.test(content)
199-
) {
197+
if (!/changeDetection\s*:\s*ChangeDetectionStrategy\.(?:OnPush|Default|Eager)/.test(content)) {
200198
filesWithComponents.add(sourceFile);
201199
} else {
200+
// The component is already migrated, but it can still use NgZone APIs that block zoneless.
201+
if (zoneSpecifier) {
202+
zoneFiles.add(sourceFile);
203+
}
202204
sendDebugMessage(
203205
`Component file already has change detection strategy: ${sourceFile.fileName}. Skipping migration.`,
204206
extras,
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
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 type { ServerContext } from '@modelcontextprotocol/server';
10+
import { join } from 'node:path';
11+
import { MockHost } from '../../testing/mock-host';
12+
import { registerZonelessMigrationTool } from './zoneless-migration';
13+
14+
function createComponent(name: string, metadata = ''): string {
15+
return `
16+
import { ChangeDetectionStrategy, Component } from '@angular/core';
17+
18+
@Component({
19+
selector: 'app-${name}',
20+
template: '',${metadata}
21+
})
22+
export class ${name}Component {}
23+
`;
24+
}
25+
26+
describe('registerZonelessMigrationTool', () => {
27+
const dir = join('/', 'project', 'src');
28+
let host: MockHost;
29+
let extras: ServerContext;
30+
let send: jasmine.Spy;
31+
32+
function setFiles(files: Record<string, string>): void {
33+
host.stat.and.resolveTo({ isDirectory: () => true });
34+
host.existsSync.and.returnValue(false);
35+
host.glob.and.callFake(() =>
36+
(async function* () {
37+
for (const name of Object.keys(files)) {
38+
yield { parentPath: dir, name };
39+
}
40+
})(),
41+
);
42+
const contents = new Map(Object.entries(files).map(([name, text]) => [join(dir, name), text]));
43+
host.readFile.and.callFake(async (path: string) => {
44+
const text = contents.get(path);
45+
if (text === undefined) {
46+
throw new Error(`Unexpected read: ${path}`);
47+
}
48+
49+
return text;
50+
});
51+
}
52+
53+
beforeEach(() => {
54+
host = new MockHost();
55+
send = jasmine.createSpy('send').and.rejectWith(new Error('sampling not supported'));
56+
extras = {
57+
mcpReq: {
58+
log: jasmine.createSpy(),
59+
notify: jasmine.createSpy(),
60+
send,
61+
},
62+
} as unknown as ServerContext;
63+
});
64+
65+
it('should skip components that already set a change detection strategy', async () => {
66+
setFiles({
67+
'a.component.ts': createComponent(
68+
'A',
69+
'\n changeDetection: ChangeDetectionStrategy.OnPush,',
70+
),
71+
'b.component.ts': createComponent(
72+
'B',
73+
'\n changeDetection: ChangeDetectionStrategy.Default,',
74+
),
75+
});
76+
77+
await registerZonelessMigrationTool(dir, host, extras);
78+
79+
expect(send).not.toHaveBeenCalled();
80+
});
81+
82+
it('should rank components that do not set a change detection strategy', async () => {
83+
setFiles({
84+
'a.component.ts': createComponent('A'),
85+
'b.component.ts': createComponent('B'),
86+
});
87+
88+
await registerZonelessMigrationTool(dir, host, extras);
89+
90+
expect(send).toHaveBeenCalledTimes(1);
91+
});
92+
93+
it('should still report NgZone usages in components that already set a strategy', async () => {
94+
setFiles({
95+
'a.component.ts': `
96+
import { ChangeDetectionStrategy, Component, NgZone } from '@angular/core';
97+
98+
@Component({
99+
selector: 'app-a',
100+
template: '',
101+
changeDetection: ChangeDetectionStrategy.OnPush,
102+
})
103+
export class AComponent {
104+
constructor(private zone: NgZone) {
105+
this.zone.onMicrotaskEmpty(() => {});
106+
}
107+
}
108+
`,
109+
});
110+
111+
const result = await registerZonelessMigrationTool(dir, host, extras);
112+
113+
expect(result.content[0].text).toContain(
114+
'The component uses NgZone APIs that are incompatible with zoneless applications',
115+
);
116+
});
117+
});

0 commit comments

Comments
 (0)