fix(language-service): Add resource files as roots to their associated projects (angular#45601)

When an external template is read, adds the template file to to the project which contains.
This is necessary to keep the projects open when navigating away from HTML files.
Since a `tsconfig` cannot express including non-TS files,
we need another way to indicate the template files are considered part of the project.

Note that this does not ensure that the project in question _directly_ contains the component
file. That is, the project might just include the component file through the program rather
than directly in the `include` glob of the `tsconfig`. This distinction is somewhat important
because the TypeScript language service/server prefers projects which _directly_ contain the TS
file (see `projectContainsInfoDirectly` in the TS codebase). What this means it that there can
possibly be a different project used between the TS and HTML files.

For example, in Nx projects, the referenced configs are `tsconfig.app.json` and
`tsconfig.editor.json`. `tsconfig.app.json` comes first in the base `tsconfig.json` and
contains the entry point of the app. `tsconfig.editor.json` contains the `**.ts` glob of all TS
files. This means that `tsconfig.editor.json` will be preferred by the TS server for TS files
but the `tsconfig.app.json` will be used for HTML files since it comes first and we cannot
effectively express `projectContainsInfoDirectly` for HTML files.

We could consider also updating the language server implementation to attempt
to select the project to use for the template file based on which project
contains its component file directly, using either the internal `project.projectContainsInfoDirectly`
or as a workaround, check `project.isRoot(componentTsFile)`.

Finally, keeping the projects open is hugely important in the solution style config case like
Nx. When a TS file is opened, TypeScript will only retain `tsconfig.editor.json` and not
`tsconfig.app.json`. However, if our extension does not also know to select
`tsconfig.editor.json`, it will automatically select `tsconfig.app.json` since it is defined
first in the `tsconfig.json` file. So we need to teach TS server that we are (1) interested in
keeping projects open when there is an HTML file open and (2) optionally attempt to do this
_only_ for projects that we know the TS language service will prioritize in TS files (i.e.,
attempt to only keep `tsconfig.editor.json` open and allow `tsconfig.app.json` to close)
and prioritize that project for all requests.

fixes angular/vscode-ng-language-service#1623
fixes angular/vscode-ng-language-service#876

PR Close angular#45601

Author: atscott

packages/compiler-cli/ngcc/BUILD.bazel

@@ -28,6 +28,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/perf",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/scope",
+        "//packages/compiler-cli/src/ngtsc/shims",
         "//packages/compiler-cli/src/ngtsc/sourcemaps",
         "//packages/compiler-cli/src/ngtsc/transform",
         "//packages/compiler-cli/src/ngtsc/translator",

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

@@ -11,6 +11,7 @@ import {IncrementalBuild} from '../../../src/ngtsc/incremental/api';
 import {SemanticSymbol} from '../../../src/ngtsc/incremental/semantic_graph';
 import {NOOP_PERF_RECORDER} from '../../../src/ngtsc/perf';
 import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
+import {isShim} from '../../../src/ngtsc/shims';
 import {CompilationMode, DecoratorHandler, DtsTransformRegistry, HandlerFlags, Trait, TraitCompiler} from '../../../src/ngtsc/transform';
 import {NgccReflectionHost} from '../host/ngcc_host';
 import {isDefined} from '../utils';
@@ -28,7 +29,7 @@ export class NgccTraitCompiler extends TraitCompiler {
     super(
         handlers, ngccReflector, NOOP_PERF_RECORDER, new NoIncrementalBuild(),
         /* compileNonExportedClasses */ true, CompilationMode.FULL, new DtsTransformRegistry(),
-        /* semanticDepGraphUpdater */ null);
+        /* semanticDepGraphUpdater */ null, {isShim, isResource: () => false});
   }
 
   get analyzedFiles(): ts.SourceFile[] {

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

@@ -43,7 +43,8 @@ export interface NgCompilerAdapter extends
     // incompatible with the `ts.CompilerHost` version which isn't. The combination of these two
     // still satisfies `ts.ModuleResolutionHost`.
         Omit<ts.ModuleResolutionHost, 'getCurrentDirectory'>,
-    Pick<ExtendedTsCompilerHost, 'getCurrentDirectory'|ExtendedCompilerHostMethods> {
+    Pick<ExtendedTsCompilerHost, 'getCurrentDirectory'|ExtendedCompilerHostMethods>,
+    SourceFileTypeIdentifier {
   /**
    * A path to a single file which represents the entrypoint of an Angular Package Format library,
    * if the current program is one.
@@ -86,7 +87,9 @@ export interface NgCompilerAdapter extends
    * Resolved list of root directories explicitly set in, or inferred from, the tsconfig.
    */
   readonly rootDirs: ReadonlyArray<AbsoluteFsPath>;
+}
 
+export interface SourceFileTypeIdentifier {
   /**
    * Distinguishes between shim files added by Angular to the compilation process (both those
    * intended for output, like ngfactory files, as well as internal shims like ngtypecheck files)
@@ -96,4 +99,14 @@ export interface NgCompilerAdapter extends
    * `true` if a file was written by the user, and `false` if a file was added by the compiler.
    */
   isShim(sf: ts.SourceFile): boolean;
+
+  /**
+   * Distinguishes between resource files added by Angular to the project and original files in the
+   * user's program.
+   *
+   * This is necessary only for the language service because it adds resource files as root files
+   * when they are read. This is done to indicate to TS Server that these resources are part of the
+   * project and ensures that projects are retained properly when navigating around the workspace.
+   */
+  isResource(sf: ts.SourceFile): boolean;
 }

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

@@ -1050,7 +1050,7 @@ export class NgCompiler {
     const traitCompiler = new TraitCompiler(
         handlers, reflector, this.delegatingPerfRecorder, this.incrementalCompilation,
         this.options.compileNonExportedClasses !== false, compilationMode, dtsTransforms,
-        semanticDepGraphUpdater);
+        semanticDepGraphUpdater, this.adapter);
 
     // Template type-checking may use the `ProgramDriver` to produce new `ts.Program`(s). If this
     // happens, they need to be tracked by the `NgCompiler`.

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

@@ -234,6 +234,16 @@ export class NgCompilerHost extends DelegatingCompilerHost implements
     return isShim(sf);
   }
 
+  /**
+   * Check whether the given `ts.SourceFile` is a resource file.
+   *
+   * This simply returns `false` for the compiler-cli since resource files are not added as root
+   * files to the project.
+   */
+  isResource(sf: ts.SourceFile): boolean {
+    return false;
+  }
+
   getSourceFile(
       fileName: string, languageVersion: ts.ScriptTarget,
       onError?: ((message: string) => void)|undefined,

packages/compiler-cli/src/ngtsc/transform/src/compilation.ts

@@ -9,16 +9,16 @@
 import {ConstantPool} from '@angular/compiler';
 import ts from 'typescript';
 
+import {SourceFileTypeIdentifier} from '../../core/api';
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
 import {IncrementalBuild} from '../../incremental/api';
 import {SemanticDepGraphUpdater, SemanticSymbol} from '../../incremental/semantic_graph';
 import {IndexingContext} from '../../indexer';
 import {PerfEvent, PerfRecorder} from '../../perf';
 import {ClassDeclaration, DeclarationNode, Decorator, isNamedClassDeclaration, ReflectionHost} from '../../reflection';
-import {isShim} from '../../shims';
 import {ProgramTypeCheckAdapter, TypeCheckContext} from '../../typecheck/api';
 import {ExtendedTemplateChecker} from '../../typecheck/extended/api';
-import {getSourceFile, isExported} from '../../util/src/typescript';
+import {getSourceFile} from '../../util/src/typescript';
 import {Xi18nContext} from '../../xi18n';
 
 import {AnalysisOutput, CompilationMode, CompileResult, DecoratorHandler, HandlerFlags, HandlerPrecedence, ResolveResult} from './api';
@@ -97,11 +97,15 @@ export class TraitCompiler implements ProgramTypeCheckAdapter {
 
   constructor(
       private handlers: DecoratorHandler<unknown, unknown, SemanticSymbol|null, unknown>[],
-      private reflector: ReflectionHost, private perf: PerfRecorder,
+      private reflector: ReflectionHost,
+      private perf: PerfRecorder,
       private incrementalBuild: IncrementalBuild<ClassRecord, unknown>,
-      private compileNonExportedClasses: boolean, private compilationMode: CompilationMode,
+      private compileNonExportedClasses: boolean,
+      private compilationMode: CompilationMode,
       private dtsTransforms: DtsTransformRegistry,
-      private semanticDepGraphUpdater: SemanticDepGraphUpdater|null) {
+      private semanticDepGraphUpdater: SemanticDepGraphUpdater|null,
+      private sourceFileTypeIdentifier: SourceFileTypeIdentifier,
+  ) {
     for (const handler of handlers) {
       this.handlersByName.set(handler.name, handler);
     }
@@ -118,8 +122,9 @@ export class TraitCompiler implements ProgramTypeCheckAdapter {
   private analyze(sf: ts.SourceFile, preanalyze: false): void;
   private analyze(sf: ts.SourceFile, preanalyze: true): Promise<void>|undefined;
   private analyze(sf: ts.SourceFile, preanalyze: boolean): Promise<void>|undefined {
-    // We shouldn't analyze declaration files.
-    if (sf.isDeclarationFile || isShim(sf)) {
+    // We shouldn't analyze declaration, shim, or resource files.
+    if (sf.isDeclarationFile || this.sourceFileTypeIdentifier.isShim(sf) ||
+        this.sourceFileTypeIdentifier.isResource(sf)) {
       return undefined;
     }
 

packages/compiler-cli/src/ngtsc/transform/test/compilation_spec.ts

@@ -17,6 +17,11 @@ import {getDeclaration, makeProgram} from '../../testing';
 import {CompilationMode, DetectResult, DtsTransformRegistry, TraitCompiler} from '../../transform';
 import {AnalysisOutput, CompileResult, DecoratorHandler, HandlerPrecedence} from '../src/api';
 
+const fakeSfTypeIdentifier = {
+  isShim: () => false,
+  isResource: () => false
+};
+
 runInEachFileSystem(() => {
   describe('TraitCompiler', () => {
     let _: typeof absoluteFrom;
@@ -49,7 +54,7 @@ runInEachFileSystem(() => {
       const reflectionHost = new TypeScriptReflectionHost(checker);
       const compiler = new TraitCompiler(
           [new FakeDecoratorHandler()], reflectionHost, NOOP_PERF_RECORDER, NOOP_INCREMENTAL_BUILD,
-          true, CompilationMode.FULL, new DtsTransformRegistry(), null);
+          true, CompilationMode.FULL, new DtsTransformRegistry(), null, fakeSfTypeIdentifier);
       const sourceFile = program.getSourceFile('lib.d.ts')!;
       const analysis = compiler.analyzeSync(sourceFile);
 
@@ -138,7 +143,7 @@ runInEachFileSystem(() => {
         const compiler = new TraitCompiler(
             [new PartialDecoratorHandler(), new FullDecoratorHandler()], reflectionHost,
             NOOP_PERF_RECORDER, NOOP_INCREMENTAL_BUILD, true, CompilationMode.PARTIAL,
-            new DtsTransformRegistry(), null);
+            new DtsTransformRegistry(), null, fakeSfTypeIdentifier);
         const sourceFile = program.getSourceFile('test.ts')!;
         compiler.analyzeSync(sourceFile);
         compiler.resolve();
@@ -168,7 +173,7 @@ runInEachFileSystem(() => {
         const compiler = new TraitCompiler(
             [new PartialDecoratorHandler(), new FullDecoratorHandler()], reflectionHost,
             NOOP_PERF_RECORDER, NOOP_INCREMENTAL_BUILD, true, CompilationMode.FULL,
-            new DtsTransformRegistry(), null);
+            new DtsTransformRegistry(), null, fakeSfTypeIdentifier);
         const sourceFile = program.getSourceFile('test.ts')!;
         compiler.analyzeSync(sourceFile);
         compiler.resolve();

packages/language-service/src/adapters.ts

@@ -61,6 +61,11 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
     return isShim(sf);
   }
 
+  isResource(sf: ts.SourceFile): boolean {
+    const scriptInfo = this.project.getScriptInfo(sf.fileName);
+    return scriptInfo?.scriptKind === ts.ScriptKind.Unknown;
+  }
+
   fileExists(fileName: string): boolean {
     return this.project.fileExists(fileName);
   }
@@ -100,14 +105,21 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
     // getScriptInfo() will not create one if it does not exist.
     // In this case, we *want* a script info to be created so that we could
     // keep track of its version.
-    const snapshot = this.project.getScriptSnapshot(fileName);
-    if (!snapshot) {
-      // This would fail if the file does not exist, or readFile() fails for
-      // whatever reasons.
-      throw new Error(`Failed to get script snapshot while trying to read ${fileName}`);
-    }
     const version = this.project.getScriptVersion(fileName);
     this.lastReadResourceVersion.set(fileName, version);
+    const scriptInfo = this.project.getScriptInfo(fileName);
+    if (!scriptInfo) {
+      // // This should not happen because it would have failed already at `getScriptVersion`.
+      throw new Error(`Failed to get script info when trying to read ${fileName}`);
+    }
+    // Add external resources as root files to the project since we project language service
+    // features for them (this is currently only the case for HTML files, but we could investigate
+    // css file features in the future). This prevents the project from being closed when navigating
+    // away from a resource file.
+    if (!this.project.isRoot(scriptInfo)) {
+      this.project.addRoot(scriptInfo);
+    }
+    const snapshot = scriptInfo.getSnapshot();
     return snapshot.getText(0, snapshot.getLength());
   }
 

packages/language-service/testing/src/project.ts

@@ -153,7 +153,8 @@ export class Project {
     const ngCompiler = this.ngLS.compilerFactory.getOrCreate();
 
     for (const sf of program.getSourceFiles()) {
-      if (sf.isDeclarationFile || sf.fileName.endsWith('.ngtypecheck.ts')) {
+      if (sf.isDeclarationFile || sf.fileName.endsWith('.ngtypecheck.ts') ||
+          !sf.fileName.endsWith('.ts')) {
         continue;
       }