Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logic to represent booting state within preview panel and parse stdout from vite to get port #349

Merged
merged 2 commits into from
Oct 12, 2024
Merged

Add logic to represent booting state within preview panel and parse stdout from vite to get port #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7edcdbf
feat: add logic to represent booting state within preview panel and p…
1egoman Oct 11, 2024
f0dab60
fix: run npm run format
1egoman Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 44 additions & 20 deletions packages/api/server/channels/app.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@ import { loadApp } from '../../apps/app.mjs';
import { fileUpdated, pathToApp } from '../../apps/disk.mjs';
import { vite } from '../../exec.mjs';

const VITE_PORT_REGEX = /Local:.*http:\/\/localhost:([0-9]{1,4})/;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the regex for extracting the port vite picks from stdout.


type AppContextType = MessageContextType<'appId'>;

const processes = new Map<string, ChildProcess>();
type ProcessMetadata = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also have the status "running"?

And potentially exit code

process: ChildProcess;
port: number | null;
};

const processMetadata = new Map<string, ProcessMetadata>();
Comment on lines +25 to +30
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that now, processes is called processMetadata and includes a process plus associated metadata (currently just the port it is running on).


async function previewStart(
_payload: PreviewStartPayloadType,
Expand All @@ -33,41 +40,56 @@ async function previewStart(
return;
}

const existingProcess = processes.get(app.externalId);
const existingProcess = processMetadata.get(app.externalId);

if (existingProcess) {
conn.reply(`app:${app.externalId}`, 'preview:status', { url: null, status: 'running' });
conn.reply(`app:${app.externalId}`, 'preview:status', {
status: 'running',
url: `http://localhost:${existingProcess.port}/`,
});
return;
}

conn.reply(`app:${app.externalId}`, 'preview:status', {
url: null,
status: 'booting',
});

const onChangePort = (newPort: number) => {
processMetadata.set(app.externalId, { process, port: newPort });
conn.reply(`app:${app.externalId}`, 'preview:status', {
url: `http://localhost:${newPort}/`,
status: 'running',
});
};

const process = vite({
// TODO: Configure port and fail if port in use
args: [],
cwd: pathToApp(app.externalId),
stdout: (data) => {
console.log(data.toString('utf8'));
const encodedData = data.toString('utf8');
console.log(encodedData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the 2 lines? Is there a behavior change?


const potentialPortMatch = VITE_PORT_REGEX.exec(encodedData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be noted that chunks of data are not guaranteed to arrive deterministically in a way that we expect. This is why eg stream parsing is typically stateful. While I expect this to behave as we want it to, it's not guaranteed.

if (potentialPortMatch) {
const portString = potentialPortMatch[1]!;
const port = parseInt(portString, 10);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number(portString) :)

onChangePort(port);
}
},
stderr: (data) => {
console.error(data.toString('utf8'));
},
onExit: (_code) => {
processes.delete(app.externalId);
processMetadata.delete(app.externalId);
conn.reply(`app:${app.externalId}`, 'preview:status', {
url: null,
status: 'stopped',
});
},
});

processes.set(app.externalId, process);

// TODO: better way to know when the server is ready
setTimeout(() => {
conn.reply(`app:${app.externalId}`, 'preview:status', {
url: 'http://localhost:5174/',
status: 'running',
});
}, 500);
processMetadata.set(app.externalId, { process, port: null });
}

async function previewStop(
Expand All @@ -81,14 +103,14 @@ async function previewStop(
return;
}

const process = processes.get(app.externalId);
const result = processMetadata.get(app.externalId);

if (!process) {
if (!result) {
conn.reply(`app:${app.externalId}`, 'preview:status', { url: null, status: 'stopped' });
return;
}

process.kill('SIGTERM');
result.process.kill('SIGTERM');

conn.reply(`app:${app.externalId}`, 'preview:status', { url: null, status: 'stopped' });
}
Expand Down Expand Up @@ -117,13 +139,15 @@ export function register(wss: WebSocketServer) {
return;
}

const existingProcess = processes.get(app.externalId);
const existingProcess = processMetadata.get(app.externalId);

ws.send(
JSON.stringify([
topic,
'preview:status',
{ url: null, status: existingProcess ? 'running' : 'stopped' },
existingProcess
? { status: 'running', url: `http://localhost:${existingProcess.port}/` }
: { url: null, status: 'stopped' },
]),
);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/components/apps/use-preview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { createContext, useContext, useEffect, useState } from 'react';
import { AppChannel } from '@/clients/websocket';
import { PreviewStatusPayloadType } from '@srcbook/shared';

export type PreviewStatusType = 'connecting' | 'booting' | 'running' | 'stopped';
export type PreviewStatusType = 'booting' | 'connecting' | 'running' | 'stopped';

export interface PreviewContextValue {
url: string | null;
Expand Down
30 changes: 21 additions & 9 deletions packages/web/src/components/apps/workspace/preview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,27 @@ type PropsType = {
};

export function Preview(props: PropsType) {
const { url } = usePreview();
const { url, status } = usePreview();

if (url === null) {
return;
}
switch (status) {
case 'connecting':
case 'booting':
return (
<div className={cn('flex justify-center items-center w-full h-full', props.className)}>
<span className="text-tertiary-foreground">Booting...</span>
</div>
);
case 'running':
if (url === null) {
return;
}

return (
<div className={cn(props.className)}>
<iframe className="w-full h-full" src={url} title="App preview" />
</div>
);
return (
<div className={cn(props.className)}>
<iframe className="w-full h-full" src={url} title="App preview" />
</div>
);
case 'stopped':
return null;
}
}
5 changes: 3 additions & 2 deletions packages/web/src/routes/apps.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,19 @@ export function AppsPage() {

function Apps(props: { app: AppType }) {
const { status: previewStatus } = usePreview();
const previewVisible = previewStatus === 'booting' || previewStatus === 'running';

return (
<div className="h-screen max-h-screen flex">
<Sidebar />
<div
className={cn(
'w-full h-full grid divide-x divide-border',
previewStatus === 'running' ? 'grid-cols-2' : 'grid-cols-1',
previewVisible ? 'grid-cols-2' : 'grid-cols-1',
)}
>
<Editor app={props.app} />
{previewStatus === 'running' && <Preview />}
{previewVisible ? <Preview /> : null}
</div>
</div>
);
Expand Down