refactor(compiler-cli): error on cyclic imports in partial compilation (#40782)

Our approach for handling cyclic imports results in code that is
not easy to tree-shake, so it is not suitable for publishing in a
library.

When compiling in partial compilation mode, we are targeting
such library publication, so we now create a fatal diagnostic
error instead of trying to handle the cyclic import situation.

Closes #40678

PR Close #40782

.pullapprove.yml

@@ -1223,6 +1223,7 @@ groups:
           'aio/content/errors/*.md',
           'aio/content/guide/glossary.md',
           'aio/content/guide/styleguide.md',
+          'aio/content/examples/errors/**',
           'aio/content/examples/styleguide/**',
           'aio/content/images/guide/styleguide/*'
           ])

aio/content/errors/NG3003.md

@@ -0,0 +1,55 @@
+@name Import Cycle Detected
+@category compiler
+@shortDescription Import cycles would need to be created to compile this component
+
+@description
+
+A component, directive or pipe that is referenced by this component would require the compiler
+to add an import that would lead to a cycle of imports.  For example, consider a scenario where
+a `ParentComponent` references a `ChildComponent` in its template:
+
+<code-example path="errors/cyclic-imports/parent.component.ts" header="parent.component.ts"></code-example>
+
+<code-example path="errors/cyclic-imports/child.component.ts" header="child.component.ts"></code-example>
+
+There is already an import from `child.component.ts` to `parent.component.ts` since the `ChildComponent`
+references the `ParentComponent` in its constructor.
+
+But note that the parent component's template contains `<child></child>`. The generated code for this
+template must therefore contain a reference to the `ChildComponent` class. In order to make this reference
+the compiler would have to add an import from `parent.component.ts` to `child.component.ts`, which would
+cause an import cycle:
+
+```
+parent.component.ts -> child.component.ts -> parent.component.ts
+```
+
+### Remote Scoping
+
+To avoid adding imports that create cycles, additional code is added to the `NgModule` class where
+the component is declared that wires up the dependencies. This is known as "remote scoping".
+
+
+### Libraries
+
+Unfortunately, "remote scoping" code is side-effectful, which prevents tree shaking, and cannot
+be used in libraries. So when building libraries using the `"compilationMode": "partial"` setting,
+any component that would require a cyclic import will cause this `NG3003` compiler error to be raised.
+
+
+@debugging
+
+The cycle that would be generated is shown as part of the error message. For example:
+
+<code-example hideCopy="true">
+<span class="nocode">The component ChildComponent is used in the template but importing it would create a cycle:
+/parent.component.ts -> /child.component.ts -> /parent.component.ts</span>
+</code-example>
+
+Use this to identify how the referenced component, pipe or directive has a dependency back to the
+component being compiled. Here are some ideas for fixing the problem:
+
+* Try to re-arrange your dependencies to avoid the cycle. For example using an intermediate interface
+that is stored in an independent file that can be imported to both dependent files without
+causing an import cycle.
+* Move the classes that reference each other into the same file, to avoid any imports between them.

aio/content/examples/errors/cyclic-imports/child.component.ts

@@ -0,0 +1,7 @@
+import { Component } from '@angular/core';
+import { ParentComponent } from './parent.component';
+
+@Component({selector: 'app-child', template: 'The child!'})
+export class ChildComponent {
+  constructor(private parent: ParentComponent) {}
+}

aio/content/examples/errors/cyclic-imports/module.ts

@@ -0,0 +1,8 @@
+import { NgModule } from '@angular/core';
+import { ChildComponent } from './child.component';
+import { ParentComponent } from './parent.component';
+
+@NgModule({
+  declarations: [ParentComponent, ChildComponent]
+})
+export class AppModule {}

aio/content/examples/errors/cyclic-imports/parent.component.ts

@@ -0,0 +1,4 @@
+import { Component } from '@angular/core';
+
+@Component({selector: 'app-parent', template: '<app-child></app-child>'})
+export class ParentComponent {}

goldens/public-api/compiler-cli/error_code.d.ts

@@ -17,6 +17,7 @@ export declare enum ErrorCode {
     COMPONENT_RESOURCE_NOT_FOUND = 2008,
     SYMBOL_NOT_EXPORTED = 3001,
     SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
+    IMPORT_CYCLE_DETECTED = 3003,
     CONFIG_FLAT_MODULE_NO_INDEX = 4001,
     CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
     HOST_BINDING_PARSE_ERROR = 5001,

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -10,7 +10,7 @@ import * as ts from 'typescript';
 
 import {ParsedConfiguration} from '../../..';
 import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
-import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
+import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../../src/ngtsc/cycles';
 import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
 import {absoluteFromSourceFile, LogicalFileSystem, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
 import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, PrivateExportAliasingHost, Reexport, ReferenceEmitter} from '../../../src/ngtsc/imports';
@@ -101,8 +101,9 @@ export class DecorationAnalyzer {
         /* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat,
         /* usePoisonedData */ false,
         /* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer,
-        this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, NOOP_DEPENDENCY_TRACKER,
-        this.injectableRegistry, !!this.compilerOptions.annotateForClosureCompiler),
+        CycleHandlingStrategy.UseRemoteScoping, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER,
+        NOOP_DEPENDENCY_TRACKER, this.injectableRegistry,
+        !!this.compilerOptions.annotateForClosureCompiler),
     // See the note in ngtsc about why this cast is needed.
     // clang-format off
     new DirectiveDecoratorHandler(

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -9,9 +9,9 @@
 import {compileComponentFromMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, makeBindingParser, ParsedTemplate, ParseSourceFile, parseTemplate, R3ComponentDef, R3ComponentMetadata, R3FactoryTarget, R3TargetBinder, R3UsedDirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler';
 import * as ts from 'typescript';
 
-import {CycleAnalyzer} from '../../cycles';
-import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
-import {absoluteFrom, AbsoluteFsPath, relative} from '../../file_system';
+import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
+import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics';
+import {absoluteFrom, relative} from '../../file_system';
 import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
 import {DependencyTracker} from '../../incremental/api';
 import {IndexingContext} from '../../indexer';
@@ -127,7 +127,8 @@ export class ComponentDecoratorHandler implements
       private enableI18nLegacyMessageIdFormat: boolean, private usePoisonedData: boolean,
       private i18nNormalizeLineEndingsInICUs: boolean|undefined,
       private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
-      private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder,
+      private cycleHandlingStrategy: CycleHandlingStrategy, private refEmitter: ReferenceEmitter,
+      private defaultImportRecorder: DefaultImportRecorder,
       private depTracker: DependencyTracker|null,
       private injectableRegistry: InjectableClassRegistry,
       private annotateForClosureCompiler: boolean) {}
@@ -546,10 +547,12 @@ export class ComponentDecoratorHandler implements
           inputs: directive.inputs.propertyNames,
           outputs: directive.outputs.propertyNames,
           exportAs: directive.exportAs,
+          isComponent: directive.isComponent,
         };
       });
 
-      const usedPipes: {ref: Reference, pipeName: string, expression: Expression}[] = [];
+      type UsedPipe = {ref: Reference, pipeName: string, expression: Expression};
+      const usedPipes: UsedPipe[] = [];
       for (const pipeName of bound.getUsedPipes()) {
         if (!pipes.has(pipeName)) {
           continue;
@@ -564,10 +567,22 @@ export class ComponentDecoratorHandler implements
 
       // 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 cycleDetected = usedDirectives.some(dir => this._isCyclicImport(dir.type, context)) ||
-          usedPipes.some(pipe => this._isCyclicImport(pipe.expression, context));
+      const cyclesFromDirectives = new Map<UsedDirective, Cycle>();
+      for (const usedDirective of usedDirectives) {
+        const cycle = this._checkForCyclicImport(usedDirective.ref, usedDirective.type, context);
+        if (cycle !== null) {
+          cyclesFromDirectives.set(usedDirective, cycle);
+        }
+      }
+      const cyclesFromPipes = new Map<UsedPipe, Cycle>();
+      for (const usedPipe of usedPipes) {
+        const cycle = this._checkForCyclicImport(usedPipe.ref, usedPipe.expression, context);
+        if (cycle !== null) {
+          cyclesFromPipes.set(usedPipe, cycle);
+        }
+      }
 
-      if (!cycleDetected) {
+      if (cyclesFromDirectives.size === 0 && cyclesFromPipes.size === 0) {
         // 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} of usedDirectives) {
@@ -592,11 +607,28 @@ export class ComponentDecoratorHandler implements
             DeclarationListEmitMode.Closure :
             DeclarationListEmitMode.Direct;
       } else {
-        // Declaring the directiveDefs/pipeDefs arrays directly would require imports that would
-        // create a cycle. Instead, mark this component as requiring remote scoping, so that the
-        // NgModule file will take care of setting the directives for the component.
-        this.scopeRegistry.setComponentRemoteScope(
-            node, usedDirectives.map(dir => dir.ref), usedPipes.map(pipe => pipe.ref));
+        if (this.cycleHandlingStrategy === CycleHandlingStrategy.UseRemoteScoping) {
+          // Declaring the directiveDefs/pipeDefs arrays directly would require imports that would
+          // create a cycle. Instead, mark this component as requiring remote scoping, so that the
+          // NgModule file will take care of setting the directives for the component.
+          this.scopeRegistry.setComponentRemoteScope(
+              node, usedDirectives.map(dir => dir.ref), usedPipes.map(pipe => pipe.ref));
+        } else {
+          // We are not able to handle this cycle so throw an error.
+          const relatedMessages: ts.DiagnosticRelatedInformation[] = [];
+          for (const [dir, cycle] of cyclesFromDirectives) {
+            relatedMessages.push(
+                makeCyclicImportInfo(dir.ref, dir.isComponent ? 'component' : 'directive', cycle));
+          }
+          for (const [pipe, cycle] of cyclesFromPipes) {
+            relatedMessages.push(makeCyclicImportInfo(pipe.ref, 'pipe', cycle));
+          }
+          throw new FatalDiagnosticError(
+              ErrorCode.IMPORT_CYCLE_DETECTED, node,
+              'One or more import cycles would need to be created to compile this component, ' +
+                  'which is not supported by the current compiler configuration.',
+              relatedMessages);
+        }
       }
     }
 
@@ -1052,14 +1084,20 @@ export class ComponentDecoratorHandler implements
     return this.moduleResolver.resolveModule(expr.value.moduleName!, origin.fileName);
   }
 
-  private _isCyclicImport(expr: Expression, origin: ts.SourceFile): boolean {
-    const imported = this._expressionToImportedFile(expr, origin);
-    if (imported === null) {
-      return false;
+  /**
+   * Check whether adding an import from `origin` to the source-file corresponding to `expr` would
+   * create a cyclic import.
+   *
+   * @returns a `Cycle` object if a cycle would be created, otherwise `null`.
+   */
+  private _checkForCyclicImport(ref: Reference, expr: Expression, origin: ts.SourceFile): Cycle
+      |null {
+    const importedFile = this._expressionToImportedFile(expr, origin);
+    if (importedFile === null) {
+      return null;
     }
-
     // Check whether the import is legal.
-    return this.cycleAnalyzer.wouldCreateCycle(origin, imported);
+    return this.cycleAnalyzer.wouldCreateCycle(origin, importedFile);
   }
 
   private _recordSyntheticImport(expr: Expression, origin: ts.SourceFile): void {
@@ -1219,3 +1257,15 @@ interface ExternalTemplateDeclaration extends CommonTemplateDeclaration {
  * record without needing to parse the original decorator again.
  */
 type TemplateDeclaration = InlineTemplateDeclaration|ExternalTemplateDeclaration;
+
+/**
+ * Generate a diagnostic related information object that describes a potential cyclic import path.
+ */
+function makeCyclicImportInfo(
+    ref: Reference, type: string, cycle: Cycle): ts.DiagnosticRelatedInformation {
+  const name = ref.debugName || '(unknown)';
+  const path = cycle.getPath().map(sf => sf.fileName).join(' -> ');
+  const message =
+      `The ${type} '${name}' is used in the template but importing it would create a cycle: `;
+  return makeRelatedInformation(ref.node, message + path);
+}

packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts

@@ -9,7 +9,7 @@
 import {ConstantPool} from '@angular/compiler';
 import * as ts from 'typescript';
 
-import {CycleAnalyzer, ImportGraph} from '../../cycles';
+import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles';
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
 import {absoluteFrom} from '../../file_system';
 import {runInEachFileSystem} from '../../file_system/testing';
@@ -73,6 +73,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil
       /* i18nNormalizeLineEndingsInICUs */ undefined,
       moduleResolver,
       cycleAnalyzer,
+      CycleHandlingStrategy.UseRemoteScoping,
       refEmitter,
       NOOP_DEFAULT_IMPORT_RECORDER,
       /* depTracker */ null,

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -10,7 +10,7 @@ import {Type} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ReferencesRegistry} from '../../annotations';
-import {CycleAnalyzer, ImportGraph} from '../../cycles';
+import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles';
 import {COMPILER_ERRORS_WITH_GUIDES, ERROR_DETAILS_PAGE_BASE_URL, ErrorCode, ngErrorCode} from '../../diagnostics';
 import {checkForPrivateExports, ReferenceGraph} from '../../entry_point';
 import {LogicalFileSystem, resolve} from '../../file_system';
@@ -985,6 +985,16 @@ export class NgCompiler {
     const defaultImportTracker = new DefaultImportTracker();
     const resourceRegistry = new ResourceRegistry();
 
+    const compilationMode =
+        this.options.compilationMode === 'partial' ? CompilationMode.PARTIAL : CompilationMode.FULL;
+
+    // Cycles are handled in full compilation mode by "remote scoping".
+    // "Remote scoping" does not work well with tree shaking for libraries.
+    // So in partial compilation mode, when building a library, a cycle will cause an error.
+    const cycleHandlingStrategy = compilationMode === CompilationMode.FULL ?
+        CycleHandlingStrategy.UseRemoteScoping :
+        CycleHandlingStrategy.Error;
+
     // Set up the IvyCompilation, which manages state for the Ivy transformer.
     const handlers: DecoratorHandler<unknown, unknown, unknown>[] = [
       new ComponentDecoratorHandler(
@@ -994,8 +1004,8 @@ export class NgCompiler {
           this.options.i18nUseExternalIds !== false,
           this.options.enableI18nLegacyMessageIdFormat !== false, this.usePoisonedData,
           this.options.i18nNormalizeLineEndingsInICUs, this.moduleResolver, this.cycleAnalyzer,
-          refEmitter, defaultImportTracker, this.incrementalDriver.depGraph, injectableRegistry,
-          this.closureCompilerEnabled),
+          cycleHandlingStrategy, refEmitter, defaultImportTracker, this.incrementalDriver.depGraph,
+          injectableRegistry, this.closureCompilerEnabled),
       // TODO(alxhub): understand why the cast here is necessary (something to do with `null`
       // not being assignable to `unknown` when wrapped in `Readonly`).
       // clang-format off
@@ -1022,8 +1032,6 @@ export class NgCompiler {
           this.closureCompilerEnabled, injectableRegistry, this.options.i18nInLocale),
     ];
 
-    const compilationMode =
-        this.options.compilationMode === 'partial' ? CompilationMode.PARTIAL : CompilationMode.FULL;
     const traitCompiler = new TraitCompiler(
         handlers, reflector, this.perfRecorder, this.incrementalDriver,
         this.options.compileNonExportedClasses !== false, compilationMode, dtsTransforms);

packages/compiler-cli/src/ngtsc/cycles/index.ts

@@ -6,5 +6,5 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export {CycleAnalyzer} from './src/analyzer';
+export {Cycle, CycleAnalyzer, CycleHandlingStrategy} from './src/analyzer';
 export {ImportGraph} from './src/imports';

packages/compiler-cli/src/ngtsc/cycles/src/analyzer.ts

@@ -17,11 +17,17 @@ export class CycleAnalyzer {
   constructor(private importGraph: ImportGraph) {}
 
   /**
-   * Check whether adding an import from `from` to `to` would create a cycle in the `ts.Program`.
+   * Check for a cycle to be created in the `ts.Program` by adding an import between `from` and
+   * `to`.
+   *
+   * @returns a `Cycle` object if an import between `from` and `to` would create a cycle; `null`
+   *     otherwise.
    */
-  wouldCreateCycle(from: ts.SourceFile, to: ts.SourceFile): boolean {
+  wouldCreateCycle(from: ts.SourceFile, to: ts.SourceFile): Cycle|null {
     // Import of 'from' -> 'to' is illegal if an edge 'to' -> 'from' already exists.
-    return this.importGraph.transitiveImportsOf(to).has(from);
+    return this.importGraph.transitiveImportsOf(to).has(from) ?
+        new Cycle(this.importGraph, from, to) :
+        null;
   }
 
   /**
@@ -34,3 +40,35 @@ export class CycleAnalyzer {
     this.importGraph.addSyntheticImport(from, to);
   }
 }
+
+/**
+ * Represents an import cycle between `from` and `to` in the program.
+ *
+ * This class allows us to do the work to compute the cyclic path between `from` and `to` only if
+ * needed.
+ */
+export class Cycle {
+  constructor(
+      private importGraph: ImportGraph, readonly from: ts.SourceFile, readonly to: ts.SourceFile) {}
+
+  /**
+   * Compute an array of source-files that illustrates the cyclic path between `from` and `to`.
+   *
+   * Note that a `Cycle` will not be created unless a path is available between `to` and `from`,
+   * so `findPath()` will never return `null`.
+   */
+  getPath(): ts.SourceFile[] {
+    return [this.from, ...this.importGraph.findPath(this.to, this.from)!];
+  }
+}
+
+
+/**
+ * What to do if a cycle is detected.
+ */
+export const enum CycleHandlingStrategy {
+  /** Add "remote scoping" code to avoid creating a cycle. */
+  UseRemoteScoping,
+  /** Fail the compilation with an error. */
+  Error,
+}

packages/compiler-cli/src/ngtsc/cycles/src/imports.ts

@@ -52,6 +52,44 @@ export class ImportGraph {
     });
   }
 
+  /**
+   * Find an import path from the `start` SourceFile to the `end` SourceFile.
+   *
+   * This function implements a breadth first search that results in finding the
+   * shortest path between the `start` and `end` points.
+   *
+   * @param start the starting point of the path.
+   * @param end the ending point of the path.
+   * @returns an array of source files that connect the `start` and `end` source files, or `null` if
+   *     no path could be found.
+   */
+  findPath(start: ts.SourceFile, end: ts.SourceFile): ts.SourceFile[]|null {
+    if (start === end) {
+      // Escape early for the case where `start` and `end` are the same.
+      return [start];
+    }
+
+    const found = new Set<ts.SourceFile>([start]);
+    const queue: Found[] = [new Found(start, null)];
+
+    while (queue.length > 0) {
+      const current = queue.shift()!;
+      const imports = this.importsOf(current.sourceFile);
+      for (const importedFile of imports) {
+        if (!found.has(importedFile)) {
+          const next = new Found(importedFile, current);
+          if (next.sourceFile === end) {
+            // We have hit the target `end` path so we can stop here.
+            return next.toPath();
+          }
+          found.add(importedFile);
+          queue.push(next);
+        }
+      }
+    }
+    return null;
+  }
+
   /**
    * Add a record of an import from `sf` to `imported`, that's not present in the original
    * `ts.Program` but will be remembered by the `ImportGraph`.
@@ -84,3 +122,27 @@ export class ImportGraph {
 function isLocalFile(sf: ts.SourceFile): boolean {
   return !sf.fileName.endsWith('.d.ts');
 }
+
+/**
+ * A helper class to track which SourceFiles are being processed when searching for a path in
+ * `getPath()` above.
+ */
+class Found {
+  constructor(readonly sourceFile: ts.SourceFile, readonly parent: Found|null) {}
+
+  /**
+   * Back track through this found SourceFile and its ancestors to generate an array of
+   * SourceFiles that form am import path between two SourceFiles.
+   */
+  toPath(): ts.SourceFile[] {
+    const array: ts.SourceFile[] = [];
+    let current: Found|null = this;
+    while (current !== null) {
+      array.push(current.sourceFile);
+      current = current.parent;
+    }
+    // Pushing and then reversing, O(n), rather than unshifting repeatedly, O(n^2), avoids
+    // manipulating the array on every iteration: https://stackoverflow.com/a/26370620
+    return array.reverse();
+  }
+}

packages/compiler-cli/src/ngtsc/cycles/test/analyzer_spec.ts

@@ -9,9 +9,9 @@ import * as ts from 'typescript';
 import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_system';
 import {runInEachFileSystem} from '../../file_system/testing';
 import {ModuleResolver} from '../../imports';
-import {CycleAnalyzer} from '../src/analyzer';
+import {Cycle, CycleAnalyzer} from '../src/analyzer';
 import {ImportGraph} from '../src/imports';
-import {makeProgramFromGraph} from './util';
+import {importPath, makeProgramFromGraph} from './util';
 
 runInEachFileSystem(() => {
   describe('cycle analyzer', () => {
@@ -22,41 +22,53 @@ runInEachFileSystem(() => {
       const {program, analyzer} = makeAnalyzer('a:b,c;b;c');
       const b = getSourceFileOrError(program, (_('/b.ts')));
       const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(analyzer.wouldCreateCycle(b, c)).toBe(false);
-      expect(analyzer.wouldCreateCycle(c, b)).toBe(false);
+      expect(analyzer.wouldCreateCycle(b, c)).toBe(null);
+      expect(analyzer.wouldCreateCycle(c, b)).toBe(null);
     });
 
     it('should detect a simple cycle between two files', () => {
       const {program, analyzer} = makeAnalyzer('a:b;b');
       const a = getSourceFileOrError(program, (_('/a.ts')));
       const b = getSourceFileOrError(program, (_('/b.ts')));
-      expect(analyzer.wouldCreateCycle(a, b)).toBe(false);
-      expect(analyzer.wouldCreateCycle(b, a)).toBe(true);
+      expect(analyzer.wouldCreateCycle(a, b)).toBe(null);
+      const cycle = analyzer.wouldCreateCycle(b, a);
+      expect(cycle).toBeInstanceOf(Cycle);
+      expect(importPath(cycle!.getPath())).toEqual('b,a,b');
     });
 
     it('should detect a cycle with a re-export in the chain', () => {
       const {program, analyzer} = makeAnalyzer('a:*b;b:c;c');
       const a = getSourceFileOrError(program, (_('/a.ts')));
+      const b = getSourceFileOrError(program, (_('/b.ts')));
       const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(analyzer.wouldCreateCycle(a, c)).toBe(false);
-      expect(analyzer.wouldCreateCycle(c, a)).toBe(true);
+      expect(analyzer.wouldCreateCycle(a, c)).toBe(null);
+      const cycle = analyzer.wouldCreateCycle(c, a);
+      expect(cycle).toBeInstanceOf(Cycle);
+      expect(importPath(cycle!.getPath())).toEqual('c,a,b,c');
     });
 
     it('should detect a cycle in a more complex program', () => {
       const {program, analyzer} = makeAnalyzer('a:*b,*c;b:*e,*f;c:*g,*h;e:f;f:c;g;h:g');
       const b = getSourceFileOrError(program, (_('/b.ts')));
+      const c = getSourceFileOrError(program, (_('/c.ts')));
+      const e = getSourceFileOrError(program, (_('/e.ts')));
+      const f = getSourceFileOrError(program, (_('/f.ts')));
       const g = getSourceFileOrError(program, (_('/g.ts')));
-      expect(analyzer.wouldCreateCycle(b, g)).toBe(false);
-      expect(analyzer.wouldCreateCycle(g, b)).toBe(true);
+      expect(analyzer.wouldCreateCycle(b, g)).toBe(null);
+      const cycle = analyzer.wouldCreateCycle(g, b);
+      expect(cycle).toBeInstanceOf(Cycle);
+      expect(importPath(cycle!.getPath())).toEqual('g,b,f,c,g');
     });
 
     it('should detect a cycle caused by a synthetic edge', () => {
       const {program, analyzer} = makeAnalyzer('a:b,c;b;c');
       const b = getSourceFileOrError(program, (_('/b.ts')));
       const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(analyzer.wouldCreateCycle(b, c)).toBe(false);
+      expect(analyzer.wouldCreateCycle(b, c)).toBe(null);
       analyzer.recordSyntheticImport(c, b);
-      expect(analyzer.wouldCreateCycle(b, c)).toBe(true);
+      const cycle = analyzer.wouldCreateCycle(b, c);
+      expect(cycle).toBeInstanceOf(Cycle);
+      expect(importPath(cycle!.getPath())).toEqual('b,c,b');
     });
   });
 

packages/compiler-cli/src/ngtsc/cycles/test/imports_spec.ts

@@ -10,46 +10,76 @@ import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_syst
 import {runInEachFileSystem} from '../../file_system/testing';
 import {ModuleResolver} from '../../imports';
 import {ImportGraph} from '../src/imports';
-import {makeProgramFromGraph} from './util';
+import {importPath, makeProgramFromGraph} from './util';
 
 runInEachFileSystem(() => {
-  describe('import graph', () => {
+  describe('ImportGraph', () => {
     let _: typeof absoluteFrom;
     beforeEach(() => _ = absoluteFrom);
 
-    it('should record imports of a simple program', () => {
-      const {program, graph} = makeImportGraph('a:b;b:c;c');
-      const a = getSourceFileOrError(program, (_('/a.ts')));
-      const b = getSourceFileOrError(program, (_('/b.ts')));
-      const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(importsToString(graph.importsOf(a))).toBe('b');
-      expect(importsToString(graph.importsOf(b))).toBe('c');
+    describe('importsOf()', () => {
+      it('should record imports of a simple program', () => {
+        const {program, graph} = makeImportGraph('a:b;b:c;c');
+        const a = getSourceFileOrError(program, (_('/a.ts')));
+        const b = getSourceFileOrError(program, (_('/b.ts')));
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        expect(importsToString(graph.importsOf(a))).toBe('b');
+        expect(importsToString(graph.importsOf(b))).toBe('c');
+      });
     });
 
-    it('should calculate transitive imports of a simple program', () => {
-      const {program, graph} = makeImportGraph('a:b;b:c;c');
-      const a = getSourceFileOrError(program, (_('/a.ts')));
-      const b = getSourceFileOrError(program, (_('/b.ts')));
-      const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(importsToString(graph.transitiveImportsOf(a))).toBe('a,b,c');
+    describe('transitiveImportsOf()', () => {
+      it('should calculate transitive imports of a simple program', () => {
+        const {program, graph} = makeImportGraph('a:b;b:c;c');
+        const a = getSourceFileOrError(program, (_('/a.ts')));
+        const b = getSourceFileOrError(program, (_('/b.ts')));
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        expect(importsToString(graph.transitiveImportsOf(a))).toBe('a,b,c');
+      });
+
+      it('should calculate transitive imports in a more complex program (with a cycle)', () => {
+        const {program, graph} = makeImportGraph('a:*b,*c;b:*e,*f;c:*g,*h;e:f;f;g:e;h:g');
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        expect(importsToString(graph.transitiveImportsOf(c))).toBe('c,e,f,g,h');
+      });
+
+      it('should reflect the addition of a synthetic import', () => {
+        const {program, graph} = makeImportGraph('a:b,c,d;b;c;d:b');
+        const b = getSourceFileOrError(program, (_('/b.ts')));
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        const d = getSourceFileOrError(program, (_('/d.ts')));
+        expect(importsToString(graph.importsOf(b))).toEqual('');
+        expect(importsToString(graph.transitiveImportsOf(d))).toEqual('b,d');
+        graph.addSyntheticImport(b, c);
+        expect(importsToString(graph.importsOf(b))).toEqual('c');
+        expect(importsToString(graph.transitiveImportsOf(d))).toEqual('b,c,d');
+      });
     });
 
-    it('should calculate transitive imports in a more complex program (with a cycle)', () => {
-      const {program, graph} = makeImportGraph('a:*b,*c;b:*e,*f;c:*g,*h;e:f;f;g:e;h:g');
-      const c = getSourceFileOrError(program, (_('/c.ts')));
-      expect(importsToString(graph.transitiveImportsOf(c))).toBe('c,e,f,g,h');
-    });
+    describe('findPath()', () => {
+      it('should be able to compute the path between two source files if there is a cycle', () => {
+        const {program, graph} = makeImportGraph('a:*b,*c;b:*e,*f;c:*g,*h;e:f;f;g:e;h:g');
+        const a = getSourceFileOrError(program, (_('/a.ts')));
+        const b = getSourceFileOrError(program, (_('/b.ts')));
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        const e = getSourceFileOrError(program, (_('/e.ts')));
+        expect(importPath(graph.findPath(a, a)!)).toBe('a');
+        expect(importPath(graph.findPath(a, b)!)).toBe('a,b');
+        expect(importPath(graph.findPath(c, e)!)).toBe('c,g,e');
+        expect(graph.findPath(e, c)).toBe(null);
+        expect(graph.findPath(b, c)).toBe(null);
+      });
 
-    it('should reflect the addition of a synthetic import', () => {
-      const {program, graph} = makeImportGraph('a:b,c,d;b;c;d:b');
-      const b = getSourceFileOrError(program, (_('/b.ts')));
-      const c = getSourceFileOrError(program, (_('/c.ts')));
-      const d = getSourceFileOrError(program, (_('/d.ts')));
-      expect(importsToString(graph.importsOf(b))).toEqual('');
-      expect(importsToString(graph.transitiveImportsOf(d))).toEqual('b,d');
-      graph.addSyntheticImport(b, c);
-      expect(importsToString(graph.importsOf(b))).toEqual('c');
-      expect(importsToString(graph.transitiveImportsOf(d))).toEqual('b,c,d');
+      it('should handle circular dependencies within the path between `from` and `to`', () => {
+        // a -> b -> c -> d
+        // ^----/    |
+        // ^---------/
+        const {program, graph} = makeImportGraph('a:b;b:a,c;c:a,d;d');
+        const a = getSourceFileOrError(program, (_('/a.ts')));
+        const c = getSourceFileOrError(program, (_('/c.ts')));
+        const d = getSourceFileOrError(program, (_('/d.ts')));
+        expect(importPath(graph.findPath(a, d)!)).toBe('a,b,c,d');
+      });
     });
   });
 

packages/compiler-cli/src/ngtsc/cycles/test/util.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as ts from 'typescript';
-import {PathManipulation} from '../../file_system';
+import {getFileSystem, PathManipulation} from '../../file_system';
 import {TestFile} from '../../file_system/testing';
 import {makeProgram} from '../../testing';
 
@@ -56,3 +56,8 @@ export function makeProgramFromGraph(fs: PathManipulation, graph: string): {
   });
   return makeProgram(files);
 }
+
+export function importPath(files: ts.SourceFile[]): string {
+  const fs = getFileSystem();
+  return files.map(sf => fs.basename(sf.fileName).replace('.ts', '')).join(',');
+}

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -52,6 +52,11 @@ export enum ErrorCode {
 
   SYMBOL_NOT_EXPORTED = 3001,
   SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
+  /**
+   * Raised when a relationship between directives and/or pipes would cause a cyclic import to be
+   * created that cannot be handled, such as in partial compilation mode.
+   */
+  IMPORT_CYCLE_DETECTED = 3003,
 
   CONFIG_FLAT_MODULE_NO_INDEX = 4001,
   CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
@@ -192,6 +197,7 @@ export const ERROR_DETAILS_PAGE_BASE_URL = 'https://angular.io/errors';
  */
 export const COMPILER_ERRORS_WITH_GUIDES = new Set([
   ErrorCode.DECORATOR_ARG_NOT_LITERAL,
+  ErrorCode.IMPORT_CYCLE_DETECTED,
   ErrorCode.PARAM_MISSING_TOKEN,
   ErrorCode.SCHEMA_INVALID_ELEMENT,
   ErrorCode.SCHEMA_INVALID_ATTRIBUTE,

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -4774,6 +4774,53 @@ runInEachFileSystem(os => {
         const jsContents = env.getContents('test.js');
         expect(jsContents).not.toMatch(/i\d\.ɵɵsetComponentScope\([^)]*OtherComponent[^)]*\)/);
       });
+
+      it('should detect a simple cycle and fatally error if doing partial-compilation', () => {
+        env.tsconfig({
+          compilationMode: 'partial',
+        });
+
+        env.write('test.ts', `
+        import {Component, NgModule} from '@angular/core';
+        import {NormalComponent} from './cyclic';
+
+        @Component({
+          selector: 'cyclic-component',
+          template: 'Importing this causes a cycle',
+        })
+        export class CyclicComponent {}
+
+        @NgModule({
+          declarations: [NormalComponent, CyclicComponent],
+        })
+        export class Module {}
+      `);
+
+        env.write('cyclic.ts', `
+        import {Component} from '@angular/core';
+
+        @Component({
+          selector: 'normal-component',
+          template: '<cyclic-component></cyclic-component>',
+        })
+        export class NormalComponent {}
+      `);
+
+        const diagnostics = env.driveDiagnostics();
+        expect(diagnostics.length).toEqual(1);
+        const error = diagnostics[0];
+        expect(error.code).toBe(ngErrorCode(ErrorCode.IMPORT_CYCLE_DETECTED));
+        expect(error.messageText)
+            .toEqual(
+                'One or more import cycles would need to be created to compile this component, ' +
+                'which is not supported by the current compiler configuration.');
+        const _abs = absoluteFrom;
+        expect(error.relatedInformation?.map(diag => diag.messageText)).toEqual([
+          `The component 'CyclicComponent' is used in the template ` +
+              `but importing it would create a cycle: ` +
+              `${_abs('/cyclic.ts')} -> ${_abs('/test.ts')} -> ${_abs('/cyclic.ts')}`,
+        ]);
+      });
     });
 
     describe('local refs', () => {

packages/compiler/src/render3/view/api.ts

@@ -277,6 +277,11 @@ export interface R3UsedDirectiveMetadata {
    * Name under which the directive is exported, if any (exportAs in Angular). Null otherwise.
    */
   exportAs: string[]|null;
+
+  /**
+   * If true then this directive is actually a component; otherwise it is not.
+   */
+  isComponent?: boolean;
 }
 
 /**