From f48e5b34216ff7d853e80fe1406d3260226525c0 Mon Sep 17 00:00:00 2001 From: Ryan Gaus Date: Wed, 23 Oct 2024 16:00:30 -0400 Subject: [PATCH] feat: add mechanism to automatically broadcast deps:* messages whenever dependencies are installed Previously, events were sent bespoke in each implementation. Many missed events and that led to cases where the client often had no idea the state of the server. Now, always send all events always. --- packages/api/apps/app.mts | 34 +-------------------- packages/api/apps/processes.mts | 44 ++++++++++++++++++++++++++-- packages/api/server/channels/app.mts | 29 ------------------ 3 files changed, 43 insertions(+), 64 deletions(-) diff --git a/packages/api/apps/app.mts b/packages/api/apps/app.mts index edda4c2f..797045d2 100644 --- a/packages/api/apps/app.mts +++ b/packages/api/apps/app.mts @@ -4,11 +4,10 @@ import { type App as DBAppType, apps as appsTable } from '../db/schema.mjs'; import { applyPlan, createViteApp, deleteViteApp, getFlatFilesForApp } from './disk.mjs'; import { CreateAppSchemaType, CreateAppWithAiSchemaType } from './schemas.mjs'; import { asc, desc, eq } from 'drizzle-orm'; -import { deleteAppProcess, npmInstall, waitForProcessToComplete } from './processes.mjs'; +import { npmInstall, waitForProcessToComplete } from './processes.mjs'; import { generateApp } from '../ai/generate.mjs'; import { toValidPackageName } from '../apps/utils.mjs'; import { getPackagesToInstall, parsePlan } from '../ai/plan-parser.mjs'; -import { wss } from '../index.mjs'; function toSecondsSinceEpoch(date: Date): number { return Math.floor(date.getTime() / 1000); @@ -46,20 +45,6 @@ export async function createAppWithAi(data: CreateAppWithAiSchemaType): Promise< }, onExit(code) { console.log(`npm install exit code: ${code}`); - - // We must clean up this process so that we can run npm install again - deleteAppProcess(app.externalId, 'npm:install'); - - wss.broadcast(`app:${app.externalId}`, 'deps:install:status', { - status: code === 0 ? 'complete' : 'failed', - code, - }); - - if (code === 0) { - wss.broadcast(`app:${app.externalId}`, 'deps:status:response', { - nodeModulesExists: true, - }); - } }, }); @@ -84,20 +69,6 @@ export async function createAppWithAi(data: CreateAppWithAiSchemaType): Promise< }, onExit(code) { console.log(`npm install exit code: ${code}`); - - // We must clean up this process so that we can run npm install again - deleteAppProcess(app.externalId, 'npm:install'); - - wss.broadcast(`app:${app.externalId}`, 'deps:install:status', { - status: code === 0 ? 'complete' : 'failed', - code, - }); - - if (code === 0) { - wss.broadcast(`app:${app.externalId}`, 'deps:status:response', { - nodeModulesExists: true, - }); - } }, }); } @@ -124,9 +95,6 @@ export async function createApp(data: CreateAppSchemaType): Promise { }, onExit(code) { console.log(`npm install exit code: ${code}`); - - // We must clean up this process so that we can run npm install again - deleteAppProcess(app.externalId, 'npm:install'); }, }); diff --git a/packages/api/apps/processes.mts b/packages/api/apps/processes.mts index 7fb2f14b..ccb99938 100644 --- a/packages/api/apps/processes.mts +++ b/packages/api/apps/processes.mts @@ -1,6 +1,7 @@ import { ChildProcess } from 'node:child_process'; import { pathToApp } from './disk.mjs'; import { npmInstall as execNpmInstall, vite as execVite } from '../exec.mjs'; +import { wss } from '../index.mjs'; export type ProcessType = 'npm:install' | 'vite:server'; @@ -84,19 +85,58 @@ export async function waitForProcessToComplete(process: AppProcessType): Promise */ export function npmInstall( appId: string, - options: Omit[0], 'cwd'> & { onStart?: () => void }, + options: Omit[0]>, 'cwd'> & { onStart?: () => void }, ) { const runningProcess = processes.get(appId, 'npm:install'); if (runningProcess) { return runningProcess; } + wss.broadcast(`app:${appId}`, 'deps:install:status', { status: 'installing' }); if (options.onStart) { options.onStart(); } + const newlyStartedProcess: NpmInstallProcessType = { type: 'npm:install', - process: execNpmInstall({ cwd: pathToApp(appId), ...options }), + process: execNpmInstall({ + ...options, + + cwd: pathToApp(appId), + stdout: (data) => { + wss.broadcast(`app:${appId}`, 'deps:install:log', { + log: { type: 'stdout', data: data.toString('utf8') }, + }); + + if (options.stdout) { + options.stdout(data); + } + }, + stderr: (data) => { + wss.broadcast(`app:${appId}`, 'deps:install:log', { + log: { type: 'stderr', data: data.toString('utf8') }, + }); + + if (options.stderr) { + options.stderr(data); + } + }, + onExit: (code) => { + // We must clean up this process so that we can run npm install again + deleteAppProcess(appId, 'npm:install'); + + wss.broadcast(`app:${appId}`, 'deps:install:status', { + status: code === 0 ? 'complete' : 'failed', + code, + }); + + if (code === 0) { + wss.broadcast(`app:${appId}`, 'deps:status:response', { + nodeModulesExists: true, + }); + } + }, + }), }; processes.set(appId, newlyStartedProcess); diff --git a/packages/api/server/channels/app.mts b/packages/api/server/channels/app.mts index 2be228a6..22beb4cf 100644 --- a/packages/api/server/channels/app.mts +++ b/packages/api/server/channels/app.mts @@ -176,36 +176,7 @@ async function dependenciesInstall( } npmInstall(app.externalId, { - args: [], packages: payload.packages ?? undefined, - onStart: () => { - wss.broadcast(`app:${app.externalId}`, 'deps:install:status', { status: 'installing' }); - }, - stdout: (data) => { - wss.broadcast(`app:${app.externalId}`, 'deps:install:log', { - log: { type: 'stdout', data: data.toString('utf8') }, - }); - }, - stderr: (data) => { - wss.broadcast(`app:${app.externalId}`, 'deps:install:log', { - log: { type: 'stderr', data: data.toString('utf8') }, - }); - }, - onExit: (code) => { - // We must clean up this process so that we can run npm install again - deleteAppProcess(app.externalId, 'npm:install'); - - wss.broadcast(`app:${app.externalId}`, 'deps:install:status', { - status: code === 0 ? 'complete' : 'failed', - code, - }); - - if (code === 0) { - wss.broadcast(`app:${app.externalId}`, 'deps:status:response', { - nodeModulesExists: true, - }); - } - }, }); }