fix: prevent project from closing when only a template file is open

After opening a file, TypeScript does a cleanup step to remove/close
"orphaned" projects: https://github.com/microsoft/TypeScript/blob/a894f8ad2b0ff35b1bcfd91defba994136c68b18/src/server/editorServices.ts#L3337
As part of this, TS checks to make sure there are no open files for the
configured project: https://github.com/microsoft/TypeScript/blob/a894f8ad2b0ff35b1bcfd91defba994136c68b18/src/server/project.ts#L2333
If the only open file for the project is an HTML file, TS will not know
about it because we do not register those with the project (instead, we
associate the template file's `ScriptInfo` with the project, but do not
make the association in the other direction).
Because we do not have this two way association between the project and
the HTML `ScriptInfo`s, TypeScript closes the configured project on
actions like "go to definition" that open a `d.ts` file outside the
project. This is problematic for two particular reasons:
1. The project should not be closed in the first place. We will take a
performance hit when the user inevitably goes back to the template file
because now the project has to be re-opened.
2. The template file's `ScriptInfo` still tracks the closed project in
its `containingProjects` array. As part of the cleanup for closing a
project, TS would detach the `ScriptInfo`s from the project
(https://github.com/microsoft/TypeScript/blob/a894f8ad2b0ff35b1bcfd91defba994136c68b18/src/server/project.ts#L752)
but again, because we don't have the two-way association as mentioned
above, this does not happen. This results in a crash the next time a
request is made from the template file.

This commit addresses both of the issues above.

First, we prevent the project from being closed by creating an external project associated
with the configured project for the template file. When
TypeScript checks if a project has open refs/files, it returns `true` if
there are external project references
(https://github.com/microsoft/TypeScript/blob/a894f8ad2b0ff35b1bcfd91defba994136c68b18/src/server/project.ts#L2334).
This means that the clean up after opening a new file process as
mentioned above will not result in our project for the template being
closed.

Second, when retrieving a script info for a file we check if the project
as been closed. If it has, we detach the script info from the project,
log an error, and simply return `undefined`. To our knowledge, this
should not happen because of the first solution we implemented, but it
would prevent future crashes if there are other scenarios we missed.

Author: atscott

integration/lsp/ivy_spec.ts

@@ -8,7 +8,7 @@
 
 import {MessageConnection} from 'vscode-jsonrpc';
 import * as lsp from 'vscode-languageserver-protocol';
-
+import {URI} from 'vscode-uri';
 import {ProjectLanguageService, ProjectLanguageServiceParams} from '../../common/notifications';
 import {NgccProgress, NgccProgressToken, NgccProgressType} from '../../common/progress';
 
@@ -69,7 +69,7 @@ describe('Angular Ivy language server', () => {
     client.sendNotification(lsp.DidOpenTextDocumentNotification.type, {
       textDocument: {
         uri: `file://${FOO_TEMPLATE}`,
-        languageId: 'typescript',
+        languageId: 'html',
         version: 1,
         text: `{{ doesnotexist }}`,
       },
@@ -81,6 +81,42 @@ describe('Angular Ivy language server', () => {
     expect(diagnostics[0].message)
         .toBe(`Property 'doesnotexist' does not exist on type 'FooComponent'.`);
   });
+
+  it('does not break after opening `.d.ts` file from external template', async () => {
+    client.sendNotification(lsp.DidOpenTextDocumentNotification.type, {
+      textDocument: {
+        uri: `file://${FOO_TEMPLATE}`,
+        languageId: 'html',
+        version: 1,
+        text: `<div *ngIf="false"></div>`,
+      },
+    });
+    const languageServiceEnabled = await waitForNgcc(client);
+    expect(languageServiceEnabled).toBeTrue();
+    const response = await client.sendRequest(lsp.DefinitionRequest.type, {
+      textDocument: {
+        uri: `file://${FOO_TEMPLATE}`,
+      },
+      position: {line: 0, character: 7},
+    }) as lsp.LocationLink[];
+    // 2 results - the NgIf class and the ngIf input
+    expect(Array.isArray(response)).toBe(true);
+    const {targetUri} = response[0];
+    expect(targetUri.endsWith('angular/common/common.d.ts')).toBeTrue();
+    // Open the `.d.ts` file
+    openTextDocument(client, URI.parse(targetUri).fsPath);
+    // try a hover operation again on *ngIf
+    const hoverResponse = await client.sendRequest(lsp.HoverRequest.type, {
+      textDocument: {
+        uri: `file://${FOO_TEMPLATE}`,
+      },
+      position: {line: 0, character: 7},
+    });
+    expect(hoverResponse?.contents).toContain({
+      language: 'typescript',
+      value: 'declare (property) NgIf<boolean>.ngIf: boolean',
+    });
+  });
 });
 
 function onNgccProgress(client: MessageConnection): Promise<string> {

server/src/session.ts

@@ -44,6 +44,7 @@ export class Session {
   private readonly projectService: ts.server.ProjectService;
   private readonly logger: ts.server.Logger;
   private readonly ivy: boolean;
+  private readonly configuredProjToExternalProj = new Map<string, string>();
   private diagnosticsTimeout: NodeJS.Timeout|null = null;
   private isProjectLoading = false;
 
@@ -307,6 +308,7 @@ export class Session {
       scriptInfo.detachAllProjects();
       scriptInfo.attachToProject(project);
     }
+    this.createExternalProject(project);
 
     return project;
   }
@@ -377,13 +379,62 @@ export class Session {
     }
   }
 
+  /**
+   * Creates an external project with the same config path as `project` so that TypeScript keeps the
+   * project open when navigating away from `html` files.
+   */
+  private createExternalProject(project: ts.server.Project) {
+    if (isConfiguredProject(project) &&
+        !this.configuredProjToExternalProj.has(project.projectName)) {
+      const extProjectName = `${project.projectName}-external`;
+      project.projectService.openExternalProject({
+        projectFileName: extProjectName,
+        rootFiles: [{fileName: project.getConfigFilePath()}],
+        options: {}
+      });
+      this.configuredProjToExternalProj.set(project.projectName, extProjectName);
+    }
+  }
+
   private onDidCloseTextDocument(params: lsp.DidCloseTextDocumentParams) {
     const {textDocument} = params;
     const filePath = uriToFilePath(textDocument.uri);
     if (!filePath) {
       return;
     }
     this.projectService.closeClientFile(filePath);
+    this.maybeCloseExternalProject(filePath);
+  }
+
+  /**
+   * We open external projects for files so that we can prevent TypeScript from closing a project
+   * when there is open external HTML template that still references the project. This function
+   * checks if there are no longer any open files in the project for the given `filePath`. If there
+   * aren't, we also close the external project that was created.
+   */
+  private maybeCloseExternalProject(filePath: string) {
+    const scriptInfo = this.projectService.getScriptInfo(filePath);
+    if (!scriptInfo) {
+      return;
+    }
+    for (const [configuredProjName, externalProjName] of this.configuredProjToExternalProj) {
+      const configuredProj = this.projectService.findProject(configuredProjName);
+      if (!configuredProj || configuredProj.isClosed()) {
+        this.projectService.closeExternalProject(externalProjName);
+        this.configuredProjToExternalProj.delete(configuredProjName);
+        continue;
+      }
+      // See if any openFiles belong to the configured project.
+      // if not, close external project this.projectService.openFiles
+      const openFiles = toArray(this.projectService.openFiles.keys());
+      if (!openFiles.some(file => {
+            const scriptInfo = this.projectService.getScriptInfo(file);
+            return scriptInfo?.isAttached(configuredProj);
+          })) {
+        this.projectService.closeExternalProject(externalProjName);
+        this.configuredProjToExternalProj.delete(configuredProjName);
+      }
+    }
   }
 
   private onDidChangeTextDocument(params: lsp.DidChangeTextDocumentParams) {
@@ -519,15 +570,10 @@ export class Session {
     if (!project?.languageServiceEnabled) {
       return;
     }
-    // TODO(kyliau): For some reasons, the project could end up in an inconsistent
-    // state where the language service is undefined. It could be that the project
-    // is closed (?!). While we investigate this issue, put in a temporary fix
-    // to force the project to reload.
-    // Also note that the return type of getLanguageService() is deceiving,
-    // because it could return undefined
-    // https://github.com/microsoft/TypeScript/blob/1c1cd9b08d8bf1c77abb57d195cc6d79b1093390/src/server/project.ts#L797
-    if (project.getLanguageService() === undefined) {
-      project.markAsDirty();
+    if (project.isClosed()) {
+      scriptInfo.detachFromProject(project);
+      this.logger.info(`Failed to get language service for closed project ${project.projectName}.`);
+      return undefined;
     }
     return {
       languageService: project.getLanguageService(),
@@ -592,8 +638,8 @@ export class Session {
   private onCompletionResolve(item: lsp.CompletionItem): lsp.CompletionItem {
     const data = readNgCompletionData(item);
     if (data === null) {
-      // This item wasn't tagged with Angular LS completion data - it probably didn't originate from
-      // this language service.
+      // This item wasn't tagged with Angular LS completion data - it probably didn't originate
+      // from this language service.
       return item;
     }
 
@@ -725,3 +771,11 @@ export class Session {
     return false;
   }
 }
+
+function toArray<T>(it: ts.Iterator<T>): T[] {
+  const results: T[] = [];
+  for (let itResult = it.next(); !itResult.done; itResult = it.next()) {
+    results.push(itResult.value);
+  }
+  return results;
+}