Skip to content

Commit

Permalink
fix(migrations): delete barrel exports in standalone migration (#49176)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
crisbeto authored and AndrewKushnir committed Feb 24, 2023
1 parent d7835e4 commit 92b0bda
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ interface RemovalLocations {
arrays: UniqueItemTracker<ts.ArrayLiteralExpression, ts.Node>;
imports: UniqueItemTracker<ts.NamedImports, ts.Node>;
exports: UniqueItemTracker<ts.NamedExports, ts.Node>;
classes: Set<ts.ClassDeclaration>;
unknown: Set<ts.Node>;
}

Expand All @@ -29,21 +28,33 @@ export function pruneNgModules(
referenceLookupExcludedFiles?: RegExp) {
const filesToRemove = new Set<ts.SourceFile>();
const tracker = new ChangeTracker(printer, importRemapper);
const typeChecker = program.getTsProgram().getTypeChecker();
const tsProgram = program.getTsProgram();
const typeChecker = tsProgram.getTypeChecker();
const referenceResolver =
new ReferenceResolver(program, host, rootFileNames, basePath, referenceLookupExcludedFiles);
const removalLocations: RemovalLocations = {
arrays: new UniqueItemTracker<ts.ArrayLiteralExpression, ts.Node>(),
imports: new UniqueItemTracker<ts.NamedImports, ts.Node>(),
exports: new UniqueItemTracker<ts.NamedExports, ts.Node>(),
classes: new Set<ts.ClassDeclaration>(),
unknown: new Set<ts.Node>()
};
const classesToRemove = new Set<ts.ClassDeclaration>();
const barrelExports = new UniqueItemTracker<ts.SourceFile, ts.ExportDeclaration>();
const nodesToRemove = new Set<ts.Node>();

sourceFiles.forEach(function walk(node: ts.Node) {
if (ts.isClassDeclaration(node) && canRemoveClass(node, typeChecker)) {
collectRemovalLocations(node, removalLocations, referenceResolver, program);
removalLocations.classes.add(node);
classesToRemove.add(node);
} else if (
ts.isExportDeclaration(node) && !node.exportClause && node.moduleSpecifier &&
ts.isStringLiteralLike(node.moduleSpecifier) && node.moduleSpecifier.text.startsWith('.')) {
const exportedSourceFile =
typeChecker.getSymbolAtLocation(node.moduleSpecifier)?.valueDeclaration?.getSourceFile();

if (exportedSourceFile) {
barrelExports.track(exportedSourceFile, node);
}
}
node.forEachChild(walk);
});
Expand All @@ -55,10 +66,27 @@ export function pruneNgModules(
removeExportReferences(removalLocations.exports, tracker);
addRemovalTodos(removalLocations.unknown, tracker);

for (const node of removalLocations.classes) {
// Collect all the nodes to be removed before determining which files to delete since we need
// to know it ahead of time when deleting barrel files that export other barrel files.
(function trackNodesToRemove(nodes: Set<ts.Node>) {
for (const node of nodes) {
const sourceFile = node.getSourceFile();

if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodes)) {
const barrelExportsForFile = barrelExports.get(sourceFile);
nodesToRemove.add(node);
filesToRemove.add(sourceFile);
barrelExportsForFile && trackNodesToRemove(barrelExportsForFile);
} else {
nodesToRemove.add(node);
}
}
})(classesToRemove);

for (const node of nodesToRemove) {
const sourceFile = node.getSourceFile();

if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, removalLocations.classes)) {
if (!filesToRemove.has(sourceFile) && canRemoveFile(sourceFile, nodesToRemove)) {
filesToRemove.add(sourceFile);
} else {
tracker.removeNode(node);
Expand Down Expand Up @@ -276,17 +304,17 @@ function isNonEmptyNgModuleProperty(node: ts.Node): node is ts.PropertyAssignmen
* Determines if a file is safe to delete. A file is safe to delete if all it contains are
* import statements, class declarations that are about to be deleted and non-exported code.
* @param sourceFile File that is being checked.
* @param classesToBeRemoved Classes that are being removed as a part of the migration.
* @param nodesToBeRemoved Nodes that are being removed as a part of the migration.
*/
function canRemoveFile(sourceFile: ts.SourceFile, classesToBeRemoved: Set<ts.ClassDeclaration>) {
function canRemoveFile(sourceFile: ts.SourceFile, nodesToBeRemoved: Set<ts.Node>) {
for (const node of sourceFile.statements) {
if (ts.isImportDeclaration(node) ||
(ts.isClassDeclaration(node) && classesToBeRemoved.has(node))) {
if (ts.isImportDeclaration(node) || nodesToBeRemoved.has(node)) {
continue;
}

if (ts.canHaveModifiers(node) &&
ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword)) {
if (ts.isExportDeclaration(node) ||
(ts.canHaveModifiers(node) &&
ts.getModifiers(node)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword))) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ export class UniqueItemTracker<K, V> {
}
}

get(key: K): Set<V>|undefined {
return this._nodes.get(key);
}

getEntries(): IterableIterator<[K, Set<V>]> {
return this._nodes.entries();
}
Expand Down
116 changes: 116 additions & 0 deletions packages/core/schematics/test/standalone_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2245,6 +2245,122 @@ describe('standalone migration', () => {
`));
});

it('should remove barrel export if the corresponding file is deleted', async () => {
writeFile('app.module.ts', `
import {NgModule} from '@angular/core';
import {MyComp} from './comp';
@NgModule({imports: [MyComp]})
export class AppModule {}
`);

writeFile('button.module.ts', `
import {NgModule} from '@angular/core';
import {MyButton} from './button';
@NgModule({imports: [MyButton], exports: [MyButton]})
export class ButtonModule {}
`);

writeFile('comp.ts', `
import {Component} from '@angular/core';
import {MyButton} from './button';
@Component({
selector: 'my-comp',
template: '<my-button>Hello</my-button>',
standalone: true,
imports: [MyButton]
})
export class MyComp {}
`);

writeFile('button.ts', `
import {Component} from '@angular/core';
@Component({selector: 'my-button', template: '<ng-content></ng-content>', standalone: true})
export class MyButton {}
`);

writeFile('index.ts', `
export * from './app.module';
export {MyComp} from './comp';
export {ButtonModule} from './button.module';
`);

await runMigration('prune-ng-modules');

expect(tree.exists('app.module.ts')).toBe(false);
expect(tree.exists('button.module.ts')).toBe(false);
expect(stripWhitespace(tree.readContent('index.ts'))).toBe(stripWhitespace(`
export {MyComp} from './comp';
`));
});

it('should remove barrel files referring to other barrel files that were deleted', async () => {
writeFile('app.module.ts', `
import {NgModule} from '@angular/core';
import {MyDir} from './dir';
@NgModule({imports: [MyDir]})
export class AppModule {}
`);

writeFile('dir.ts', `
import {Directive} from '@angular/core';
@Directive({selector: '[dir]', standalone: true})
export class MyDir {}
`);

writeFile('index.ts', `export * from './app.module';`);
writeFile('index-2.ts', `export * from './index';`);
writeFile('index-3.ts', `export * from './index-2';`);

await runMigration('prune-ng-modules');

expect(tree.exists('index.ts')).toBe(false);
expect(tree.exists('index-2.ts')).toBe(false);
expect(tree.exists('index-3.ts')).toBe(false);
});

it('should not delete dependent barrel files if they have some barrel exports that will not be removed',
async () => {
writeFile('app.module.ts', `
import {NgModule} from '@angular/core';
import {MyDir} from './dir';
@NgModule({imports: [MyDir]})
export class AppModule {}
`);

writeFile('dir.ts', `
import {Directive} from '@angular/core';
@Directive({selector: '[dir]', standalone: true})
export class MyDir {}
`);

writeFile('utils.ts', `
export function sum(a: number, b: number) { return a + b; }
`);

writeFile('index.ts', `export * from './app.module';`);
writeFile('index-2.ts', `
export * from './index';
export * from './utils';
`);
writeFile('index-3.ts', `export * from './index-2';`);

await runMigration('prune-ng-modules');

expect(tree.exists('index.ts')).toBe(false);
expect(stripWhitespace(tree.readContent('index-2.ts')))
.toBe(stripWhitespace(`export * from './utils';`));
expect(stripWhitespace(tree.readContent('index-3.ts')))
.toBe(stripWhitespace(`export * from './index-2';`));
});

it('should add a comment to locations that cannot be removed automatically', async () => {
writeFile('app.module.ts', `
import {NgModule} from '@angular/core';
Expand Down

0 comments on commit 92b0bda

Please sign in to comment.