implement custom builds, for dev and publish
#149
Conversation
🦋 Changeset detectedLatest commit: 4ae5c27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -371,7 +424,7 @@ type EsbuildBundle = { | |||
| }; | |||
|
|
|||
| function useEsbuild(props: { | |||
| entry: string; | |||
| entry: void | string; | |||
There was a problem hiding this comment.
OOC - what's the difference between void and undefined in this type?
There was a problem hiding this comment.
I need to revisit very void and replace it with undefined. I just got trigger happy and regret it now.
threepointone
force-pushed
the
custom-builds
branch
from
81717a7
to
a4c3ea8
Compare
|
I made some changes -
I'll edit the main PR description as well. |
|
Any thoughts on |
|
Pages dev has an additional requirement of 'detecting' what ports were opened and proxying them, etc, so I can see the requirement there. That said, I'd revisit the choice of the blank |
|
(That said, I'd like for |
| @@ -30,7 +30,8 @@ | |||
| "scripts": { | |||
| "lint": "eslint packages/**", | |||
| "check": "prettier packages/** --check && tsc && npm run lint", | |||
| "prettify": "prettier packages/** --write" | |||
| "prettify": "prettier packages/** --write", | |||
| "test": "npm run test --workspace=wrangler" | |||
There was a problem hiding this comment.
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.
Strongly agreed, just did this as a quick dx win for myself rn.
packages/wrangler/src/dev.tsx
Outdated
| watch(watch_dir, { persistent: true, ignoreInitial: true }).on( | ||
| "all", | ||
| (_event, _path) => { | ||
| console.log("Restarting build..."); |
There was a problem hiding this comment.
Perhaps log the path of the changed file that caused the rebuild?
Something like?
console.log(`The file ${path} changed`);There was a problem hiding this comment.
done and done
| cmd = execa(commandPieces[0], commandPieces.slice(1), { | ||
| ...(cwd && { cwd }), | ||
| stderr: "inherit", | ||
| stdout: "inherit", | ||
| }); |
There was a problem hiding this comment.
This block is now repeated - let's move it into its own function?
There was a problem hiding this comment.
It's just one statement, doesn't pass my threshold for a new abstraction yet imo
The code here is a bit jank, but we should probably refactor the way we pass arguments to dev and publish to 'fix' it properly. Anyway. This PR adds custom builds to wrangler2 (https://developers.cloudflare.com/workers/cli-wrangler/configuration#build). - In `wrangler.toml`, you can configure `build.command` and `build.cwd` and it'll be called before `publish` or `dev`. There's some discussion to be had whether we're going to deprecate this config commands, but we'll support it until then anyway. - We _don't_ support `build.watch_dir`. We could, and I'm happy to add it after we discuss; but the idea is that watching shouldn't be wrangler's responsibility (which we've already broken with plain `dev`, and `pages dev`, so maybe we're wrong) - You can pass the command after a last `--` in the cli (no support for `cwd` there). eg - `wrangler dev output/index.js -- some-build-command --out output/index.js`.
- removed the `--` option. We should have documentation showing how to do this with bash etc. - Added support for `watch_dir` - also added a definition for running tests from the root because it was annoying me to keep changing dirs to run tests
threepointone
force-pushed
the
custom-builds
branch
from
a4c3ea8
to
4ae5c27
Compare
The code here is a bit jank, but we should probably refactor the way we pass arguments to dev and publish to 'fix' it properly. Anyway.
This PR adds custom builds to wrangler2 (https://developers.cloudflare.com/workers/cli-wrangler/configuration#build).
wrangler.toml, you can configurebuild.commandandbuild.cwdand and it'll be called beforepublishordev. We also optionally watchbuild.watch_dirfor changes during dev. There's some discussion to be had whether we're going to deprecate this config commands, but we'll support it until then anyway.I swear I'll start writing tests for
publishbefore the year is over. Maybe evendev.closes #120