rfcs

rfcs copied to clipboard

Rendered

---

cc @ashleygwilliams @alexcrichton @DebugSteven @drager

I think this is a great idea! Both commands and functionality makes sense to implement. I agree that if wasm-pack implements the serve command, watching and automatic rebuild is required together with serve.

Just pushed two more commits ådding an unresolved question about how to choose which files to watch and added a --root <root> option to the proposed wasm-pack serve subcommand.

Random thought out of the blue: there is a related, potential use case not covered by this RFC which is integrating with a JS bundler watch process (already hinted by @chinedufn I think)

In a big JS project where Rust is introduced incrementally, it's very likely that there is already a bundler in place (with a sizable configuration) taking care of the watching and rebuilding. There currently exist a few <bundler>-rust-loader-plugin-thingy for the popular bundlers but, from my superficial impressions, they focus in raw rustc usage and single Rust files exposing the barebones Wasm FFI.

Now, if wasm-pack wanted to help with this use case, I suspect it would have to yield the watching, and instead expose as much build-plan information as possible so the loaders can kick this information up to the bundler's own watcher.

OTOH, it's also possible to write loaders top of cargo alone and let wasm-pack completely out of it. But I wonder if there is some functionality that will have to be re-implemented by the loader. I'm thinking about the tiny things that wasm-pack does to ensure a smooth experience, like checking your Rust installation, or downloading wasm-bindgen binaries (these would be the extra things that would be exposed in the "build plan").

Of course this might very well be out of scope for an initial MVP, or for wasm-pack outright :sweat_smile:

integrating with a JS bundler watch process

I think we can split this scenario into two designs:

1. The JS bundler asks wasm-pack to watch for changes, since wasm-pack knows more about the Rust sources than the JS bundler does. Then wasm-pack watch is exactly what is needed, and the JS bundler can watch the pkg directory to get its downstream updates. The potential downside is overlapping watch directories with the project's other JS, and in an ideal world a single file would only ever be watched by a single thing.

2. The JS bundler wants to watch for changes and then tell wasm-pack when to rebuild. This can potentially solve the "only a single watcher for a single file" thing. In this scenario, the JS bundler wants cargo build --build-plan to figure out which files to watch, same as we want for wasm-pack watch. It can re-invoke wasm-pack build on each change however it sees fit. The latency of wasm-pack build is pretty good too, so false positives shouldn't be a big deal, and means that until cargo build --build-plan exists, it is free to use its own heuristics for choosing what to watch, similar to what we plan on shipping for wasm-pack watch initially.

Most importantly, in both integrating-with-JS-bundler-watching scenarios, I don't see anything that needs to change in this RFC, or any fundamental design contradictions.

The discussion here seems to have slowed down a bit, and there seems to be broad consensus that we want this general feature, and that we can continue to improve and evolve the subcommands during and after initial implementation.

Therefore, I'd like to propose that this RFC enter its final comment period with disposition to merge.

@rustwasm/core team members to sign off:

• [x] @alexcrichton
• [ ] @ashleygwilliams
• [x] @fitzgen

Still quite a fan of this feature, so checking my box!

Hey! So ~as I said previously~ I'd like to postpone this until we've shipped 0.8.0 (April 26th). At that point I'll be in a better position to evaluate and respond to this RFC (because we will be closer to being able to prioritize these features and I'd prefer to not have a large amount of time between the design and implementation work.)

---

EDIT: I'm generally super in favor of this, and just anticipate I may have some design input, not that I would reject these features.

Wanted to ping on this! @ashleygwilliams and @drager do y'all feel that wasm-pack is in a position where this can be reviewed/implemented?

@alexcrichton I'm not sure in which sprint we can do this in wasm-pack at the moment. Both @ashleygwilliams and I have been pretty busy these last two weeks and we have a couple of issues left labeled sprint and some new issues I think we need to deal with and/or investigate.

Personally, if we can fit some new functionality into the next sprint I think this should probably be one of those features.

Note though that this isn't really about implementation work, for now this is purely just "needs a review and discussion on the RFC" style work.

I'm on vacation but I 100% think we should open this up to conversation about design as we should be able to do this, likely not the next sprint (as originally intended) but the one after.

is there anything community members can do to help finalize this RFC?

if we need more opinions about embedded vs external http handling, my opinion is that https://github.com/rustwasm/rfcs/pull/10#discussion_r263795579 is a good reason to build the server directly into wasm-pack instead of shelling out to an external server. having weird timing conditions where users can sometimes get stale data, or nothing at all, substantially weakens the user experience, which is a serious drawback in subcommands that are designed to streamline the user experience.

Also worth noting is that per https://github.com/rust-lang/cargo/issues/7614 cargo build --build-plan is being removed, so watch will have to decide on its own which files should be watched.

I've thrown together a draft of a plausible implementation at https://github.com/rustwasm/wasm-pack/pull/745, partially out of curiosity and partially because @ashleygwilliams suggested that having a short turnaround time between design and implementation would be helpful.

My implementation uses an integrated server and a RwLock to pause the server while a build is running, and adds less than 200 lines of code, so it seems that an integrated server would not in fact be more work to implement.

What's the blocker for this RFC?

The JS bundler wants to watch for changes and then tell wasm-pack when to rebuild.

This didn't work for me (using Rollup) when I integrated webpack into my build config, because it created an infinite loop. When wasm-pack rebuilt the Rust code, the regenerated pkg/ dir caused Rollup to rebuild, which invoked wasm-pack, which regenerated pkg/, which triggered Rollup, and so on.

I ended up running a separate watcher that only watches Rust code and invokes wasm-pack.

In the meantime if anyone is using esbuild https://github.com/Tschrock/esbuild-plugin-wasm-pack supports watching Rust files (as well as any JS files)