Skip to content

Commit 3d48a72

Browse files
alxhubalxhub
committed
Revert "refactor(migrations): properly handle multi query migration (#57525)" (#57555)
This reverts commit f454ad3. Reason: breaks g3 PR Close #57555
1 parent 5d2e243 commit 3d48a72

File tree

9 files changed

+67
-379
lines changed

9 files changed

+67
-379
lines changed

‎packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@
88

99
import ts from 'typescript';
1010
import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator';
11-
import {applyImportManagerChanges} from '../../../../utils/tsurge/helpers/apply_import_manager';
1211
import {MigrationResult} from '../result';
12+
import {
13+
absoluteFrom,
14+
absoluteFromSourceFile,
15+
} from '../../../../../../compiler-cli/src/ngtsc/file_system';
16+
import {Replacement, TextUpdate} from '../../../../utils/tsurge/replacement';
1317

1418
/**
1519
* Phase that applies all changes recorded by the import manager in
@@ -20,5 +24,57 @@ export function pass10_applyImportManager(
2024
result: MigrationResult,
2125
sourceFiles: readonly ts.SourceFile[],
2226
) {
23-
applyImportManagerChanges(importManager, result.replacements, sourceFiles);
27+
const {newImports, updatedImports, deletedImports} = importManager.finalize();
28+
const printer = ts.createPrinter({});
29+
const pathToFile = new Map<string, ts.SourceFile>(sourceFiles.map((s) => [s.fileName, s]));
30+
31+
// Capture new imports
32+
newImports.forEach((newImports, fileName) => {
33+
newImports.forEach((newImport) => {
34+
const printedImport = printer.printNode(
35+
ts.EmitHint.Unspecified,
36+
newImport,
37+
pathToFile.get(fileName)!,
38+
);
39+
result.replacements.push(
40+
new Replacement(
41+
absoluteFrom(fileName),
42+
new TextUpdate({position: 0, end: 0, toInsert: `${printedImport}\n`}),
43+
),
44+
);
45+
});
46+
});
47+
48+
// Capture updated imports
49+
for (const [oldBindings, newBindings] of updatedImports.entries()) {
50+
const printedBindings = printer.printNode(
51+
ts.EmitHint.Unspecified,
52+
newBindings,
53+
oldBindings.getSourceFile(),
54+
);
55+
result.replacements.push(
56+
new Replacement(
57+
absoluteFromSourceFile(oldBindings.getSourceFile()),
58+
new TextUpdate({
59+
position: oldBindings.getStart(),
60+
end: oldBindings.getEnd(),
61+
toInsert: printedBindings,
62+
}),
63+
),
64+
);
65+
}
66+
67+
// Update removed imports
68+
for (const removedImport of deletedImports) {
69+
result.replacements.push(
70+
new Replacement(
71+
absoluteFromSourceFile(removedImport.getSourceFile()),
72+
new TextUpdate({
73+
position: removedImport.getStart(),
74+
end: removedImport.getEnd(),
75+
toInsert: '',
76+
}),
77+
),
78+
);
79+
}
2480
}

‎packages/core/schematics/migrations/signal-queries-migration/convert_query_property.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {ImportManager} from '../../../../compiler-cli/private/migrations';
1414
import assert from 'assert';
1515
import {WrappedNodeExpr} from '@angular/compiler';
1616
import {removeFromUnionIfPossible} from '../signal-migration/src/utils/remove_from_union';
17-
import {extractQueryListType} from './query_list_type';
1817

1918
const printer = ts.createPrinter();
2019

@@ -59,16 +58,6 @@ export function computeReplacementsToMigrateQuery(
5958
];
6059
let type = node.type;
6160

62-
// For multi queries, attempt to unwrap `QueryList` types, or infer the
63-
// type from the initializer, if possible.
64-
if (!metadata.queryInfo.first) {
65-
if (type === undefined && node.initializer !== undefined) {
66-
type = extractQueryListType(node.initializer);
67-
} else if (type !== undefined) {
68-
type = extractQueryListType(type);
69-
}
70-
}
71-
7261
if (metadata.queryInfo.read !== null) {
7362
assert(metadata.queryInfo.read instanceof WrappedNodeExpr);
7463
optionProperties.push(

‎packages/core/schematics/migrations/signal-queries-migration/migration.spec.ts‎

Lines changed: 7 additions & 186 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ interface TestCase {
1919
focus?: boolean;
2020
}
2121

22-
const declarationTestCases: TestCase[] = [
23-
// View Child
22+
const testCases: TestCase[] = [
2423
{
2524
id: 'viewChild with string locator and nullable',
2625
before: `@ViewChild('myBtn') button: MyButton|undefined = undefined;`,
@@ -51,154 +50,26 @@ const declarationTestCases: TestCase[] = [
5150
before: `@ViewChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`,
5251
after: `readonly buttonEl = viewChild.required('myBtn', { read: ElementRef });`,
5352
},
54-
// Content Child
55-
{
56-
id: 'contentChild with string locator and nullable',
57-
before: `@ContentChild('myBtn') button: MyButton|undefined = undefined;`,
58-
after: `readonly button = contentChild<MyButton>('myBtn');`,
59-
},
60-
{
61-
id: 'contentChild with class type locator and nullable',
62-
before: `@ContentChild(MyButton) button: MyButton|undefined = undefined;`,
63-
after: `readonly button = contentChild(MyButton);`,
64-
},
65-
{
66-
id: 'contentChild with class type locator and nullable via question-mark shorthand',
67-
before: `@ContentChild(MyButton) button?: MyButton;`,
68-
after: `readonly button = contentChild(MyButton);`,
69-
},
70-
{
71-
id: 'contentChild with class type locator and exclamation mark to simulate required',
72-
before: `@ContentChild(MyButton) button!: MyButton;`,
73-
after: `readonly button = contentChild.required(MyButton);`,
74-
},
75-
{
76-
id: 'contentChild with string locator and read option, nullable shorthand',
77-
before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl?: ElementRef;`,
78-
after: `readonly buttonEl = contentChild('myBtn', { read: ElementRef });`,
79-
},
80-
{
81-
id: 'contentChild with string locator and read option, required',
82-
before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`,
83-
after: `readonly buttonEl = contentChild.required('myBtn', { read: ElementRef });`,
84-
},
85-
{
86-
id: 'contentChild with string locator and read option, required',
87-
before: `@ContentChild('myBtn', {read: ElementRef}) buttonEl!: ElementRef;`,
88-
after: `readonly buttonEl = contentChild.required('myBtn', { read: ElementRef });`,
89-
},
90-
{
91-
id: 'contentChild with descendants option',
92-
before: `@ContentChild('myBtn', {descendants: false}) buttonEl!: ElementRef;`,
93-
after: `readonly buttonEl = contentChild.required<ElementRef>('myBtn', { descendants: false });`,
94-
},
95-
// ViewChildren
96-
{
97-
id: 'viewChildren with string locator and nullable',
98-
before: `@ViewChildren('myBtn') button?: QueryList<ElementRef>;`,
99-
after: `readonly button = viewChildren<ElementRef>('myBtn');`,
100-
},
101-
{
102-
id: 'viewChildren with class type locator and nullable',
103-
before: `@ViewChildren(MyButton) button?: QueryList<MyButton>;`,
104-
after: `readonly button = viewChildren(MyButton);`,
105-
},
106-
{
107-
id: 'viewChildren with class type locator and exclamation mark',
108-
before: `@ViewChildren(MyButton) button!: QueryList<MyButton>;`,
109-
after: `readonly button = viewChildren(MyButton);`,
110-
},
111-
{
112-
id: 'viewChild with string locator and read option, nullable shorthand',
113-
before: `@ViewChildren('myBtn', {read: ElementRef}) buttonEl?: QueryList<ElementRef>;`,
114-
after: `readonly buttonEl = viewChildren('myBtn', { read: ElementRef });`,
115-
},
116-
{
117-
id: 'viewChildren with string locator and read option, required',
118-
before: `@ViewChildren('myBtn', {read: ElementRef}) buttonEl!: QueryList<ElementRef>;`,
119-
after: `readonly buttonEl = viewChildren('myBtn', { read: ElementRef });`,
120-
},
121-
{
122-
id: 'viewChildren with query list as initializer value',
123-
before: `@ViewChildren('myBtn') buttonEl = new QueryList<ElementRef>()`,
124-
after: `readonly buttonEl = viewChildren<ElementRef>('myBtn');`,
125-
},
126-
{
127-
id: 'viewChildren with query list as initializer value, and descendants option',
128-
before: `@ViewChildren('myBtn', {descendants: false}) buttonEl = new QueryList<ElementRef>()`,
129-
after: `readonly buttonEl = viewChildren<ElementRef>('myBtn', { descendants: false });`,
130-
},
131-
{
132-
id: 'viewChildren with query list as initializer value, and descendants option but same as default',
133-
before: `@ViewChildren('myBtn', {descendants: true}) buttonEl = new QueryList<ElementRef>()`,
134-
after: `readonly buttonEl = viewChildren<ElementRef>('myBtn');`,
135-
},
136-
// ContentChildren
137-
{
138-
id: 'contentChildren with string locator and nullable',
139-
before: `@ContentChildren('myBtn') button?: QueryList<ElementRef>;`,
140-
after: `readonly button = contentChildren<ElementRef>('myBtn');`,
141-
},
142-
{
143-
id: 'contentChildren with class type locator and nullable',
144-
before: `@ContentChildren(MyButton) button?: QueryList<MyButton>;`,
145-
after: `readonly button = contentChildren(MyButton);`,
146-
},
147-
{
148-
id: 'contentChildren with class type locator and exclamation mark',
149-
before: `@ContentChildren(MyButton) button!: QueryList<MyButton>;`,
150-
after: `readonly button = contentChildren(MyButton);`,
151-
},
152-
{
153-
id: 'contentChildren with string locator and read option, nullable shorthand',
154-
before: `@ContentChildren('myBtn', {read: ElementRef}) buttonEl?: QueryList<ElementRef>;`,
155-
after: `readonly buttonEl = contentChildren('myBtn', { read: ElementRef });`,
156-
},
157-
{
158-
id: 'contentChildren with string locator and read option, required',
159-
before: `@ContentChildren('myBtn', {read: ElementRef}) buttonEl!: QueryList<ElementRef>;`,
160-
after: `readonly buttonEl = contentChildren('myBtn', { read: ElementRef });`,
161-
},
162-
{
163-
id: 'contentChildren with query list as initializer value',
164-
before: `@ContentChildren('myBtn') buttonEl = new QueryList<ElementRef>()`,
165-
after: `readonly buttonEl = contentChildren<ElementRef>('myBtn');`,
166-
},
167-
{
168-
id: 'contentChildren with query list as initializer value, and descendants option',
169-
before: `@ContentChildren('myBtn', {descendants: true}) buttonEl = new QueryList<ElementRef>()`,
170-
after: `readonly buttonEl = contentChildren<ElementRef>('myBtn', { descendants: true });`,
171-
},
172-
{
173-
id: 'contentChildren with query list as initializer value, and descendants option but same as default',
174-
before: `@ContentChildren('myBtn', {descendants: false}) buttonEl = new QueryList<ElementRef>()`,
175-
after: `readonly buttonEl = contentChildren<ElementRef>('myBtn');`,
176-
},
17753
];
17854

17955
describe('signal queries migration', () => {
18056
beforeEach(() => {
18157
initMockFileSystem('Native');
18258
});
18359

184-
describe('declaration test cases', () => {
185-
for (const c of declarationTestCases) {
60+
describe('test cases', () => {
61+
for (const c of testCases) {
18662
(c.focus ? fit : it)(c.id, async () => {
18763
const fs = await runTsurgeMigration(new SignalQueriesMigration(), [
18864
{
18965
name: absoluteFrom('/app.component.ts'),
19066
isProgramRootFile: true,
191-
contents: populateDeclarationTestCaseComponent(c.before),
67+
contents: populateTestCaseComponent(c.before),
19268
},
19369
]);
19470

195-
let actual = fs.readFile(absoluteFrom('/app.component.ts'));
196-
let expected = populateDeclarationTestCaseComponent(c.after);
197-
198-
// Cut off the string before the class declaration.
199-
// The import diff is irrelevant here for now.
200-
actual = actual.substring(actual.indexOf('@Directive'));
201-
expected = expected.substring(expected.indexOf('@Directive'));
71+
const actual = fs.readFile(absoluteFrom('/app.component.ts'));
72+
const expected = populateTestCaseComponent(c.after);
20273

20374
if (actual !== expected) {
20475
expect(diffText(expected, actual)).toBe('');
@@ -231,59 +102,9 @@ describe('signal queries migration', () => {
231102
expect(actual).not.toContain(`viewChild`);
232103
expect(actual).toContain(`@ViewChild('labelRef') label?: ElementRef;`);
233104
});
234-
235-
it('should update imports when migrating', async () => {
236-
const fs = await runTsurgeMigration(new SignalQueriesMigration(), [
237-
{
238-
name: absoluteFrom('/app.component.ts'),
239-
isProgramRootFile: true,
240-
contents: `
241-
import {ViewChild, ElementRef, Directive} from '@angular/core';
242-
243-
@Directive()
244-
class MyComp {
245-
@ViewChild('labelRef') label?: ElementRef;
246-
}
247-
`,
248-
},
249-
]);
250-
251-
const actual = fs.readFile(absoluteFrom('/app.component.ts'));
252-
expect(actual).toContain(`import { ElementRef, Directive, viewChild } from '@angular/core';`);
253-
expect(actual).toContain(`label = viewChild<ElementRef>('labelRef')`);
254-
});
255-
256-
it('should not remove imports when partially migrating', async () => {
257-
const fs = await runTsurgeMigration(new SignalQueriesMigration(), [
258-
{
259-
name: absoluteFrom('/app.component.ts'),
260-
isProgramRootFile: true,
261-
contents: `
262-
import {ViewChild, ElementRef, Directive} from '@angular/core';
263-
264-
@Directive()
265-
class MyComp {
266-
@ViewChild('labelRef') label?: ElementRef;
267-
@ViewChild('labelRef2') label2?: ElementRef;
268-
269-
click() {
270-
this.label2 = undefined;
271-
}
272-
}
273-
`,
274-
},
275-
]);
276-
277-
const actual = fs.readFile(absoluteFrom('/app.component.ts'));
278-
expect(actual).toContain(
279-
`import { ViewChild, ElementRef, Directive, viewChild } from '@angular/core';`,
280-
);
281-
expect(actual).toContain(`label = viewChild<ElementRef>('labelRef')`);
282-
expect(actual).toContain(`@ViewChild('labelRef2') label2?: ElementRef;`);
283-
});
284105
});
285106

286-
function populateDeclarationTestCaseComponent(declaration: string): string {
107+
function populateTestCaseComponent(declaration: string): string {
287108
return `
288109
import {
289110
ViewChild,

0 commit comments

Comments
 (0)