fix(compiler-cli): handle standalone components with cycles (#46029)

The Angular compiler performs cycle detection when generating imports within
component files. This was previously necessary as reifying dependencies
discovered via NgModules into the component output could add imports that
weren't present in the original component and potentially create cycles.
Doing this could cause order-of-execution issues with existing user imports,
so the compiler detects this case and falls back to an alternative way of
specifying component dependencies that doesn't risk creating cycles.

For standalone components, Angular does not need to add new imports to the
component file as the user has already explicitly referenced dependencies
in the `@Component.imports`. As a result, the cycle detection can be
skipped.

Correctly authoring a program with import cycles is always challenging. One
side of a cyclic import will always initially evaluate to `undefined`, and
this can result in errors in the component definition when this happens
within component `imports`.

Our compiler _could_ detect the cycle and choose to wrap the component
dependencies in an automatic closure instead, avoiding any issues with
`undefined` during an eager evaluation. However, this commit makes an active
choice not to do that as it only serves to mask the problems with cyclic
imports. Future refactorings may cause the "other half" of the cycle to
break. Users should instead be aware of the potential problems with cycles
and explicitly defer evaluations with `forwardRef` where needed. This
ensures that future implementations of Angular compilation which may not be
able to automatically detect import cycles and correct accordingly can still
compile such components.

PR Close #46029
This commit is contained in:
Alex Rickabaugh
2022-05-17 14:43:03 -07:00
committed by Jessica Janiuk
parent 038ba8adea
commit 91ce543ef4
2 changed files with 58 additions and 14 deletions
@@ -717,20 +717,26 @@ export class ComponentDecoratorHandler implements
symbol.usedPipes = declarations.filter(isUsedPipe).map(getSemanticReference);
}
// Scan through the directives/pipes actually used in the template and check whether any
// import which needs to be generated would create a cycle.
const cyclesFromDirectives = new Map<UsedDirective, Cycle>();
const cyclesFromPipes = new Map<UsedPipe, Cycle>();
for (const usedDep of declarations) {
const cycle = this._checkForCyclicImport(usedDep.importedFile, usedDep.type, context);
if (cycle !== null) {
switch (usedDep.kind) {
case R3TemplateDependencyKind.Directive:
cyclesFromDirectives.set(usedDep, cycle);
break;
case R3TemplateDependencyKind.Pipe:
cyclesFromPipes.set(usedDep, cycle);
break;
// Scan through the directives/pipes actually used in the template and check whether any
// import which needs to be generated would create a cycle. This check is skipped for
// standalone components as the dependencies of a standalone component have already been
// imported directly by the user, so Angular won't introduce any imports that aren't already
// in the user's program.
if (!metadata.isStandalone) {
for (const usedDep of declarations) {
const cycle = this._checkForCyclicImport(usedDep.importedFile, usedDep.type, context);
if (cycle !== null) {
switch (usedDep.kind) {
case R3TemplateDependencyKind.Directive:
cyclesFromDirectives.set(usedDep, cycle);
break;
case R3TemplateDependencyKind.Pipe:
cyclesFromPipes.set(usedDep, cycle);
break;
}
}
}
}
@@ -740,7 +746,7 @@ export class ComponentDecoratorHandler implements
// No cycle was detected. Record the imports that need to be created in the cycle detector
// so that future cyclic import checks consider their production.
for (const {type, importedFile} of declarations) {
this._recordSyntheticImport(importedFile, type, context);
this.maybeRecordSyntheticImport(importedFile, type, context);
}
// Check whether the dependencies arrays in ɵcmp need to be wrapped in a closure.
@@ -932,7 +938,7 @@ export class ComponentDecoratorHandler implements
return this.cycleAnalyzer.wouldCreateCycle(origin, imported);
}
private _recordSyntheticImport(
private maybeRecordSyntheticImport(
importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): void {
const imported = resolveImportedFile(this.moduleResolver, importedFile, expr, origin);
if (imported === null) {
@@ -93,6 +93,44 @@ runInEachFileSystem(() => {
expect(dtsCode).toContain('i0.ɵɵPipeDeclaration<TestPipe, "test", true>');
});
it('should compile a standalone component even in the presence of cycles', () => {
env.write('dep.ts', `
import {Directive, Input} from '@angular/core';
// This import creates a cycle, since 'test.ts' imports 'dir.ts'.
import {TestType} from './test';
@Directive({
standalone: true,
selector: '[dir]',
})
export class TestDir {
@Input() value?: TestType;
}
`);
env.write('test.ts', `
import {Component} from '@angular/core';
import {TestDir} from './dep';
export interface TestType {
value: string;
}
@Component({
standalone: true,
imports: [TestDir],
selector: 'test-cmp',
template: '<div dir></div>',
})
export class TestCmp {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('dependencies: [i1.TestDir]');
});
it('should error when a non-standalone component tries to use imports', () => {
env.write('test.ts', `
import {Component, Directive} from '@angular/core';