Skip to content

Commit

Permalink
Update tsserver when deps are updated. Dont crash on missing tsserver (
Browse files Browse the repository at this point in the history
  • Loading branch information
benjreinhart authored Jul 15, 2024
1 parent a095cdc commit c71b23c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
26 changes: 24 additions & 2 deletions packages/api/server/ws.mts
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,30 @@ async function depsInstall(payload: DepsInstallPayloadType) {
source: updatedJsonSource,
status: 'idle',
};
updateSession(session, { cells: replaceCell(session, updatedCell) }, false);
wss.broadcast(`session:${session.id}`, 'cell:updated', { cell: updatedCell });

const updatedSession = await updateSession(
session,
{ cells: replaceCell(session, updatedCell) },
false,
);

wss.broadcast(`session:${updatedSession.id}`, 'cell:updated', { cell: updatedCell });

if (updatedSession.metadata.language === 'typescript') {
const mustCreateTsServer = !tsservers.has(updatedSession.id);

// Make sure to handle the following case here:
//
// 1. User creates a new typescript Srcbook
// 3. There is no tsserver running because it relies on the typescript package in the Srcbook's node modules, which are not yet installed.
// 4. Now that we just installed the dependencies, we need to create a new tsserver instance.
const tsserver = mustCreateTsServer
? createTsServer(updatedSession)
: tsservers.get(updatedSession.id);

// Update all code cell diagnostics now that we have new packages available.
requestAllDiagnostics(tsserver, updatedSession);
}
},
}),
);
Expand Down
14 changes: 9 additions & 5 deletions packages/api/tsserver/tsservers.mts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export class TsServers {
}

// This is using the TypeScript dependency in the user's Srcbook.
//
// Note: If a user creates a typescript Srcbook, when it is first
// created, the dependencies are not installed and thus this will
// shut down immediately. Make sure that we handle this case after
// package.json has finished installing its deps.
const child = spawn('npx', ['tsserver'], {
cwd: options.cwd,
});
Expand All @@ -55,14 +60,13 @@ export class TsServers {
}

shutdown(id: string) {
const server = this.get(id);

if (!server) {
throw new Error(`tsserver for ${id} does not exist.`);
if (!this.has(id)) {
console.warn(`tsserver for ${id} does not exist. Skipping shutdown.`);
return;
}

// The server is removed from this.servers in the
// process exit handler which covers all exit cases.
return server.shutdown();
return this.get(id).shutdown();
}
}

0 comments on commit c71b23c

Please sign in to comment.