Publish to NPM #23
Publish to NPM #23
Conversation
Makefile
Outdated
| @@ -0,0 +1,78 @@ | |||
|
|
|||
| LATEST_COMPATIBILITY_DATE=$(shell bazel build @capnp-cpp//src/capnp:capnp_tool && bazel-bin/external/capnp-cpp/src/capnp/capnp_tool eval src/workerd/io/compatibility-date.capnp supportedCompatibilityDate) | |||
There was a problem hiding this comment.
Using a Makefile is likely fine for launch but given that there are bazel rules to support publishing to npm and we use bazel for everything else, we should likely convert this into a bazel task.
There was a problem hiding this comment.
Agreed we should probably have a comment like # TODO(cleanup): Convert to bazel rules?
Makefile
Outdated
| LATEST_COMPATIBILITY_DATE=$(shell bazel build @capnp-cpp//src/capnp:capnp_tool && bazel-bin/external/capnp-cpp/src/capnp/capnp_tool eval src/workerd/io/compatibility-date.capnp supportedCompatibilityDate) | ||
| WORKERD_VERSION=1.$(shell bazel build @capnp-cpp//src/capnp:capnp_tool && bazel-bin/external/capnp-cpp/src/capnp/capnp_tool eval src/workerd/io/compatibility-date.capnp supportedCompatibilityDate | tr -d '-' | tr -d '"').0 | ||
|
|
||
| platform-bazel-build: |
There was a problem hiding this comment.
None of the targets in this makefile appear to be actual files to be built. Instead it looks like you're using make as a way to write a shell script with multiple entrypoints. I think it would be better to write this as an actual shell script. It's easy to use case to match the first argument. Make doesn't really provide any advantages over shell when the targets aren't actually files. I would use bash (#! /bin/bash) and make sure to use set -euo pipefail at the start of the script to turn on error checking.
This would also help make it clear that this makefile is not intended to build the project. Otherwise I think many people will see Makefile and immediately assume they should use make to build the code, which will confusingly then do npm-related work.
npm/lib/node-install.ts
Outdated
| @@ -0,0 +1,297 @@ | |||
| // Adapted from evanw/esbuild | |||
There was a problem hiding this comment.
If this is a derivative work it should probably mention the original copyright and license.
| @@ -6,7 +6,7 @@ | |||
|
|
|||
| $import "/capnp/c++.capnp".namespace("workerd"); | |||
|
|
|||
| const supportedCompatibilityDate :Text = "2022-09-17"; | |||
| const supportedCompatibilityDate :Text = "2022-09-20"; | |||
There was a problem hiding this comment.
Let's either bump this to today or leave it out of the PR.
Makefile
Outdated
| @@ -0,0 +1,78 @@ | |||
|
|
|||
| LATEST_COMPATIBILITY_DATE=$(shell bazel build @capnp-cpp//src/capnp:capnp_tool && bazel-bin/external/capnp-cpp/src/capnp/capnp_tool eval src/workerd/io/compatibility-date.capnp supportedCompatibilityDate) | |||
| WORKERD_VERSION=1.$(shell bazel build @capnp-cpp//src/capnp:capnp_tool && bazel-bin/external/capnp-cpp/src/capnp/capnp_tool eval src/workerd/io/compatibility-date.capnp supportedCompatibilityDate | tr -d '-' | tr -d '"').0 | |||
There was a problem hiding this comment.
Could you use LATEST_COMPATIBILITY_DATE here?
| @@ -0,0 +1,6 @@ | |||
| # 👷 `workerd` for macOS 64-bit, Cloudflare's JavaScript/Wasm Runtime | |||
There was a problem hiding this comment.
Do you know why esbuild checks the npm/* folders into the repository? Is this required for npm to fetch the README.md correctly?
There was a problem hiding this comment.
Not entirely sure, but I think it just makes it easier to manage. I suppose each folder could be generated while publishing?
Pending this repo being made public, this enables publishing to NPM (for Miniflare's consumption), modelled after the approach of esbuild. The scripts added in the Makefile will ideally be moved to a github action after the repo is made public.
What the trigger should be for that GH action is definitely a discussion point to have—my current thinking is that we should build on every commit to
main, and publish new1.{version}.0packages to NPM whenever the compatibility date changes (and trigger a GH release with downloadable assets). It might also be useful to publish every commit as1.{version}.{patchversion}to NPM if the compatibility date hasn't changes since the last release.