Skip to content

Commit 7dd1957

Browse files
crisbetoAndrewKushnir
crisbeto
authored and
AndrewKushnir
committed
fix(migrations): delete barrel exports in standalone migration (#49176)
Adds some logic to automatically delete `export * from './foo'` style imports. Previously they weren't being picked up, because finding all the references using the language service doesn't include barrel exports. PR Close #49176
1 parent 79cdfeb commit 7dd1957

File tree

3 files changed

+160
-12
lines changed

3 files changed

+160
-12
lines changed

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

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ interface RemovalLocations {
1919
arrays: UniqueItemTracker<ts.ArrayLiteralExpression, ts.Node>;
2020
imports: UniqueItemTracker<ts.NamedImports, ts.Node>;
2121
exports: UniqueItemTracker<ts.NamedExports, ts.Node>;
22-
classes: Set<ts.ClassDeclaration>;
2322
unknown: Set<ts.Node>;
2423
}
2524

@@ -29,21 +28,33 @@ export function pruneNgModules(
2928
referenceLookupExcludedFiles?: RegExp) {
3029
const filesToRemove = new Set<ts.SourceFile>();
3130
const tracker = new ChangeTracker(printer, importRemapper);
32-
const typeChecker = program.getTsProgram().getTypeChecker();
31+
const tsProgram = program.getTsProgram();
32+
const typeChecker = tsProgram.getTypeChecker();
3333
const referenceResolver =
3434
new ReferenceResolver(program, host, rootFileNames, basePath, referenceLookupExcludedFiles);
3535
const removalLocations: RemovalLocations = {
3636
arrays: new UniqueItemTracker<ts.ArrayLiteralExpression, ts.Node>(),
3737
imports: new UniqueItemTracker<ts.NamedImports, ts.Node>(),
3838
exports: new UniqueItemTracker<ts.NamedExports, ts.Node>(),
39-
classes: new Set<ts.ClassDeclaration>(),
4039
unknown: new Set<ts.Node>()
4140
};
41+
const classesToRemove = new Set<ts.ClassDeclaration>();
42+
const barrelExports = new UniqueItemTracker<ts.SourceFile, ts.ExportDeclaration>();
43+
const nodesToRemove = new Set<ts.Node>();
4244

4345
sourceFiles.forEach(function walk(node: ts.Node) {
4446
if (ts.isClassDeclaration(node) && canRemoveClass(node, typeChecker)) {
4547
collectRemovalLocations(node, removalLocations, referenceResolver, program);
46-
removalLocations.classes.add(node);
48+
classesToRemove.add(node);
49+
} else if (
50+
ts.isExportDeclaration(node) && !node.exportClause && node.moduleSpecifier &&
51+
ts.isStringLiteralLike(node.moduleSpecifier) && node.moduleSpecifier.text.startsWith('.')) {
52+
const exportedSourceFile =
53+
typeChecker.getSymbolAtLocation(node.moduleSpecifier)?.valueDeclaration?.getSourceFile();
54+
55+
if (exportedSourceFile) {
56+
barrelExports.track(exportedSourceFile, node);
57+
}
4758
}
4859
node.forEachChild(walk);
4960
});
@@ -55,10 +66,27 @@ export function pruneNgModules(
5566
removeExportReferences(removalLocations.exports, tracker);
5667
addRemovalTodos(removalLocations.unknown, tracker);
5768

58-
for (const node of removalLocations.classes) {
69+
// Collect all the nodes to be removed before determining which files to delete since we need
70+
// to know it ahead of time when deleting barrel files that export other barrel files.
71+
(function trackNodesToRemove(nodes: Set<ts.Node>) {
72+
for (const node of nodes) {
73+
const sourceFile = node.getSourceFile();
74+
75+
if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodes)) {
76+
const barrelExportsForFile = barrelExports.get(sourceFile);
77+
nodesToRemove.add(node);
78+
filesToRemove.add(sourceFile);
79+
barrelExportsForFile && trackNodesToRemove(barrelExportsForFile);
80+
} else {
81+
nodesToRemove.add(node);
82+
}
83+
}
84+
})(classesToRemove);
85+
86+
for (const node of nodesToRemove) {
5987
const sourceFile = node.getSourceFile();
6088

61-
if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, removalLocations.classes)) {
89+
if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodesToRemove)) {
6290
filesToRemove.add(sourceFile);
6391
} else {
6492
tracker.removeNode(node);
@@ -276,17 +304,17 @@ function isNonEmptyNgModuleProperty(node: ts.Node): node is ts.PropertyAssignmen
276304
* Determines if a file is safe to delete. A file is safe to delete if all it contains are
277305
* import statements, class declarations that are about to be deleted and non-exported code.
278306
* @param sourceFile File that is being checked.
279-
* @param classesToBeRemoved Classes that are being removed as a part of the migration.
307+
* @param nodesToBeRemoved Nodes that are being removed as a part of the migration.
280308
*/
281-
function canRemoveFile(sourceFile: ts.SourceFile, classesToBeRemoved: Set<ts.ClassDeclaration>) {
309+
function canRemoveFile(sourceFile: ts.SourceFile, nodesToBeRemoved: Set<ts.Node>) {
282310
for (const node of sourceFile.statements) {
283-
if (ts.isImportDeclaration(node) ||
284-
(ts.isClassDeclaration(node) && classesToBeRemoved.has(node))) {
311+
if (ts.isImportDeclaration(node) || nodesToBeRemoved.has(node)) {
285312
continue;
286313
}
287314

288-
if (ts.canHaveModifiers(node) &&
289-
ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword)) {
315+
if (ts.isExportDeclaration(node) ||
316+
(ts.canHaveModifiers(node) &&
317+
ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword))) {
290318
return false;
291319
}
292320
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ export class UniqueItemTracker<K, V> {
175175
}
176176
}
177177

178+
get(key: K): Set<V>|undefined {
179+
return this._nodes.get(key);
180+
}
181+
178182
getEntries(): IterableIterator<[K, Set<V>]> {
179183
return this._nodes.entries();
180184
}

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2245,6 +2245,122 @@ describe('standalone migration', () => {
22452245
`));
22462246
});
22472247

2248+
it('should remove barrel export if the corresponding file is deleted', async () => {
2249+
writeFile('app.module.ts', `
2250+
import {NgModule} from '@angular/core';
2251+
import {MyComp} from './comp';
2252+
2253+
@NgModule({imports: [MyComp]})
2254+
export class AppModule {}
2255+
`);
2256+
2257+
writeFile('button.module.ts', `
2258+
import {NgModule} from '@angular/core';
2259+
import {MyButton} from './button';
2260+
2261+
@NgModule({imports: [MyButton], exports: [MyButton]})
2262+
export class ButtonModule {}
2263+
`);
2264+
2265+
writeFile('comp.ts', `
2266+
import {Component} from '@angular/core';
2267+
import {MyButton} from './button';
2268+
2269+
@Component({
2270+
selector: 'my-comp',
2271+
template: '<my-button>Hello</my-button>',
2272+
standalone: true,
2273+
imports: [MyButton]
2274+
})
2275+
export class MyComp {}
2276+
`);
2277+
2278+
writeFile('button.ts', `
2279+
import {Component} from '@angular/core';
2280+
2281+
@Component({selector: 'my-button', template: '<ng-content></ng-content>', standalone: true})
2282+
export class MyButton {}
2283+
`);
2284+
2285+
writeFile('index.ts', `
2286+
export * from './app.module';
2287+
export {MyComp} from './comp';
2288+
export {ButtonModule} from './button.module';
2289+
`);
2290+
2291+
await runMigration('prune-ng-modules');
2292+
2293+
expect(tree.exists('app.module.ts')).toBe(false);
2294+
expect(tree.exists('button.module.ts')).toBe(false);
2295+
expect(stripWhitespace(tree.readContent('index.ts'))).toBe(stripWhitespace(`
2296+
export {MyComp} from './comp';
2297+
`));
2298+
});
2299+
2300+
it('should remove barrel files referring to other barrel files that were deleted', async () => {
2301+
writeFile('app.module.ts', `
2302+
import {NgModule} from '@angular/core';
2303+
import {MyDir} from './dir';
2304+
2305+
@NgModule({imports: [MyDir]})
2306+
export class AppModule {}
2307+
`);
2308+
2309+
writeFile('dir.ts', `
2310+
import {Directive} from '@angular/core';
2311+
2312+
@Directive({selector: '[dir]', standalone: true})
2313+
export class MyDir {}
2314+
`);
2315+
2316+
writeFile('index.ts', `export * from './app.module';`);
2317+
writeFile('index-2.ts', `export * from './index';`);
2318+
writeFile('index-3.ts', `export * from './index-2';`);
2319+
2320+
await runMigration('prune-ng-modules');
2321+
2322+
expect(tree.exists('index.ts')).toBe(false);
2323+
expect(tree.exists('index-2.ts')).toBe(false);
2324+
expect(tree.exists('index-3.ts')).toBe(false);
2325+
});
2326+
2327+
it('should not delete dependent barrel files if they have some barrel exports that will not be removed',
2328+
async () => {
2329+
writeFile('app.module.ts', `
2330+
import {NgModule} from '@angular/core';
2331+
import {MyDir} from './dir';
2332+
2333+
@NgModule({imports: [MyDir]})
2334+
export class AppModule {}
2335+
`);
2336+
2337+
writeFile('dir.ts', `
2338+
import {Directive} from '@angular/core';
2339+
2340+
@Directive({selector: '[dir]', standalone: true})
2341+
export class MyDir {}
2342+
`);
2343+
2344+
writeFile('utils.ts', `
2345+
export function sum(a: number, b: number) { return a + b; }
2346+
`);
2347+
2348+
writeFile('index.ts', `export * from './app.module';`);
2349+
writeFile('index-2.ts', `
2350+
export * from './index';
2351+
export * from './utils';
2352+
`);
2353+
writeFile('index-3.ts', `export * from './index-2';`);
2354+
2355+
await runMigration('prune-ng-modules');
2356+
2357+
expect(tree.exists('index.ts')).toBe(false);
2358+
expect(stripWhitespace(tree.readContent('index-2.ts')))
2359+
.toBe(stripWhitespace(`export * from './utils';`));
2360+
expect(stripWhitespace(tree.readContent('index-3.ts')))
2361+
.toBe(stripWhitespace(`export * from './index-2';`));
2362+
});
2363+
22482364
it('should add a comment to locations that cannot be removed automatically', async () => {
22492365
writeFile('app.module.ts', `
22502366
import {NgModule} from '@angular/core';

0 commit comments

Comments
 (0)