-
Notifications
You must be signed in to change notification settings - Fork 1.2k
implement custom builds, for dev and publish
#149
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.
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
Changes from all commits
a5e3055
bbfa3f9
c87040d
c4d40b4
875843a
4ae5c27
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "wrangler": patch | ||
| --- | ||
|
|
||
| Custom builds for `dev` and `publish` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,4 +35,3 @@ jobs: | |
|
|
||
| - name: Test | ||
| run: npm run test | ||
| working-directory: packages/wrangler | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| packages/wrangler/vendor/ | ||
| packages/wrangler/wrangler-dist/ | ||
| packages/example-worker-app/dist/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| dist |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import esbuild from "esbuild"; | ||
| import { readFile } from "fs/promises"; | ||
| import { existsSync } from "fs"; | ||
| import type { DirectoryResult } from "tmp-promise"; | ||
| import tmp from "tmp-promise"; | ||
| import type { CfPreviewToken } from "./api/preview"; | ||
|
|
@@ -16,14 +17,15 @@ import onExit from "signal-exit"; | |
| import { syncAssets } from "./sites"; | ||
| import clipboardy from "clipboardy"; | ||
| import http from "node:http"; | ||
| import serveStatic from "serve-static"; | ||
| import commandExists from "command-exists"; | ||
| import assert from "assert"; | ||
| import { getAPIToken } from "./user"; | ||
| import fetch from "node-fetch"; | ||
| import makeModuleCollector from "./module-collection"; | ||
| import { withErrorBoundary, useErrorHandler } from "react-error-boundary"; | ||
| import { createHttpProxy } from "./proxy"; | ||
| import { execa } from "execa"; | ||
| import { watch } from "chokidar"; | ||
|
|
||
| type CfScriptFormat = void | "modules" | "service-worker"; | ||
|
|
||
|
|
@@ -42,6 +44,11 @@ type Props = { | |
| compatibilityDate: void | string; | ||
| compatibilityFlags: void | string[]; | ||
| usageModel: void | "bundled" | "unbound"; | ||
| buildCommand: { | ||
| command?: undefined | string; | ||
| cwd?: undefined | string; | ||
| watch_dir?: undefined | string; | ||
| }; | ||
| }; | ||
|
|
||
| function Dev(props: Props): JSX.Element { | ||
|
|
@@ -54,8 +61,13 @@ function Dev(props: Props): JSX.Element { | |
| const apiToken = getAPIToken(); | ||
| const directory = useTmpDir(); | ||
|
|
||
| // if there isn't a build command, we just return the entry immediately | ||
| // ideally there would be a conditional here, but the rules of hooks | ||
| // kinda forbid that, so we thread the entry through useCustomBuild | ||
| const entry = useCustomBuild(props.entry, props.buildCommand); | ||
|
|
||
| const bundle = useEsbuild({ | ||
| entry: props.entry, | ||
| entry, | ||
| destination: directory, | ||
| staticRoot: props.public, | ||
| jsxFactory: props.jsxFactory, | ||
|
|
@@ -361,6 +373,77 @@ function useTmpDir(): string | void { | |
| return directory?.path; | ||
| } | ||
|
|
||
| function runCommand() {} | ||
|
|
||
| function useCustomBuild( | ||
| expectedEntry: string, | ||
| props: { | ||
| command?: undefined | string; | ||
| cwd?: undefined | string; | ||
| watch_dir?: undefined | string; | ||
| } | ||
| ): void | string { | ||
| const [entry, setEntry] = useState<string | void>( | ||
| // if there's no build command, just return the expected entry | ||
| props.command ? null : expectedEntry | ||
| ); | ||
| const { command, cwd, watch_dir } = props; | ||
| useEffect(() => { | ||
| if (!command) return; | ||
| let cmd, interval; | ||
| console.log("running:", command); | ||
| const commandPieces = command.split(" "); | ||
| cmd = execa(commandPieces[0], commandPieces.slice(1), { | ||
| ...(cwd && { cwd }), | ||
| stderr: "inherit", | ||
| stdout: "inherit", | ||
| }); | ||
| if (watch_dir) { | ||
| watch(watch_dir, { persistent: true, ignoreInitial: true }).on( | ||
| "all", | ||
| (_event, _path) => { | ||
| console.log(`The file ${path} changed, restarting build...`); | ||
| cmd.kill(); | ||
| cmd = execa(commandPieces[0], commandPieces.slice(1), { | ||
| ...(cwd && { cwd }), | ||
| stderr: "inherit", | ||
| stdout: "inherit", | ||
| }); | ||
|
Comment on lines
407
to
411
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block is now repeated - let's move it into its own function?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just one statement, doesn't pass my threshold for a new abstraction yet imo |
||
| } | ||
| ); | ||
| } | ||
|
|
||
| // check every so often whether `expectedEntry` exists | ||
| // if it does, we're done | ||
| const startedAt = Date.now(); | ||
| interval = setInterval(() => { | ||
| if (existsSync(expectedEntry)) { | ||
| clearInterval(interval); | ||
| setEntry(expectedEntry); | ||
| } else { | ||
| const elapsed = Date.now() - startedAt; | ||
| // timeout after 30 seconds of waiting | ||
| if (elapsed > 1000 * 60 * 30) { | ||
| console.error("⎔ Build timed out."); | ||
| clearInterval(interval); | ||
| cmd.kill(); | ||
| } | ||
| } | ||
| }, 200); | ||
| // TODO: we could probably timeout here after a while | ||
|
|
||
| return () => { | ||
| if (cmd) { | ||
| cmd.kill(); | ||
| cmd = undefined; | ||
| } | ||
| clearInterval(interval); | ||
| interval = undefined; | ||
| }; | ||
| }, [command, cwd, expectedEntry, watch_dir]); | ||
| return entry; | ||
| } | ||
|
|
||
| type EsbuildBundle = { | ||
| id: number; | ||
| path: string; | ||
|
|
@@ -371,7 +454,7 @@ type EsbuildBundle = { | |
| }; | ||
|
|
||
| function useEsbuild(props: { | ||
| entry: string; | ||
| entry: void | string; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OOC - what's the difference between
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to revisit very |
||
| destination: string | void; | ||
| staticRoot: void | string; | ||
| jsxFactory: string | void; | ||
|
|
@@ -382,7 +465,7 @@ function useEsbuild(props: { | |
| useEffect(() => { | ||
| let result: esbuild.BuildResult; | ||
| async function build() { | ||
| if (!destination) return; | ||
| if (!destination || !entry) return; | ||
| const moduleCollector = makeModuleCollector(); | ||
| result = await esbuild.build({ | ||
| entryPoints: [entry], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is fine for now, we should think about a more generalised solution for when the other packages in this workspace have tests to run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strongly agreed, just did this as a quick dx win for myself rn.