Skip to content

Should we use a bundler for our web assets #610

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

Closed
mattico opened this issue Feb 5, 2018 · 11 comments
Closed

Should we use a bundler for our web assets #610

mattico opened this issue Feb 5, 2018 · 11 comments

Comments

@mattico
Copy link
Contributor

mattico commented Feb 5, 2018

My primary motivation for considering a bundler is to get full-featured Rust docs onto Debian: #495. The best way to do that would be to vendor our dependencies' source, and provide a way to produce minified builds for inclusion into mdBook. This could be a one-off system that most people ignore besides package maintainers. However, if we're doing the work to enable this, we may want to take advantage of some other benefits it may give.

Advantages:

  • We now have a regimented way to manage all these npm dependencies, rather than copying .min.js around.
  • We can easily minify the JS that we write, and can use ES6 modules or at least separate files.
  • We can easily support Debian's needs by vendoring our dependencies and minifying them ourselves.
  • We can use TypeScript, which fits with the Rust ethos (though I'm not convinced its worth the trouble with what little JS we have)
  • We can use Webpack loaders to customize dependencies like FontAwesome to reduce size
  • Webpack tree-shaking can automatically remove dead code from our (ES6 module) libraries, though our largest dependencies (Ace and MathJax) are not ES6 modules
  • We can use Webpack to manage other asset build steps like compressing before including them into the binary

Disadvantages:

  • Now you need npm to edit JS, not just for CSS
  • The complexity of JS tooling makes me sad
  • If we minify deps ourselves, we can't benefit from possibly having the deps already in browser cache

The discussion has focused around alternatives to solving these problems without using a JS bundler. I'll try to summarize that idea here:

  • Remove Stylus
    • Removes the dependency on npm
    • Could use mdbook serve --theme <dir> to edit theme without any build step
    • Our primary use for a CSS preprocessor (themes) can be done well using CSS variables
  • Vendor JS dependencies that Debian would use and provide a way to build and use them. It could just be a shell script using yui-compressor, since it would likely only be used by Debian.
  • Remove use of CDNs. We'd need local copies of e.g. fonts which we currently only get from CDNs (or just use system fonts...)
    • Addresses concerns about using CDNs
    • Ensures docs always work offline
    • Cache benefit of CDN not significant enough to warrant a config option
  • Make it easy to disable fonts and whatever else Debian doesn't want
  • If we wanted to compress assets before including them in our binary we could do that in build.rs. This is okay to do there since (unlike stylus processing) it isn't required when you're editing the theme.
@mattico mattico changed the title Add Webpack asset build pipeline' Add Webpack asset build pipeline Feb 5, 2018
@mattico mattico changed the title Add Webpack asset build pipeline What do we do about all this JavaScript Feb 5, 2018
@sorin-davidoi
Copy link
Contributor

I would personally avoid introducing Webpack & co, but don't really see the need to be as aggressive as you suggest in reducing JavaScript usage either.

Let's take the current pain-points:

Need npm installed to change Stylus files

What about ditching Stylus altogether? CSS custom properties support everything in RFC 1985 except UC Browser for Android.

Most of our JS code is in one giant file because its too tedious to deal with multiple

We recently split it into modules - we could just put each module into it's own file, though it would mean more files to copy from Rust.

it doesn't get minified

I would not worry to much about it since it's 19KB.

We ship 70KB of FontAwesome despite only using 4 icons because we have no way to preprocess it

No sure how FontAwesome works, but can't we just manually extract them and place them in the codebase?

@mattico
Copy link
Contributor Author

mattico commented Feb 5, 2018

@sorin-davidoi I don't disagree with your points, the 'current situation' is not that bad, and it was wrong to present this as a dichotomy of choice. My primary reason for considering this was to fix The Debian Problem. I'll reformat the issue because I'm thinking about this in a slightly different way now.

@mattico mattico changed the title What do we do about all this JavaScript Should we use a bundler for our web assets Feb 5, 2018
@Michael-F-Bryan
Copy link
Contributor

I'm also keen to fix The Debian Problem, although from my limited experience with webpack I worry we may just be replacing one problem with another.

That said, we probably need a better system than copying minified JavaScript and CSS around. Abusing feature flags to regenerate-css is also probably not a good idea, I recently read an interesting blog post where you create a second tools crate next to your main one which contains programs to run frequently used bits of code.

One possible solution is to pull all the CSS/JS stuff out of the src/themes/ directory and use something like webpack to generate a fully minimized bundle.js which we then include in mdbook using include_bytes!(). We'd could also take the opportunity to formalise theming a bit more and document it, which would make creating your own themes easier.

If we minify deps ourselves, we can't benefit from possibly having the deps already in browser cache

I think this could also go down as an advantage. Quite a lot of people have complained about the generated books not being fully available offline because we use CDNs and such (see #511 and the linked issues). If all necessary assets are bundled into the mdbook binary then all generated books are self-contained and we'll be fixing this papercut.

@est31
Copy link
Member

est31 commented Feb 7, 2018

We can easily support Debian's needs by vendoring our dependencies and minifying them ourselves.

Not sure whether adding a build-time dependency to a gigantic JavaScript codebase (which most bundlers are) will make Debian happy, as I don't think these are already packaged by debian. But @infinity0 knows more.

I think the best solution to this would be to be more humble with JavaScript usage. I think right now it is pretty rude how much JavaScript mdbook needs and how much it is requesting. Why does it have to send an XHR request to crates.io???

@sorin-davidoi
Copy link
Contributor

sorin-davidoi commented Feb 7, 2018

I think the best solution to this would be to be more humble with JavaScript usage.

A good idea in general, but there is no way to implement the current feature-set while also removing JavaScript dependencies.

Why does it have to send an XHR request to crates.io???

In order to enable Rust snippets to be executed. I agree that this is not needed for pages without Rust code, so this can be optimized (we can also delay the request until the "Run" button is clicked).

@azerupi
Copy link
Contributor

azerupi commented Feb 7, 2018

What about ditching Stylus altogether? CSS custom properties support everything in RFC 1985 except UC Browser for Android.

Another option is to use Sass instead of stylus. There is sass-rs, a Rust wrapper around libsass. In addition to ditching stylus, it would allow us to easily let users define new themes by creating a new file defining a couple of variables and compile it at runtime into the CSS file.

Maybe that is also possible with CSS and custom properties? I don't know, I haven't used them yet.

@sorin-davidoi
Copy link
Contributor

Maybe that is also possible with CSS and custom properties? I don't know, I haven't used them yet.

Yes, and they can also be tweaked at runtime (example).

@mattico
Copy link
Contributor Author

mattico commented Feb 7, 2018

@Michael-F-Bryan

I agree re: CDNs. I'd be happy just removing them altogether. People have valid concerns, and I'm not sure it's worth keeping an option around just in case your browser has the exact version from the exact CDN cached just to save 100KB of bandwidth.

@est31

I chose webpack over newer/simpler alternatives because it is already packaged in Debian , although it's in Sid, so it can't be in stable until... 2021? So there's a wrinkle I hadn't quite considered. We'd have to use something like YUI-Compressor, in the mean time. The fact that webpack wouldn't help Debian right now diminishes its usefulness in my eyes. I still think it would be useful to have some system to manage assets, even if its just a script or a rust tool.

Since we're pinging @infinity0 I might as well ask for his opinion specifically. This is how I propose that an mdBook build would work for Debian:

  • mathjax feature disabled (not used in Rust docs?)
  • playground feature disabled (can't connect to play.rust-lang.org, therefore no need for any of the editor support)
  • Stylus Not needed since book.css counts as source code. We may end up switching to scss-rs or just CSS anyway.
  • Web fonts disabled (we should add an option for this)

Which leaves the following dependencies:

  • highlight.js
  • clipboard.js
  • elasticlunr.js (soon)
  • mark.js (soon)
  • FontAwesome

The JS dependencies would be vendored. We would then provide a way to either use them as-is, or to minify them using an existing stable Debian package.

If FontAwesome is an issue we can provide a solution for that as well, like just ship the 4 PNGs that we use.

@infinity0
Copy link

Typically a bundler is only needed if the original code is a CommonJS/NodeJS module that don't have support for being used via <script> tags. AFAICT all/most of the existing JS components of mdBook can be used directly as in-browser javascript via <script> tags so bundling them using webpack/browserify really serves no useful purpose. Some of the js files might themselves already be "packed" from original CommonJS modules; mdBook packing these packs would be extra-pointless. Also, MathJax has been available in Debian for years and works fine as a <script> include, it does not need to be disabled nor packed.

If you don't want to serve CDN files, then just have the build script download them and copy them into a local relative path that is available in the webroot so <script> can access it. That would be very easy for Debian and other distros to adapt to use system JS packages instead.

For example, my current patches for mdBook can be seen here. Packing everything together would make these patches much much more awkward.

@mattico
Copy link
Contributor Author

mattico commented Feb 7, 2018

Okay, I'm pretty convinced that webpack-et-al is not the solution for this problem. Rather than try to steer this thread towards some other ideas, I'm going to close this issue. Thank you all for your thoughts.

@sassman
Copy link

sassman commented Dec 22, 2020

There is also https://github.com/connorskees/grass that can compile scss and is a regular rust crate. With this we could improve the css situation specifically themes could be simplified by for example a css framework like bulma.
I wonder if that is a direction the maintainers would accept and support?

I'm working currently on a feature where I want to avoid implement an overlay by hand and myself and rather rely on some foundation that is battle tested. Specifically here scss and a proper framework would save a lot of code and improve the maintainability with avoiding to pull in webpack and other npm packages.

Any thoughts?

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

No branches or pull requests

7 participants