Add logic to represent booting state within preview panel and parse stdout from vite to get port #349
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…arse stdout from vite to get port
1egoman
commented
Oct 11, 2024
Comment on lines
+25
to
+30
| type ProcessMetadata = { | ||
| process: ChildProcess; | ||
| port: number | null; | ||
| }; | ||
|
|
||
| const processMetadata = new Map<string, ProcessMetadata>(); |
There was a problem hiding this comment.
Note that now, processes is called processMetadata and includes a process plus associated metadata (currently just the port it is running on).
Comment on lines
11
to
19
| switch (status) { | ||
| case "booting": | ||
| case "connecting": | ||
| 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": |
There was a problem hiding this comment.
Right now, this is what the loading state looks like. An official design (I couldn't find one for something like this in figma) would be appreciated if something less plain is desired!
| @@ -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})/; | |||
There was a problem hiding this comment.
Here's the regex for extracting the port vite picks from stdout.
benjreinhart
approved these changes
Oct 11, 2024
| const potentialPortMatch = VITE_PORT_REGEX.exec(encodedData); | ||
| if (potentialPortMatch) { | ||
| const portString = potentialPortMatch[1]!; | ||
| const port = parseInt(portString, 10); |
There was a problem hiding this comment.
Number(portString) :)
| const encodedData = data.toString('utf8'); | ||
| console.log(encodedData); | ||
|
|
||
| const potentialPortMatch = VITE_PORT_REGEX.exec(encodedData); |
There was a problem hiding this comment.
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.
nichochar
reviewed
Oct 12, 2024
| type AppContextType = MessageContextType<'appId'>; | ||
|
|
||
| const processes = new Map<string, ChildProcess>(); | ||
| type ProcessMetadata = { |
There was a problem hiding this comment.
Should this also have the status "running"?
And potentially exit code
| args: [], | ||
| cwd: pathToApp(app.externalId), | ||
| stdout: (data) => { | ||
| console.log(data.toString('utf8')); | ||
| const encodedData = data.toString('utf8'); | ||
| console.log(encodedData); |
There was a problem hiding this comment.
Why the 2 lines? Is there a behavior change?
nichochar
approved these changes
Oct 12, 2024
BeRecursive22
pushed a commit
to BeRecursive22/srcbook
that referenced
this pull request
Oct 13, 2024
…tdout from vite to get port (srcbookdev#349) * feat: add logic to represent booting state within preview panel and parse stdout from vite to get port * fix: run npm run format
nichochar
pushed a commit
that referenced
this pull request
Oct 14, 2024
#333) * [FIX]:285 - Type proccess.env based off the available secrets in a notebook * [ADD] - changeset * [FIX] - linting and formatting * Style apps page sidebar and header (#340) * Update sans font (#341) * File tree functionality and styles (#344) * Add support for deleting and renaming files (#345) * Add support for deleting and renaming files * Fix lint * Exclude a few directories * Implement top level create file and folder (#346) * Homepage UI Rework! (#348) * inital homepage rework, sections, card style, srcbook to notebook * fix lil linter error * move user facing Srcbook (in ref to notebook) to Notebook * style up to match figma * some cleanup around purples on create app (#351) * fix CI bug with pnpm CI mismatch in release (#354) * Add logic to represent booting state within preview panel and parse stdout from vite to get port (#349) * feat: add logic to represent booting state within preview panel and parse stdout from vite to get port * fix: run npm run format * Add folder delete and rename behavior (#355) * Add folder delete and rename behavior * Remove unused files state * Fix lint warnings * [FIX] - env type errors * put the env.d.ts file under root directory instead of src * remove cell:format --------- Co-authored-by: Ben Reinhart <[email protected]> Co-authored-by: versecafe <[email protected]> Co-authored-by: Ryan Gaus <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the preview panel in the app builder is a bit unreliable - when it opens, the iframe sometimes is pointing to the wrong vite server or nothing at all.
This pull request attempts to make this behavior more consistient, by doing three things:
preview:startmessage is sent to the server, the server now sends back apreview:statusmessage including astatusof"booting". - the client then shows a loading indicator. Once a port is detected in the stdout, a secondary message is sent to the client including this port, and the loading indicator goes away, and an iframe is rendered going to this port.1.is sent back to the client again, resulting in the preview panel opening back up to the correct url.