-
Notifications
You must be signed in to change notification settings - Fork 653
refactor(rome_wasm): move the WASM bindings for the Workspace to a crate #3056
Conversation
Deploying with
|
Latest commit: |
12839f7
|
Status: | ✅ Deploy successful! |
Preview URL: | https://469098d3.tools-8rn.pages.dev |
Branch Preview URL: | https://feature-workspace-wasm.tools-8rn.pages.dev |
I think it makes sense to have platform specifics packages, for the time being. For the browser one, yes, it's definitely something that we have to. For Node.js, I would say YES for the time being, because it's one of the options we want to try and deliver the APIs. If one day we decide that we don't need it anymore, we can deprecate it.
There are pros and cons. For the time being I think we should keep them private, at least we figure out how to use these packages in our internal Then, once we know how to work with these, we might want to consider to promoting them and let the user decide. |
891e9dc
to
906377f
Compare
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.
Few questions:
- do we plan to set a publishing workflow of this packages in another PR?
- do we plan to publish
@rometools/wasm-web
and use it directly in our playground using another PR? - do you plan to use pnpm workspace to import
@rometools/wasm-web
in another PR?
The publishing workflow is already included in this PR, the
That's something we may do in the future, but at the moment I think the Workspace API is not stable enough to allow this (we need to push changes to the playground in sync with every modification we do on the workspace), so I'm importing the
I'll probably try to have a single pnpm workspace at the root of the repo eventually like we do for Cargo, I just need to come up with a good list of what package should be included in it and what package should not (out of the vscode extension, various npm packages, website and playground). And also figure out if having a bunch of auto-generated packages that may or may not exist is handled correctly |
Why the workflow it's together with the CLI? If there are errors in these packages (even though the source code is slim), should we not be able to publish them also independently ? Not a blocker for this PR |
This is because the CLI is published to npm under the same |
…ate (rome#3056) * refactor(rome_wasm): move the WASM bindings for the Workspace to a dedicated crate * fix pull_request_js workflow * fix CI
Summary
This moves the WASM bindings to the Workspace API out of the playground into a dedicated
rome_wasm
crate. This crate is built withwasm-pack
in the CLI release workflow to generate 3 npm packages:@rometools/wasm-bundler
,@rometools/wasm-nodejs
and@rometools/wasm-web
, acting as wrappers for the WebAssembly module for different platforms (bundler
imports the WASM blob using ESM,nodejs
loads it with the node.jsfs
API, andbrowser
loads it using the WHATWGfetch
API)The
wasm-web
package is now used to provide the playground features, although it makes the playground development workflow somewhat convoluted since it's necessary to runpnpm build:wasm
(orpnpm build:wasm-dev
) before runningpnpm install
to ensure the package has been previously generated on the file system.Test Plan
This is mostly a build infrastructure change, tested through the playground deployment workflow and nightly release workflow.
Besides that, the
wasm-bindgen
ecosystem supports building tests to WASM and running them in a headless browser withwasm-bindgen-test
. We could perform some additional testing on the Workspace API there, but I'm not sure if it would provide much additional value or be mostly redundant with the existing CLI and LSP tests