Skip to content

Commit 288156b

Browse files
crisbetoatscott
crisbeto
authored and
atscott
committed
Revert "fix(migrations): preserve trailing commas in code generated by standalone migration (#49533)" (#49547)
This reverts commit bf4d856. PR Close #49547
1 parent bf4d856 commit 288156b

File tree

4 files changed

+11
-208
lines changed

4 files changed

+11
-208
lines changed

‎packages/core/schematics/ng-generate/standalone-migration/prune-modules.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,7 @@ function removeArrayReferences(
152152
tracker: ChangeTracker): void {
153153
for (const [array, toRemove] of locations.getEntries()) {
154154
const newElements = filterRemovedElements(array.elements, toRemove);
155-
tracker.replaceNode(
156-
array,
157-
ts.factory.updateArrayLiteralExpression(
158-
array, ts.factory.createNodeArray(newElements, array.elements.hasTrailingComma)));
155+
tracker.replaceNode(array, ts.factory.updateArrayLiteralExpression(array, newElements));
159156
}
160157
}
161158

‎packages/core/schematics/ng-generate/standalone-migration/standalone-bootstrap.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,9 @@ function replaceBootstrapCallExpression(
244244

245245
// Push the providers after `importProvidersFrom` call for better readability.
246246
combinedProviders.push(...providers);
247-
248-
const providersArray = ts.factory.createNodeArray(
249-
combinedProviders,
250-
analysis.metadata.properties.hasTrailingComma && combinedProviders.length > 2);
251247
const initializer = remapDynamicImports(
252248
sourceFile.fileName,
253-
ts.factory.createArrayLiteralExpression(providersArray, combinedProviders.length > 1));
249+
ts.factory.createArrayLiteralExpression(combinedProviders, combinedProviders.length > 1));
254250

255251
args.push(ts.factory.createObjectLiteralExpression(
256252
[ts.factory.createPropertyAssignment('providers', initializer)], true));

‎packages/core/schematics/ng-generate/standalone-migration/to-standalone.ts‎

Lines changed: 9 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,10 @@ export function convertNgModuleDeclarationToStandalone(
9898
decl, allDeclarations, tracker, typeChecker, importRemapper);
9999

100100
if (importsToAdd.length > 0) {
101-
const hasTrailingComma = importsToAdd.length > 2 &&
102-
!!extractMetadataLiteral(directiveMeta.decorator)?.properties.hasTrailingComma;
103101
decorator = addPropertyToAngularDecorator(
104102
decorator,
105103
ts.factory.createPropertyAssignment(
106-
'imports',
107-
ts.factory.createArrayLiteralExpression(
108-
// Create a multi-line array when it has a trailing comma.
109-
ts.factory.createNodeArray(importsToAdd, hasTrailingComma), hasTrailingComma)));
104+
'imports', ts.factory.createArrayLiteralExpression(importsToAdd)));
110105
}
111106
}
112107

@@ -220,9 +215,6 @@ function moveDeclarationsToImports(
220215
const declarationsToCopy: ts.Expression[] = [];
221216
const properties: ts.ObjectLiteralElementLike[] = [];
222217
const importsProp = findLiteralProperty(literal, 'imports');
223-
const hasAnyArrayTrailingComma = literal.properties.some(
224-
prop => ts.isPropertyAssignment(prop) && ts.isArrayLiteralExpression(prop.initializer) &&
225-
prop.initializer.elements.hasTrailingComma);
226218

227219
// Separate the declarations that we want to keep and ones we need to copy into the `imports`.
228220
if (ts.isPropertyAssignment(declarationsProp)) {
@@ -256,9 +248,7 @@ function moveDeclarationsToImports(
256248
// If there are no `imports`, create them with the declarations we want to copy.
257249
if (!importsProp && declarationsToCopy.length > 0) {
258250
properties.push(ts.factory.createPropertyAssignment(
259-
'imports',
260-
ts.factory.createArrayLiteralExpression(ts.factory.createNodeArray(
261-
declarationsToCopy, hasAnyArrayTrailingComma && declarationsToCopy.length > 2))));
251+
'imports', ts.factory.createArrayLiteralExpression(declarationsToCopy)));
262252
}
263253

264254
for (const prop of literal.properties) {
@@ -270,13 +260,8 @@ function moveDeclarationsToImports(
270260
// If we have declarations to preserve, update the existing property, otherwise drop it.
271261
if (prop === declarationsProp) {
272262
if (declarationsToPreserve.length > 0) {
273-
const hasTrailingComma = ts.isArrayLiteralExpression(prop.initializer) ?
274-
prop.initializer.elements.hasTrailingComma :
275-
hasAnyArrayTrailingComma;
276263
properties.push(ts.factory.updatePropertyAssignment(
277-
prop, prop.name,
278-
ts.factory.createArrayLiteralExpression(ts.factory.createNodeArray(
279-
declarationsToPreserve, hasTrailingComma && declarationsToPreserve.length > 2))));
264+
prop, prop.name, ts.factory.createArrayLiteralExpression(declarationsToPreserve)));
280265
}
281266
continue;
282267
}
@@ -288,16 +273,10 @@ function moveDeclarationsToImports(
288273

289274
if (ts.isArrayLiteralExpression(prop.initializer)) {
290275
initializer = ts.factory.updateArrayLiteralExpression(
291-
prop.initializer,
292-
ts.factory.createNodeArray(
293-
[...prop.initializer.elements, ...declarationsToCopy],
294-
prop.initializer.elements.hasTrailingComma));
276+
prop.initializer, [...prop.initializer.elements, ...declarationsToCopy]);
295277
} else {
296-
initializer = ts.factory.createArrayLiteralExpression(ts.factory.createNodeArray(
297-
[ts.factory.createSpreadElement(prop.initializer), ...declarationsToCopy],
298-
// Expect the declarations to be greater than 1 since
299-
// we have the pre-existing initializer already.
300-
hasAnyArrayTrailingComma && declarationsToCopy.length > 1));
278+
initializer = ts.factory.createArrayLiteralExpression(
279+
[ts.factory.createSpreadElement(prop.initializer), ...declarationsToCopy]);
301280
}
302281

303282
properties.push(ts.factory.updatePropertyAssignment(prop, prop.name, initializer));
@@ -309,9 +288,7 @@ function moveDeclarationsToImports(
309288
}
310289

311290
tracker.replaceNode(
312-
literal,
313-
ts.factory.updateObjectLiteralExpression(
314-
literal, ts.factory.createNodeArray(properties, literal.properties.hasTrailingComma)),
291+
literal, ts.factory.updateObjectLiteralExpression(literal, properties),
315292
ts.EmitHint.Expression);
316293
}
317294

@@ -336,12 +313,10 @@ function addPropertyToAngularDecorator(
336313
}
337314

338315
let literalProperties: ts.ObjectLiteralElementLike[];
339-
let hasTrailingComma = false;
340316

341317
if (node.expression.arguments.length === 0) {
342318
literalProperties = [property];
343319
} else if (ts.isObjectLiteralExpression(node.expression.arguments[0])) {
344-
hasTrailingComma = node.expression.arguments[0].properties.hasTrailingComma;
345320
literalProperties = [...node.expression.arguments[0].properties, property];
346321
} else {
347322
// Unsupported case (e.g. `@Component(SOME_CONST)`). Return the original node.
@@ -352,9 +327,7 @@ function addPropertyToAngularDecorator(
352327
// the latter ends up duplicating the node's leading comment.
353328
return ts.factory.createDecorator(ts.factory.createCallExpression(
354329
node.expression.expression, node.expression.typeArguments,
355-
[ts.factory.createObjectLiteralExpression(
356-
ts.factory.createNodeArray(literalProperties, hasTrailingComma),
357-
literalProperties.length > 1)]));
330+
[ts.factory.createObjectLiteralExpression(literalProperties, literalProperties.length > 1)]));
358331
}
359332

360333
/** Checks if a node is a `PropertyAssignment` with a name. */
@@ -586,16 +559,12 @@ export function migrateTestDeclarations(
586559
}
587560

588561
if (importsToAdd && importsToAdd.size > 0) {
589-
const hasTrailingComma = importsToAdd.size > 2 &&
590-
!!extractMetadataLiteral(decorator.node)?.properties.hasTrailingComma;
591-
const importsArray = ts.factory.createNodeArray(Array.from(importsToAdd), hasTrailingComma);
592-
593562
tracker.replaceNode(
594563
decorator.node,
595564
addPropertyToAngularDecorator(
596565
newDecorator,
597566
ts.factory.createPropertyAssignment(
598-
'imports', ts.factory.createArrayLiteralExpression(importsArray))));
567+
'imports', ts.factory.createArrayLiteralExpression(Array.from(importsToAdd)))));
599568
} else {
600569
tracker.replaceNode(decorator.node, newDecorator);
601570
}

‎packages/core/schematics/test/standalone_migration_spec.ts‎

Lines changed: 0 additions & 159 deletions
Original file line numberDiff line numberDiff line change
@@ -1657,128 +1657,6 @@ describe('standalone migration', () => {
16571657
`));
16581658
});
16591659

1660-
it('should preserve the trailing comma when adding an `imports` array', async () => {
1661-
writeFile('module.ts', `
1662-
import {NgModule, Directive} from '@angular/core';
1663-
1664-
@Directive({selector: '[dir]'})
1665-
export class MyDir {}
1666-
1667-
@NgModule({
1668-
declarations: [MyDir],
1669-
exports: [MyDir],
1670-
})
1671-
export class Mod {}
1672-
`);
1673-
1674-
await runMigration('convert-to-standalone');
1675-
1676-
expect(stripWhitespace(tree.readContent('module.ts'))).toContain(stripWhitespace(`
1677-
@NgModule({
1678-
imports: [MyDir],
1679-
exports: [MyDir],
1680-
})
1681-
`));
1682-
});
1683-
1684-
it('should preserve the trailing comma when adding to an existing `imports` array', async () => {
1685-
writeFile('module.ts', `
1686-
import {NgModule, Directive} from '@angular/core';
1687-
import {CommonModule} from '@angular/common';
1688-
import {RouterModule} from '@angular/router';
1689-
1690-
@Directive({selector: '[dir]'})
1691-
export class MyDir {}
1692-
1693-
@NgModule({
1694-
imports: [
1695-
CommonModule,
1696-
RouterModule,
1697-
],
1698-
declarations: [MyDir],
1699-
exports: [MyDir],
1700-
})
1701-
export class Mod {}
1702-
`);
1703-
1704-
await runMigration('convert-to-standalone');
1705-
1706-
expect(stripWhitespace(tree.readContent('module.ts'))).toContain(stripWhitespace(`
1707-
@NgModule({
1708-
imports: [
1709-
CommonModule,
1710-
RouterModule,
1711-
MyDir,
1712-
],
1713-
exports: [MyDir],
1714-
})
1715-
`));
1716-
});
1717-
1718-
it('should preserve the trailing comma when marking a directive as standalone', async () => {
1719-
writeFile('module.ts', `
1720-
import {NgModule, Directive} from '@angular/core';
1721-
1722-
@Directive({
1723-
selector: '[dir]',
1724-
exportAs: 'dir',
1725-
})
1726-
export class MyDir {}
1727-
1728-
@NgModule({declarations: [MyDir]})
1729-
export class Mod {}
1730-
`);
1731-
1732-
await runMigration('convert-to-standalone');
1733-
1734-
expect(stripWhitespace(tree.readContent('module.ts'))).toContain(stripWhitespace(`
1735-
@Directive({
1736-
selector: '[dir]',
1737-
exportAs: 'dir',
1738-
standalone: true,
1739-
})
1740-
`));
1741-
});
1742-
1743-
it('should add a trailing comma when generating an imports array in a component', async () => {
1744-
writeFile('module.ts', `
1745-
import {NgModule, Directive, Component} from '@angular/core';
1746-
1747-
@Directive({selector: '[dir-one]'})
1748-
export class DirOne {}
1749-
1750-
@Directive({selector: '[dir-two]'})
1751-
export class DirTwo {}
1752-
1753-
@Directive({selector: '[dir-three]'})
1754-
export class DirThree {}
1755-
1756-
@Component({
1757-
selector: 'my-comp',
1758-
template: '<div dir-one dir-two dir-three></div>',
1759-
})
1760-
export class MyComp {}
1761-
1762-
@NgModule({declarations: [DirOne, DirTwo, DirThree, MyComp]})
1763-
export class Mod {}
1764-
`);
1765-
1766-
await runMigration('convert-to-standalone');
1767-
1768-
expect(stripWhitespace(tree.readContent('module.ts'))).toContain(stripWhitespace(`
1769-
@Component({
1770-
selector: 'my-comp',
1771-
template: '<div dir-one dir-two dir-three></div>',
1772-
standalone: true,
1773-
imports: [
1774-
DirOne,
1775-
DirTwo,
1776-
DirThree,
1777-
],
1778-
})
1779-
`));
1780-
});
1781-
17821660
it('should remove a module that only has imports and exports', async () => {
17831661
writeFile('app.module.ts', `
17841662
import {NgModule} from '@angular/core';
@@ -2535,43 +2413,6 @@ describe('standalone migration', () => {
25352413
`));
25362414
});
25372415

2538-
it('should preserve the trailing comma when deleting a module', async () => {
2539-
const initialAppModule = `
2540-
import {NgModule, InjectionToken} from '@angular/core';
2541-
import {CommonModule} from '@angular/common';
2542-
import {RouterModule} from '@angular/router';
2543-
2544-
const token = new InjectionToken('token');
2545-
2546-
@NgModule()
2547-
export class ToDelete {}
2548-
2549-
@NgModule({
2550-
imports: [
2551-
CommonModule,
2552-
ToDelete,
2553-
RouterModule,
2554-
],
2555-
providers: [{provide: token, useValue: 123}],
2556-
})
2557-
export class AppModule {}
2558-
`;
2559-
2560-
writeFile('app.module.ts', initialAppModule);
2561-
2562-
await runMigration('prune-ng-modules');
2563-
2564-
expect(stripWhitespace(tree.readContent('app.module.ts'))).toContain(stripWhitespace(`
2565-
@NgModule({
2566-
imports: [
2567-
CommonModule,
2568-
RouterModule,
2569-
],
2570-
providers: [{provide: token, useValue: 123}],
2571-
})
2572-
`));
2573-
});
2574-
25752416
it('should switch a platformBrowser().bootstrapModule call to bootstrapApplication', async () => {
25762417
writeFile('main.ts', `
25772418
import {AppModule} from './app/app.module';

0 commit comments

Comments
 (0)