fix(compiler-cli): exclude type-only imports from cycle analysis (#42453)
Type-only imports are known to be elided by TypeScript, so the compiler can be certain that such imports do not contribute to potential import cycles. As such, type-only imports are no longer considered during cycle analysis. Regular import statements that would eventually be fully elided by TypeScript during emit if none of the imported symbols are used in a value position continue to be included in the cycle analysis, as the cycle analyzer is unaware of these elision opportunities. Only explicit `import type` statements are excluded. PR Close #42453
This commit is contained in:
parent
374fa2c26f
commit
bd1836b999
|
@ -110,6 +110,13 @@ export class ImportGraph {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (ts.isImportDeclaration(stmt) && stmt.importClause !== undefined &&
|
||||||
|
stmt.importClause.isTypeOnly) {
|
||||||
|
// Exclude type-only imports as they are always elided, so they don't contribute to
|
||||||
|
// cycles.
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
const symbol = this.checker.getSymbolAtLocation(stmt.moduleSpecifier);
|
const symbol = this.checker.getSymbolAtLocation(stmt.moduleSpecifier);
|
||||||
if (symbol === undefined || symbol.valueDeclaration === undefined) {
|
if (symbol === undefined || symbol.valueDeclaration === undefined) {
|
||||||
// No symbol could be found to skip over this import/export.
|
// No symbol could be found to skip over this import/export.
|
||||||
|
|
|
@ -70,6 +70,17 @@ runInEachFileSystem(() => {
|
||||||
expect(cycle).toBeInstanceOf(Cycle);
|
expect(cycle).toBeInstanceOf(Cycle);
|
||||||
expect(importPath(cycle!.getPath())).toEqual('b,c,b');
|
expect(importPath(cycle!.getPath())).toEqual('b,c,b');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not consider type-only imports', () => {
|
||||||
|
const {program, analyzer} = makeAnalyzer('a:b,c!;b;c');
|
||||||
|
const a = getSourceFileOrError(program, (_('/a.ts')));
|
||||||
|
const b = getSourceFileOrError(program, (_('/b.ts')));
|
||||||
|
const c = getSourceFileOrError(program, (_('/c.ts')));
|
||||||
|
expect(analyzer.wouldCreateCycle(c, a)).toBe(null);
|
||||||
|
const cycle = analyzer.wouldCreateCycle(b, a);
|
||||||
|
expect(cycle).toBeInstanceOf(Cycle);
|
||||||
|
expect(importPath(cycle!.getPath())).toEqual('b,a,b');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
function makeAnalyzer(graph: string): {program: ts.Program, analyzer: CycleAnalyzer} {
|
function makeAnalyzer(graph: string): {program: ts.Program, analyzer: CycleAnalyzer} {
|
||||||
|
|
|
@ -30,6 +30,8 @@ import {makeProgram} from '../../testing';
|
||||||
* "a:*b,c;b;c"
|
* "a:*b,c;b;c"
|
||||||
*
|
*
|
||||||
* represents a program where a.ts exports from b.ts and imports from c.ts.
|
* represents a program where a.ts exports from b.ts and imports from c.ts.
|
||||||
|
*
|
||||||
|
* An import can be suffixed with ! to make it a type-only import.
|
||||||
*/
|
*/
|
||||||
export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
|
export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
|
||||||
program: ts.Program,
|
program: ts.Program,
|
||||||
|
@ -43,6 +45,9 @@ export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
|
||||||
if (i.startsWith('*')) {
|
if (i.startsWith('*')) {
|
||||||
const sym = i.substr(1);
|
const sym = i.substr(1);
|
||||||
return `export {${sym}} from './${sym}';`;
|
return `export {${sym}} from './${sym}';`;
|
||||||
|
} else if (i.endsWith('!')) {
|
||||||
|
const sym = i.substr(0, i.length - 1);
|
||||||
|
return `import type {${sym}} from './${sym}';`;
|
||||||
} else {
|
} else {
|
||||||
return `import {${i}} from './${i}';`;
|
return `import {${i}} from './${i}';`;
|
||||||
}
|
}
|
||||||
|
|
|
@ -4846,6 +4846,41 @@ function allTests(os: string) {
|
||||||
expect(jsContents).not.toContain('setComponentScope');
|
expect(jsContents).not.toContain('setComponentScope');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not consider type-only imports during cycle detection', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {NgModule} from '@angular/core';
|
||||||
|
import {ACmp} from './a';
|
||||||
|
import {BCmp} from './b';
|
||||||
|
|
||||||
|
@NgModule({declarations: [ACmp, BCmp]})
|
||||||
|
export class Module {}
|
||||||
|
`);
|
||||||
|
env.write('a.ts', `
|
||||||
|
import {Component} from '@angular/core';
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'a-cmp',
|
||||||
|
template: '<b-cmp></b-cmp>',
|
||||||
|
})
|
||||||
|
export class ACmp {}
|
||||||
|
`);
|
||||||
|
env.write('b.ts', `
|
||||||
|
import {Component} from '@angular/core';
|
||||||
|
import type {ACmp} from './a';
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'b-cmp',
|
||||||
|
template: 'does not use a-cmp',
|
||||||
|
})
|
||||||
|
export class BCmp {
|
||||||
|
a: ACmp;
|
||||||
|
}
|
||||||
|
`);
|
||||||
|
env.driveMain();
|
||||||
|
const jsContents = env.getContents('test.js');
|
||||||
|
expect(jsContents).not.toContain('setComponentScope');
|
||||||
|
});
|
||||||
|
|
||||||
it('should only pass components actually used to setComponentScope', () => {
|
it('should only pass components actually used to setComponentScope', () => {
|
||||||
env.write('test.ts', `
|
env.write('test.ts', `
|
||||||
import {Component, NgModule} from '@angular/core';
|
import {Component, NgModule} from '@angular/core';
|
||||||
|
|
Loading…
Reference in New Issue