Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first stab at watch and serve implementation #745

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

boringcactus
Copy link

The RFC for this feature, rustwasm/rfcs#10, hasn't been formally accepted yet, but I was wondering how hard it'd be to build these features, and apparently it's possible to build a simple but functional prototype in <200 lines of new code.

Things I like about this code:

  • preserving build option logic verbatim is easy with #[structopt(flatten)]
  • a RwLock is perfect for holding the server until the build finishes
  • using an external crate for file handling means we get the right MIME type basically for free (although i could probably be using the hyper-staticfile crate more effectively)
  • a zero-element mpsc sync channel allows the watch event thread to pass a notification to the watch build thread only if it isn't already building and discard changes found while building (a more naive approach would queue three new builds if three files were changed while building, and that's not good)

Things that need to be resolved:

  • can the server listen on both ipv4 and ipv6 loopback addresses? i can't tell if hyper supports that
  • which files should be watched? the cargo-watch crate appears to watch the entire crate directory except for things in .gitignore and a handful of manually defined exclusions
  • do these subcommands need to cleanly exit on Ctrl-C or is it fine to just exit the program immediately?
  • how should any of this be automatically tested?
  • ideally, if some set of files are changed during the build process, the build should rerun once; this could probably just be achieved by giving the build channel a buffer size of 1 instead of 0, but i'd have to test further to be sure that it works

@boringcactus boringcactus mentioned this pull request Aug 8, 2020
@ashleygwilliams ashleygwilliams marked this pull request as ready for review August 28, 2020 18:09
@yoshuawuyts
Copy link
Contributor

Hi @boringcactus; thanks so much for this PR! Sorry this has taken a while to review. @ashleygwilliams and I had a call about this PR yesterday and we reviewed it together.

We found this PR to be an excellent starting point, and are happy to land the code. A few adjustments will need to be made though: specifically fixing the merge conflict, and adding tests. But @ashleygwilliams has said she would pick that up.

Either way we're really excited about this PR, and wanted to let you know we're planning to move ahead with the work you've done! Thanks so much!

@juchiast
Copy link

Hi team, any news on this PR? I came from cargo-web and would love that we have a serve command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants