Add watch and serve subcommands to wasm-pack

[fitzgen]

Rendered

---

cc @ashleygwilliams @alexcrichton @DebugSteven @drager

[alexcrichton]

Thanks for writing this up @fitzgen! I think this wasn't something I realized I wanted until you articulated it...

Long-term I'd love to explore possibilities of removing the static file server in wasm-bindgen-test-runner as well. If we could leverage wasm-pack's own internal server somehow that'd be great. I think that's pretty squarely outside the scope of this RFC, though, and we can deal with that later.

[alexcrichton]

An interesting aspect of the serve command is that I think it basically doesn't work if you either don't pass -t or if you pass an incompatible value like -t nodejs.

I wonder if we should have some stricter validation (and documentation) which says that wasm-pack serve is only compatible with the no-modules and web (new) target?

[alexcrichton]

FWIW this doesn't seem like too much of a con to me, if we were dealing with C/C++ I'd definitely agree but with Rust error recovery is pretty simple in-process.

That being said I still agree the next con outweighs the pro

[fitzgen]

I wasn't thinking just memory, but also other resources like sockets, but yeah much less of a deal in Rust.

[alexcrichton]

Thinking about this, one feature of webpack-dev-server is that when you request a page it actually waits to send any data until the current build is done. In that sense I think that a con of using an external binary is that we can't implement logic like that.

For example if I make a change and then quickly go and refresh a web page, I might get stale output if the build hasn't finished yet.

I wonder though if we could perhaps destroy --out-dir on each build to ensure that if the page is loaded stale artifacts are never served up? (not a great experience but perhaps better than seeing stale data)

[drager]

Maybe this should also be able to be controlled from wasm-pack's toml configuration? https://rustwasm.github.io/wasm-pack/book/cargo-toml-configuration.html

[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.

[fitzgen]

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.

[jrvidal]

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 😅

[fitzgen]

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.

[fitzgen]

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:

• @alexcrichton
• @ashleygwilliams
• @fitzgen

[alexcrichton]

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

[ashleygwilliams]

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.

[alexcrichton]

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?

[drager]

@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.

[alexcrichton]

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.

[ashleygwilliams]

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.

[boringcactus]

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 #10 (comment) 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.

[boringcactus]

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

[boringcactus]

I've thrown together a draft of a plausible implementation at rustwasm/wasm-pack#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.

[jasonwilliams]

What's the blocker for this RFC?

[pastelmind]

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.

[jasonwilliams]

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)