From b66e81c6953777c5cf72d5899ff1115917c8fa2e Mon Sep 17 00:00:00 2001 From: Ryan Gaus Date: Tue, 22 Oct 2024 16:56:28 -0400 Subject: [PATCH 1/4] feat: add socket broadcasts for all npm installs globally in the app --- packages/api/apps/app.mts | 79 ++++++++++++++-------------- packages/api/server/channels/app.mts | 32 ++--------- 2 files changed, 43 insertions(+), 68 deletions(-) diff --git a/packages/api/apps/app.mts b/packages/api/apps/app.mts index 626bda0c..62612af8 100644 --- a/packages/api/apps/app.mts +++ b/packages/api/apps/app.mts @@ -8,6 +8,7 @@ import { npmInstall } from '../exec.mjs'; import { generateApp } from '../ai/generate.mjs'; import { toValidPackageName } from '../apps/utils.mjs'; import { parsePlan } from '../ai/plan-parser.mjs'; +import { wss as webSocketServer } from '../index.mjs'; function toSecondsSinceEpoch(date: Date): number { return Math.floor(date.getTime() / 1000); @@ -36,18 +37,7 @@ export async function createAppWithAi(data: CreateAppWithAiSchemaType): Promise< await createViteApp(app); // Note: we don't surface issues or retries and this is "running in the background". // In this case it works in our favor because we'll kickoff generation while it happens - npmInstall({ - cwd: pathToApp(app.externalId), - stdout(data) { - console.log(data.toString('utf8')); - }, - stderr(data) { - console.error(data.toString('utf8')); - }, - onExit(code) { - console.log(`npm install exit code: ${code}`); - }, - }); + installAppDependencies(app); const files = await getFlatFilesForApp(app.externalId); const result = await generateApp(toValidPackageName(app.name), files, data.prompt); @@ -55,18 +45,7 @@ export async function createAppWithAi(data: CreateAppWithAiSchemaType): Promise< await applyPlan(app, plan); // Run npm install again since we don't have a good way of parsing the plan to know if we should... - npmInstall({ - cwd: pathToApp(app.externalId), - stdout(data) { - console.log(data.toString('utf8')); - }, - stderr(data) { - console.error(data.toString('utf8')); - }, - onExit(code) { - console.log(`npm install exit code: ${code}`); - }, - }); + installAppDependencies(app); return app; } @@ -78,21 +57,7 @@ export async function createApp(data: CreateAppSchemaType): Promise { await createViteApp(app); - // TODO: handle this better. - // This should be done somewhere else and surface issues or retries. - // Not awaiting here because it's "happening in the background". - npmInstall({ - cwd: pathToApp(app.externalId), - stdout(data) { - console.log(data.toString('utf8')); - }, - stderr(data) { - console.error(data.toString('utf8')); - }, - onExit(code) { - console.log(`npm install exit code: ${code}`); - }, - }); + installAppDependencies(app); return app; } @@ -120,3 +85,39 @@ export async function updateApp(id: string, attrs: { name: string }) { .returning(); return updatedApp; } + +export async function installAppDependencies(app: DBAppType, packages?: Array) { + webSocketServer.broadcast(`app:${app.externalId}`, 'deps:install:status', { + status: 'installing', + }); + + npmInstall({ + args: [], + cwd: pathToApp(app.externalId), + packages, + stdout: (data) => { + webSocketServer.broadcast(`app:${app.externalId}`, 'deps:install:log', { + log: { type: 'stdout', data: data.toString('utf8') }, + }); + }, + stderr: (data) => { + webSocketServer.broadcast(`app:${app.externalId}`, 'deps:install:log', { + log: { type: 'stderr', data: data.toString('utf8') }, + }); + }, + onExit: (code) => { + console.log(`npm install exit code: ${code}`); + + webSocketServer.broadcast(`app:${app.externalId}`, 'deps:install:status', { + status: code === 0 ? 'complete' : 'failed', + code, + }); + + if (code === 0) { + webSocketServer.broadcast(`app:${app.externalId}`, 'deps:status:response', { + nodeModulesExists: true, + }); + } + }, + }); +} diff --git a/packages/api/server/channels/app.mts b/packages/api/server/channels/app.mts index b2cd6465..dded314f 100644 --- a/packages/api/server/channels/app.mts +++ b/packages/api/server/channels/app.mts @@ -20,9 +20,9 @@ import WebSocketServer, { type MessageContextType, type ConnectionContextType, } from '../ws-client.mjs'; -import { loadApp } from '../../apps/app.mjs'; +import { installAppDependencies, loadApp } from '../../apps/app.mjs'; import { fileUpdated, pathToApp } from '../../apps/disk.mjs'; -import { vite, npmInstall } from '../../exec.mjs'; +import { vite } from '../../exec.mjs'; import { directoryExists } from '../../fs-utils.mjs'; const VITE_PORT_REGEX = /Local:.*http:\/\/localhost:([0-9]{1,4})/; @@ -155,33 +155,7 @@ async function dependenciesInstall( return; } - npmInstall({ - args: [], - cwd: pathToApp(app.externalId), - packages: payload.packages ?? undefined, - stdout: (data) => { - conn.reply(`app:${app.externalId}`, 'deps:install:log', { - log: { type: 'stdout', data: data.toString('utf8') }, - }); - }, - stderr: (data) => { - conn.reply(`app:${app.externalId}`, 'deps:install:log', { - log: { type: 'stderr', data: data.toString('utf8') }, - }); - }, - onExit: (code) => { - conn.reply(`app:${app.externalId}`, 'deps:install:status', { - status: code === 0 ? 'complete' : 'failed', - code, - }); - - if (code === 0) { - conn.reply(`app:${app.externalId}`, 'deps:status:response', { - nodeModulesExists: true, - }); - } - }, - }); + installAppDependencies(app, payload.packages); } async function clearNodeModules( From 87d12bd949fc0d799284cb8bc0f8d73e1b6f8fc5 Mon Sep 17 00:00:00 2001 From: Ryan Gaus Date: Tue, 22 Oct 2024 16:56:51 -0400 Subject: [PATCH 2/4] fix: make sure that if a process has a null port to not send that to the client, kill+restart it instead --- packages/api/server/channels/app.mts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/api/server/channels/app.mts b/packages/api/server/channels/app.mts index dded314f..33df65d0 100644 --- a/packages/api/server/channels/app.mts +++ b/packages/api/server/channels/app.mts @@ -50,11 +50,16 @@ async function previewStart( const existingProcess = processMetadata.get(app.externalId); if (existingProcess) { - wss.broadcast(`app:${app.externalId}`, 'preview:status', { - status: 'running', - url: `http://localhost:${existingProcess.port}/`, - }); - return; + if (existingProcess.port === null) { + existingProcess.process.kill('SIGTERM'); + processMetadata.delete(app.externalId); + } else { + wss.broadcast(`app:${app.externalId}`, 'preview:status', { + status: 'running', + url: `http://localhost:${existingProcess.port}/`, + }); + return; + } } wss.broadcast(`app:${app.externalId}`, 'preview:status', { From 13fe79f798a2f86385b8e8cc92cbd6eddd83ea4d Mon Sep 17 00:00:00 2001 From: Ryan Gaus Date: Tue, 22 Oct 2024 16:57:41 -0400 Subject: [PATCH 3/4] feat: hide the installation toast when a server driven npm install finishes --- .../src/components/apps/package-install-toast.tsx | 2 ++ .../web/src/components/apps/use-package-json.tsx | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/web/src/components/apps/package-install-toast.tsx b/packages/web/src/components/apps/package-install-toast.tsx index 702cdd0c..004c6e3c 100644 --- a/packages/web/src/components/apps/package-install-toast.tsx +++ b/packages/web/src/components/apps/package-install-toast.tsx @@ -33,6 +33,8 @@ const PackageInstallToast: React.FunctionComponent = () => { useEffect(() => { if (nodeModulesExists === false && (status === 'idle' || status === 'complete')) { setShowToast(true); + } else if (nodeModulesExists === true) { + setShowToast(false); } }, [nodeModulesExists, status]); diff --git a/packages/web/src/components/apps/use-package-json.tsx b/packages/web/src/components/apps/use-package-json.tsx index a3457e60..da72dc81 100644 --- a/packages/web/src/components/apps/use-package-json.tsx +++ b/packages/web/src/components/apps/use-package-json.tsx @@ -52,6 +52,17 @@ export function PackageJsonProvider({ channel, children }: ProviderPropsType) { }; }, [channel]); + useEffect(() => { + const callback = ({ status }: DepsInstallStatusPayloadType) => { + setStatus(status); + }; + channel.on('deps:install:status', callback); + + return () => { + channel.off('deps:install:status', callback); + }; + }, [channel]); + const npmInstall = useCallback( async (packages?: Array) => { addLog( @@ -74,7 +85,6 @@ export function PackageJsonProvider({ channel, children }: ProviderPropsType) { const statusCallback = ({ status, code }: DepsInstallStatusPayloadType) => { channel.off('deps:install:log', logCallback); channel.off('deps:install:status', statusCallback); - setStatus(status); addLog( 'info', From 4decc86bbebc8beb44748442bd441b55c75a29a2 Mon Sep 17 00:00:00 2001 From: Ryan Gaus Date: Tue, 22 Oct 2024 17:02:44 -0400 Subject: [PATCH 4/4] fix: temporarily disable npm install when starting the app This is to eliminate possible cases as part of some experimentation --- packages/web/src/components/apps/use-preview.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/web/src/components/apps/use-preview.tsx b/packages/web/src/components/apps/use-preview.tsx index 0ea327e5..6868b964 100644 --- a/packages/web/src/components/apps/use-preview.tsx +++ b/packages/web/src/components/apps/use-preview.tsx @@ -56,9 +56,10 @@ export function PreviewProvider({ channel, children }: ProviderPropsType) { }, [channel, addLog]); async function start() { - if (nodeModulesExists === false) { - await npmInstall(); - } + // NOTE: only run this if status !== 'installing' maybe? + // if (nodeModulesExists === false) { + // await npmInstall(); + // } channel.push('preview:start', {}); }