Use tsserver geterr command & events. reloadProjects for file renames (…

…#132)

packages/api/server/ws.mts

@@ -43,7 +43,7 @@ import {
 import tsservers from '../tsservers.mjs';
 import { TsServer } from '../tsserver/tsserver.mjs';
 import WebSocketServer from './ws-client.mjs';
-import { pathToCodeFile } from '../srcbook/path.mjs';
+import { filenameFromPath, pathToCodeFile } from '../srcbook/path.mjs';
 import { normalizeDiagnostic } from '../tsserver/utils.mjs';
 import { removeCodeCellFromDisk } from '../srcbook/index.mjs';
 
@@ -312,12 +312,29 @@ async function cellUpdate(payload: CellUpdatePayloadType) {
     tsserver.close({ file: oldFilePath });
     tsserver.open({ file: newFilePath, fileContent: cellAfterUpdate.source });
 
-    // Send all diagnostics so that other cells that import this updated cell
-    // are informed about any updates (changes to exports, file renames).
-    //
-    // TODO: Can we be smarter and only do this for cells that import
-    // this cell, rather than all cells?
-    sendAllTypeScriptDiagnostics(tsserver, session);
+    // TODO: Given the amount of differences here and elsewhere when renaming cells,
+    // it's probably worth it at this point to make those separate websocket events.
+    if (oldFilePath !== newFilePath) {
+      // Tsserver can get into a bad state if we don't reload the project after renaming a file.
+      // This consistently happens under the following condition:
+      //
+      // 1. Rename a `a.ts` that is imported by `b.ts` to `c.ts`
+      // 2. Semantic diagnostics report an error in `b.ts` that `a.ts` doesn't exist
+      // 3. Great, all works so far.
+      // 4. Rename `c.ts` back to `a.ts`.
+      // 5. Semantic diagnostics still report an error in `b.ts` that `a.ts` doesn't exist.
+      // 6. This is wrong, `a.ts` does exist.
+      //
+      // If we reload the project, this issue resolves itself.
+      //
+      // NOTE: reloading the project sends diagnostic events without calling `geterr`.
+      // However, it seems to take a while for the diagnostics to be sent, so we still
+      // request it below.
+      //
+      tsserver.reloadProjects();
+    }
+
+    requestAllDiagnostics(tsserver, session);
   }
 }
 
@@ -351,44 +368,60 @@ async function cellDelete(payload: CellDeletePayloadType) {
       const file = pathToCodeFile(updatedSession.dir, cell.filename);
       const tsserver = tsservers.get(updatedSession.id);
       tsserver.close({ file });
-      sendAllTypeScriptDiagnostics(tsserver, updatedSession);
+      requestAllDiagnostics(tsserver, updatedSession);
     }
   }
 }
 
 /**
- * Send semantic diagnostics for a TypeScript cell to the client.
+ * Request async diagnostics for all files in the project.
  */
-async function sendTypeScriptDiagnostics(
-  tsserver: TsServer,
-  session: SessionType,
-  cell: CodeCellType,
-) {
-  const response = await tsserver.semanticDiagnosticsSync({
-    file: pathToCodeFile(session.dir, cell.filename),
-  });
+function requestAllDiagnostics(tsserver: TsServer, session: SessionType, delay = 0) {
+  const codeCells = session.cells.filter((cell) => cell.type === 'code') as CodeCellType[];
+  const files = codeCells.map((cell) => pathToCodeFile(session.dir, cell.filename));
+  tsserver.geterr({ files, delay });
+}
 
-  if (!response.success) {
-    console.warn(`Failed to get diagnostics for cell ${cell.id}: ${response.message}`);
-    return;
-  }
+function createTsServer(session: SessionType) {
+  const tsserver = tsservers.create(session.id, { cwd: session.dir });
+
+  const sessionId = session.id;
+
+  tsserver.onSemanticDiag(async (event) => {
+    const eventBody = event.body;
+
+    // Get most recent session state
+    const session = await findSession(sessionId);
+
+    if (!eventBody || !session) {
+      return;
+    }
 
-  // The client will always reset diagnostics when the server sends them.
-  // Therefore, it is important to send diagnostics even when the list is
-  // empty because the client will not clear stale diagnostics otherwise.
-  const diagnostics = response.body || [];
-  wss.broadcast(`session:${session.id}`, 'tsserver:cell:diagnostics', {
-    cellId: cell.id,
-    diagnostics: diagnostics.map(normalizeDiagnostic),
+    const filename = filenameFromPath(eventBody.file);
+    const cells = session.cells.filter((cell) => cell.type === 'code') as CodeCellType[];
+    const cell = cells.find((c) => c.filename === filename);
+
+    if (!cell) {
+      return;
+    }
+
+    wss.broadcast(`session:${session.id}`, 'tsserver:cell:diagnostics', {
+      cellId: cell.id,
+      diagnostics: eventBody.diagnostics.map(normalizeDiagnostic),
+    });
   });
-}
 
-function sendAllTypeScriptDiagnostics(tsserver: TsServer, session: SessionType) {
+  // Open all code cells in tsserver
   for (const cell of session.cells) {
     if (cell.type === 'code') {
-      sendTypeScriptDiagnostics(tsserver, session, cell);
+      tsserver.open({
+        file: pathToCodeFile(session.dir, cell.filename),
+        fileContent: cell.source,
+      });
     }
   }
+
+  return tsserver;
 }
 
 async function tsserverStart(payload: TsServerStartPayloadType) {
@@ -402,21 +435,10 @@ async function tsserverStart(payload: TsServerStartPayloadType) {
     throw new Error(`tsserver can only be used with TypeScript Srcbooks.`);
   }
 
-  if (!tsservers.has(session.id)) {
-    const tsserver = tsservers.create(session.id, { cwd: session.dir });
-
-    // Open all code cells in tsserver
-    for (const cell of session.cells) {
-      if (cell.type === 'code') {
-        tsserver.open({
-          file: pathToCodeFile(session.dir, cell.filename),
-          fileContent: cell.source,
-        });
-      }
-    }
-  }
-
-  sendAllTypeScriptDiagnostics(tsservers.get(session.id), session);
+  requestAllDiagnostics(
+    tsservers.has(session.id) ? tsservers.get(session.id) : createTsServer(session),
+    session,
+  );
 }
 
 async function tsserverStop(payload: TsServerStopPayloadType) {

packages/api/srcbook/path.mts

@@ -20,3 +20,7 @@ export function pathToTsconfigJson(baseDir: string) {
 export function pathToCodeFile(baseDir: string, filename: string) {
   return Path.join(baseDir, 'src', filename);
 }
+
+export function filenameFromPath(filePath: string) {
+  return Path.basename(filePath);
+}

packages/api/tsserver/tsserver.mts

@@ -1,14 +1,44 @@
-import { parseTsServerMessages } from './utils.mjs';
-import type { ChildProcess } from 'child_process';
+import EventEmitter from 'node:events';
+import type { ChildProcess } from 'node:child_process';
 import type { server as tsserver } from 'typescript';
+import { parseTsServerMessages } from './utils.mjs';
 
-export class TsServer {
+/**
+ * This class provides a wrapper around a process running tsserver and is used to communicate
+ * with the server, mainly to support diagnostics for user code (type errors, sytnax errors,
+ * type definitions, etc).
+ *
+ * tsserver is not documented. Here is a brief overview.
+ *
+ * tsserver is a process which listens for messages over stdin and
+ * sends messages over stdout. tsserver has three types of messages:
+ *
+ * 1. Request: A request from the client to the server.
+ * 2. Response: A response from the server to a specific client request.
+ * 3. Event: An event from the server to the client.
+ *
+ * Request and responses are identified a unique number called `seq`. `seq` is incremented
+ * for each request the client sends. The client will send a `seq` field with its request
+ * and the server will provide a `request_seq` in its response which is used to tie a message
+ * from the server to a specific request from the client.
+ *
+ * Events can arrive at any time but are often used as an asynchronous response from the server.
+ * For example, syntax and semantic diagnostics are sent as events when using the `geterr` command.
+ *
+ * Most of this is learned by reading through the source (protocol.ts) as well as trial
+ * and error. They also have an introduction, but it's hardly useful. See links below.
+ *
+ * - https://github.com/microsoft/TypeScript/blob/v5.5.3/src/server/protocol.ts
+ * - https://github.com/microsoft/TypeScript/wiki/Standalone-Server-(tsserver)
+ */
+export class TsServer extends EventEmitter {
   private _seq: number = 0;
   private buffered: Buffer = Buffer.from('');
   private readonly process: ChildProcess;
   private readonly resolvers: Record<number, (value: any) => void> = {};
 
   constructor(process: ChildProcess) {
+    super();
     this.process = process;
     this.process.stdout?.on('data', (chunk) => {
       const { messages, buffered } = parseTsServerMessages(chunk, this.buffered);
@@ -32,7 +62,7 @@ export class TsServer {
 
     if (!resolve) {
       console.warn(
-        `Received a response for command '${response.command}' and request_seq '${response.request_seq}' but no resolver was found. This may be a bug in the code.`,
+        `Received a response for command '${response.command}' and request_seq '${response.request_seq}' but no resolver was found. This may be a bug in the code.\n\nResponse:\n${JSON.stringify(response, null, 2)}\n`,
       );
 
       return;
@@ -43,8 +73,8 @@ export class TsServer {
     resolve(response);
   }
 
-  private handleEvent(_event: tsserver.protocol.Event) {
-    // Ignoring telemetry events for now
+  private handleEvent(event: tsserver.protocol.Event) {
+    this.emit(event.event, event);
   }
 
   private send(request: tsserver.protocol.Request) {
@@ -58,10 +88,32 @@ export class TsServer {
     });
   }
 
+  /**
+   * Wrapper around the `semanticDiag` event for convenience and type safety.
+   */
+  onSemanticDiag(callback: (event: tsserver.protocol.DiagnosticEvent) => void) {
+    this.on('semanticDiag', callback);
+  }
+
+  /**
+   * Wrapper around the `syntaxDiag` event for convenience and type safety.
+   */
+  onSyntaxDiag(callback: (event: tsserver.protocol.DiagnosticEvent) => void) {
+    this.on('syntaxDiag', callback);
+  }
+
+  /**
+   * Wrapper around the `suggestionDiag` event for convenience and type safety.
+   */
+  onSuggestionDiag(callback: (event: tsserver.protocol.DiagnosticEvent) => void) {
+    this.on('suggestionDiag', callback);
+  }
+
   /**
    * Shutdown the underlying tsserver process.
    */
   shutdown() {
+    this.removeAllListeners();
     return this.process.kill('SIGTERM');
   }
 
@@ -94,57 +146,60 @@ export class TsServer {
   }
 
   /**
-   * Get info about the project.
+   * Ask tsserver to send diagnostics for a set of files.
    *
-   * This can be useful during development to inspect the tsserver integration.
+   * This is used to get the errors for a set of files in a project.
+   *
+   * Note that the diagnostics are sent as asynchronous events instead of responding to this request.
    */
-  projectInfo(args: tsserver.protocol.ProjectInfoRequestArgs) {
-    return this.sendWithResponsePromise<tsserver.protocol.ProjectInfoResponse>({
+  geterr(args: tsserver.protocol.GeterrRequestArgs) {
+    this.send({
       seq: this.seq,
       type: 'request',
-      command: 'projectInfo',
+      command: 'geterr',
       arguments: args,
     });
   }
 
   /**
-   * Get info about a term at a specific location in a file.
+   * Reload the project in tsserver.
    *
-   * This is used for type definitions and documentation lookups on hover.
+   * This is used to tell tsserver to reload the project configuration
+   * which helps ensure that the project is up-to-date. This helps resolve
+   * errors that can occur when renaming files.
    */
-  quickinfo(args: tsserver.protocol.FileLocationRequestArgs) {
-    return this.sendWithResponsePromise<tsserver.protocol.QuickInfoResponse>({
+  reloadProjects() {
+    this.send({
       seq: this.seq,
       type: 'request',
-      command: 'quickinfo',
-      arguments: args,
+      command: 'reloadProjects',
     });
   }
 
   /**
-   * Get semantic information about a file.
+   * Get info about the project.
    *
-   * This is used to report type errors in a file.
+   * This can be useful during development to inspect the tsserver integration.
    */
-  semanticDiagnosticsSync(args: tsserver.protocol.SemanticDiagnosticsSyncRequestArgs) {
-    return this.sendWithResponsePromise<tsserver.protocol.SemanticDiagnosticsSyncResponse>({
+  projectInfo(args: tsserver.protocol.ProjectInfoRequestArgs) {
+    return this.sendWithResponsePromise<tsserver.protocol.ProjectInfoResponse>({
       seq: this.seq,
       type: 'request',
-      command: 'semanticDiagnosticsSync',
+      command: 'projectInfo',
       arguments: args,
     });
   }
 
   /**
-   * Get syntactic information about a file.
+   * Get info about a term at a specific location in a file.
    *
-   * This is used to report syntax errors in a file.
+   * This is used for type definitions and documentation lookups on hover.
    */
-  syntacticDiagnosticsSync(args: tsserver.protocol.SyntacticDiagnosticsSyncRequestArgs) {
-    return this.sendWithResponsePromise<tsserver.protocol.SyntacticDiagnosticsSyncResponse>({
+  quickinfo(args: tsserver.protocol.FileLocationRequestArgs) {
+    return this.sendWithResponsePromise<tsserver.protocol.QuickInfoResponse>({
       seq: this.seq,
       type: 'request',
-      command: 'syntacticDiagnosticsSync',
+      command: 'quickinfo',
       arguments: args,
     });
   }

packages/api/tsserver/utils.mts

@@ -68,7 +68,7 @@ export function normalizeDiagnostic(
     };
   } else {
     return {
-      // From what I can tell, code should always be present depsite the type.
+      // From what I can tell, code should always be present despite the type.
       // If it's not, we use 1000 as the 'unknown' error code, which is not a
       // code defined in diagnosticMessages.json in TypeScript's source.
       code: diagnostic.code || 1000,