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

Implement carton bundle command #16

Closed
MaxDesiatov opened this issue Jul 1, 2020 · 2 comments · Fixed by #97
Closed

Implement carton bundle command #16

MaxDesiatov opened this issue Jul 1, 2020 · 2 comments · Fixed by #97
Labels
enhancement New feature or request

Comments

@MaxDesiatov
Copy link
Collaborator

MaxDesiatov commented Jul 1, 2020

This command should invoke swift build (reusing the toolchain inference code that's already used by carton dev and carton sdk) in the directory for the only executable product, or a product specified with the --product option, similar to how carton dev infers a product to build. It should build the app in the release mode with swift build -c release, possibly passing -Osize flag to swiftc if it the release configuration doesn't do this by default already. After that, it should look for the presence of wasm-strip and wasm-opt on the system and apply size optimizations with those if present, otherwise suggesting to install corresponding WABT and binaryen packages (via Homebrew on macOS).

We also need a new bundle.js JavaScript entrypoint, similar to what we have with dev.js, but without the WebSocket bits, as the release bundle doesn't need hot reloads. This entrypoint should also by compiled by webpack in prod mode for size optimizations. The only question I don't have a clear answer to is, what the final bundle directory should be to which carton bundle writes the output? A few suggestions:

  • .build/bundle/wasm, means that the generated fails get ignored by Git most probably (since .build is in .gitignore most of the time)
  • Bundle, capitalized for consistency with Sources and Tests
  • CartonBundle, in case Bundle is used by other tools, but this one seems a bit verbose to me.

I have a slight preference towards the first option as these files are not supposed to be added to the repository in most of the cases, but the problem is that .build is not visible in Finder and other file managers by default, so I could see some beginner users not finding the final product on the file system. With directories like Bundle we'd need users to explicitly opt out of committing the final products to their repository (again, most of the time, unless they don't have .build in their .gitignore yet, which would make this point applicable to the .build option too).

@MaxDesiatov MaxDesiatov added the enhancement New feature or request label Jul 1, 2020
@laosb
Copy link

laosb commented Jul 5, 2020

If bundle.js is just an entry point, I would suggest just minify it when building Carton.

Personally I would prefer Bundle since different tools always seemed to use different default output directory. My frontend experience tells me it can often differs project to project. At the end I don't think it's necessary to "guess" what users' .gitignore may contains. (Actually none of my own projects, nor my user default includes .build in .gitignore.) I think mentioning the output directory and when the .gitignore does not include Bundle, warn users to do so is enough.

Just my 2 cents, just tried SwiftWASM and it's good. Saw this wandering throughout repos so sorry for suddenly dropping a comment from nowhere. 😅

@MaxDesiatov
Copy link
Collaborator Author

MaxDesiatov commented Jul 5, 2020

If bundle.js is just an entry point, I would suggest just minify it when building Carton.

Yes, this is exactly what we're doing already for dev.js. It's packaged in the static.zip archive attached to the release assets. This archive doesn't change much between releases (but obviously will change when bundle.js is included in it), so the latest 0.2.0 version still has a url for static.zip from 0.1.3 hardcoded in it. The archive is not committed to the git repository though, so we don't increases the size of the repository for no good reason. It's downloaded and unpacked automatically when carton detects that ~/.carton/static directory is missing, or when SHA256 hash of dev.js differs from the hash hardcoded in the carton binary itself.

I think mentioning the output directory and when the .gitignore does not includes Bundle, warn users to do so is enough.

Great point about warning users, this is definitely something I think would be great to implement 👍

Just my 2 cents, just tried SwiftWASM and it's good. Saw this wandering throughout repos so sorry for suddenly dropping a comment from nowhere. 😅

Thanks for your feedback, and no problem at all. 🙂 I definitely do want people to feel free leaving their feedback. It's all still at an early stage, but the more feedback we have, the better we can become. All feedback and contributions are very welcome where you have it, that's why all the development is open and I hope is as transparent as possible.

@MaxDesiatov MaxDesiatov pinned this issue Aug 20, 2020
MaxDesiatov added a commit that referenced this issue Aug 31, 2020
New `bundle.js` entrypoint is added, which only differs from `dev.js` in the lack of the WebSocket hot reloading bit.

`wabt` and `binaryen` Homebrew dependencies are added as required for `wasm-strip` and `wasm-opt` respectively that reduce the resulting bundle binary size.

All resulting bundle files except `index.html` are named by their content hashes to enable [cache busting](https://www.keycdn.com/support/what-is-cache-busting).

Resolves #16.
@MaxDesiatov MaxDesiatov unpinned this issue Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants