fix(compiler-cli): properly catch fatal diagnostics in type checking (#54309)

devversion · thePunderWoman · commit 40e1edc977fb · 2024-02-07T16:39:22.000Z
An identical addition to: 760b1f3. This commit expands the `try/catch`-es: - to properly NOT throw and just convert the diagnostic. - to be in place for all top-level instances. Notably, this logic cannot reside in the template type checker directly as otherwise we would risk multiple duplicate diagnostics. PR Close #54309
diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts
@@ -420,11 +420,28 @@ export class NgCompiler {
    * Get all Angular-related diagnostics for this compilation.
    */
   getDiagnostics(): ts.Diagnostic[] {
-    const diagnostics: ts.Diagnostic[] = [];
-    diagnostics.push(...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics());
-    if (this.options.strictTemplates) {
-      diagnostics.push(...this.getExtendedTemplateDiagnostics());
+    const diagnostics: ts.Diagnostic[] = [
+      ...this.getNonTemplateDiagnostics(),
+    ];
+
+    // Type check code may throw fatal diagnostic errors if e.g. the type check
+    // block cannot be generated. Gracefully return the associated diagnostic.
+    // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics
+    // by running the extended template checking code, which will attempt to
+    // generate the same TCB.
+    try {
+      diagnostics.push(...this.getTemplateDiagnostics());
+
+      if (this.options.strictTemplates) {
+        diagnostics.push(...this.getExtendedTemplateDiagnostics());
+      }
+    } catch (err: unknown) {
+      if (!(err instanceof FatalDiagnosticError)) {
+        throw err;
+      }
+      diagnostics.push(err.toDiagnostic());
     }
+
     return this.addMessageTextDetails(diagnostics);
   }
 
@@ -437,21 +454,22 @@ export class NgCompiler {
     const diagnostics: ts.Diagnostic[] =
         [...this.getNonTemplateDiagnostics().filter(diag => diag.file === file)];
 
+    // Type check code may throw fatal diagnostic errors if e.g. the type check
+    // block cannot be generated. Gracefully return the associated diagnostic.
+    // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics
+    // by running the extended template checking code, which will attempt to
+    // generate the same TCB.
     try {
       diagnostics.push(...this.getTemplateDiagnosticsForFile(file, optimizeFor));
+
       if (this.options.strictTemplates) {
         diagnostics.push(...this.getExtendedTemplateDiagnostics(file));
       }
-    } catch (e) {
-      // Type check code may throw fatal diagnostic errors if e.g. the type check
-      // block cannot be generated. Gracefully return the associated diagnostic.
-      // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics
-      // by running the extended template checking code, which will attempt to
-      // generate the same TCB.
-      if (e instanceof FatalDiagnosticError) {
-        diagnostics.push(e.toDiagnostic());
+    } catch (err: unknown) {
+      if (!(err instanceof FatalDiagnosticError)) {
+        throw err;
       }
-      throw e;
+      diagnostics.push(err.toDiagnostic());
     }
 
     return this.addMessageTextDetails(diagnostics);
@@ -464,6 +482,12 @@ export class NgCompiler {
     const compilation = this.ensureAnalyzed();
     const ttc = compilation.templateTypeChecker;
     const diagnostics: ts.Diagnostic[] = [];
+
+    // Type check code may throw fatal diagnostic errors if e.g. the type check
+    // block cannot be generated. Gracefully return the associated diagnostic.
+    // Note: If a fatal diagnostic is raised, do not repeat the same diagnostics
+    // by running the extended template checking code, which will attempt to
+    // generate the same TCB.
     try {
       diagnostics.push(...ttc.getDiagnosticsForComponent(component));
 
@@ -876,23 +900,16 @@ export class NgCompiler {
 
   private getTemplateDiagnostics(): ReadonlyArray<ts.Diagnostic> {
     const compilation = this.ensureAnalyzed();
-
-    // Get the diagnostics.
     const diagnostics: ts.Diagnostic[] = [];
+
+    // Get diagnostics for all files.
     for (const sf of this.inputProgram.getSourceFiles()) {
       if (sf.isDeclarationFile || this.adapter.isShim(sf)) {
         continue;
       }
 
-      try {
-        diagnostics.push(
-            ...compilation.templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram));
-      } catch (err) {
-        if (!(err instanceof FatalDiagnosticError)) {
-          throw err;
-        }
-        diagnostics.push(err.toDiagnostic());
-      }
+      diagnostics.push(
+          ...compilation.templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram));
     }
 
     const program = this.programDriver.getProgram();
@@ -1018,8 +1035,8 @@ export class NgCompiler {
       ]);
 
       // If an entrypoint is present, then all user imports should be directed through the
-      // entrypoint and private exports are not needed. The compiler will validate that all publicly
-      // visible directives/pipes are importable via this entrypoint.
+      // entrypoint and private exports are not needed. The compiler will validate that all
+      // publicly visible directives/pipes are importable via this entrypoint.
       if (this.entryPoint === null && this.options.generateDeepReexports === true) {
         // No entrypoint is present and deep re-exports were requested, so configure the aliasing
         // system to generate them.
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -69,6 +69,34 @@ export declare class AnimationEvent {
       expect(getSourceCodeForDiagnostic(diags[0])).toBe('does_not_exist');
     });
 
+    it('should not fail with a runtime error when generating TCB', () => {
+      env.tsconfig({strictTemplates: true});
+      env.write('test.ts', `
+        import {Component, input} from '@angular/core';
+
+        @Component({
+          selector: 'sub-cmp',
+          standalone: true,
+          template: '',
+        })
+        class Sub { // intentionally not exported
+          someInput = input.required<string>();
+        }
+
+        @Component({
+          template: \`<sub-cmp [someInput]="''" />\`,
+          standalone: true,
+          imports: [Sub],
+        })
+        export class MyComponent {}
+      `);
+
+      const diagnostics = env.driveDiagnostics();
+      expect(diagnostics).toEqual([jasmine.objectContaining(
+          {messageText: jasmine.objectContaining({messageText: 'Unable to import symbol Sub.'})},
+          )]);
+    });
+
     it('should check regular attributes that are directive inputs', () => {
       env.tsconfig(
           {fullTemplateTypeCheck: true, strictInputTypes: true, strictAttributeTypes: true});
