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

rustdoc: use webpack to bundle code #51735

Closed
ishitatsuyuki opened this issue Jun 23, 2018 · 24 comments
Closed

rustdoc: use webpack to bundle code #51735

ishitatsuyuki opened this issue Jun 23, 2018 · 24 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ishitatsuyuki
Copy link
Contributor

webpack is the most popular script bundler for JavaScript and other assets.

With webpack, we can benefit from:

  • Transpilation of ES6 or TypeScript
  • Code splitting/dynamic imports
  • Effortless code minification
  • Eliminating the need of resource-suffix with chunk hash

However, this adds the requirement of Node.js + NPM to the build dependencies. We need to care about:

  • How to execute webpack
  • How to bundle (vendor) node_modules

Below are some implementation notes:

  • In rustdoc's build scripts we run webpack and then generate the code for including into the binary. Example:
resources.insert("bundle.ef0d81c.js", include_bytes!(concat!(env!("OUT_DIR"), "bundle.ef0d81c.js")));
@ishitatsuyuki ishitatsuyuki added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jun 23, 2018
@est31
Copy link
Member

est31 commented Jun 23, 2018

related thread for mdbook rust-lang/mdBook#610

@QuietMisdreavus
Copy link
Member

cc @rust-lang/rustdoc for changing how we package JavaScript, and @rust-lang/infra and @rust-lang/release for the issue of wanting to add a new build dependency.

As for resource-suffix, that was added so that docs.rs could have a per-release suffix it could use to aggregate rustdoc's static files into a global cache. If you want to remove --resource-suffix, you'll need to work the new build hash into there.

@QuietMisdreavus QuietMisdreavus added A-build T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Aug 23, 2018
@BatmanAoD
Copy link
Member

I'm not sure I understand; is this just a request to add a recommendation to the Rust documentation to use webpack to bundle and deploy WASM code? Or are we talking about changing the behavior of the wasm32-unknown-unknown target, and adding Node.js and NPM as dependencies for using that target?

@QuietMisdreavus
Copy link
Member

If i'm reading the request right, it's asking us to bundle our js/css using webpack instead of just packing/minifying the files by hand. However, it seems that to do that properly, we'll need to include nodejs and some npm dependencies to do the packaging, which means that those now become full build dependencies for rustdoc, rather than just dependencies for some of its tests.

@BatmanAoD
Copy link
Member

@QuietMisdreavus Oh, I think I understand now; I thought this was changing documentation, but it's about changing the rustdoc tool itself.

@XAMPPRocky
Copy link
Member

I wonder if instead of webpack, because webpack is really a lot to manage and maintain even in the simplest of projects, we could use Parcel. This would still add NPM & Node, but we wouldn't have to really worry about build configuration.

Parcel would allow us to use some more advanced stuff in web development such as TypeScript, ES6, Sass, without adding the maintenance of cost of those dependencies. It even has builtin support for Rust to WASM.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Aug 24, 2018

I'm against adding any non rust dependency, especially NPM and Node.js.

@GuillaumeGomez
Copy link
Member

PS: I'm REALLY against adding JS into the Rust build process. It's very annoying for any rust contributor to add such a useless burden. We spent a very huge amount of time trying to get rid of any non-rust part because it was too difficult to maintain (and fix in case of bugs) and I clearly don't see the gain in here. It'd be awesome to have better tools in order to have a better JS/CSS output, and I think we should work on rust tools.

@ishitatsuyuki
Copy link
Contributor Author

I'm REALLY against adding JS into the Rust build process. It's very annoying for any rust contributor to add such a useless burden.

I guess you're trying to express the hate for JavaScript, but your words sound exaggerating. JavaScript is not useless, and there are many people that are familiar with NPM. I agree with you that installing Node.js for first comers requires some effort, but it's no harder compared than Rust.

We spent a very huge amount of time trying to get rid of any non-rust part because it was too difficult to maintain (and fix in case of bugs) and I clearly don't see the gain in here.

Certainly, NPM packages are not as well made as Rust partly due to its dynamically typed nature. However, the popular ones are certainly useful for the right job.

You also didn't give any concrete examples regarding maintenance being hard, but in some cases you've got no alternatives. LLVM is the case, where you need C++ compiler and some additional build tools, and that's not getting replaced in near future.

It'd be awesome to have better tools in order to have a better JS/CSS output, and I think we should work on rust tools.

What you are saying is basically "Rewrite it in Rust", which is not the first thing we should do. Tools in the JavaScript ecosystem gets a lot of commits, and achieving the same level in Rust isn't a easy thing.

@GuillaumeGomez
Copy link
Member

I'm REALLY against adding JS into the Rust build process. It's very annoying for any rust contributor to add such a useless burden.
I guess you're trying to express the hate for JavaScript, but your words sound exaggerating. JavaScript is not useless, and there are many people that are familiar with NPM.

I have wrote way too much JS for rustdoc to be put in JS-hater case I think. ;)

I agree with you that installing Node.js for first comers requires some effort, but it's no harder compared than Rust.

Clearly, you underestimate how fast JS world is moving. A good example is bower. It got deprecated whereas some time ago it was THE package manager to use. If we can avoid having to go through last JS trends and then moving to the next one, I'd be very happy.

Certainly, NPM packages are not as well made as Rust partly due to its dynamically typed nature. However, the popular ones are certainly useful for the right job.

They are... until they get deprecated because a new more trending one got out.

You also didn't give any concrete examples regarding maintenance being hard, but in some cases you've got no alternatives. LLVM is the case, where you need C++ compiler and some additional build tools, and that's not getting replaced in near future.

Just in case of rustdoc, we had the hoedown case. I made a (few? don't remember...) PR to fix some bugs and it never got merged. Also, maintaining C code into rust was very problematic (and still is).

What you are saying is basically "Rewrite it in Rust", which is not the first thing we should do. Tools in the JavaScript ecosystem gets a lot of commits, and achieving the same level in Rust isn't a easy thing.

And so is any big language, I don't see any proposal for Java tools for example.

In here, my point is: the impact on Rust would be huge for not much gain.

@XAMPPRocky
Copy link
Member

Clearly, you underestimate how fast JS world is moving. A good example is bower. It got deprecated whereas some time ago it was THE package manager to use. If we can avoid having to go through last JS trends and then moving to the next one, I'd be very happy.

I think this is condescending and completely unneeded in this discussion, anecdotal experiences don't help make this decision. Bower was released in 2012 and deprecated in 2017 and is still maintained, by that definition Cargo is more unstable than Bower.

And so is any big language, I don't see any proposal for Java tools for example.

We aren't using Java anywhere, we are using JavaScript and are stuck with it for the foreseeable future, so a proposal about using tooling for it is warranted.

They are... until they get deprecated because a new more trending one got out.

This is "JavaScript is completely experimental" argument is a straw man as the issue hasn't proposed anything that could be called experimental. Webpack has been in development for 6 years, NPM has 8 years behind it, Node has 9. ES6 has been frozen for 3 years. TypeScript is older than Rust 1.0 by nearly 3 years. This kind of rhetoric is completely unhelpful to actually making decisions about our infrastructure.

@cuviper
Copy link
Member

cuviper commented Aug 24, 2018

A nodejs build dependency would be a significant challenge for older platforms like RHEL6. Even RHEL7 might be a challenge, but I'd have to see what's actually required.

But as a compromise, maybe the generated webpack bundle could be included in the source tarball? (Similar to how we handle src/vendor, enabling offline builds.) This way packagers won't need to worry about it.

@QuietMisdreavus
Copy link
Member

@GuillaumeGomez I think the comparisons with the JavaScript ecosystem in general are not warranted. As mentioned, Rust is still newer than some of these efforts anyway, so we have no useful point of comparison. That said, we did just have a conversation on Discord to dig into this proposal a bit more, so let me try to add some points:

  • As far as i understand, adding Webpack, Parcel, or some other build tool to rustdoc's JavaScript would allow us to bundle and minify our JS/CSS/font/image assets to make them smaller for distribution.
    • However, per @GuillaumeGomez on Discord, we can't do this for the theme CSS, because the way we switch themes requires them to be in separate files. And we can't do them for any JS but main.js, because the storage/setting files need to be loaded separately, and first. Will these build tools still allow us to do this?
  • While i'm sure an ES6/TypeScript compiler could ensure this for us, we need to ensure that rustdoc's assets are compatible with our tiered browser support, if not way farther back than that. In addition, i would say that an unstated goal in rustdoc's JS is to not require external libraries or extra packages. Ever since we got rid of jQuery last year, all the JS we've used has been defined in this repo.
  • Per our conversation on Discord, @GuillaumeGomez believes that rustdoc's JavaScript is not big enough to warrant bundling up and zipping up, any more than his own minifier crate which is already being applied to some JS assets.
  • Changing how we store/emit our assets will probably break docs.rs, too, since it keeps a cache of assets per-release, separate from the generated HTML. (It also keeps the font and image files in a universal cache, since those barely change at all.) Unless i'm misunderstanding what a bundler tool would do, if we have everything packed all together, that processing would need to change before it could handle the new packing strategy.
  • It cannot be overstated that this would add a new build dependency for building and distributing Rust. For me, this would require a clear, strong benefit before i would be willing to commit to something like that. As @cuviper mentioned, this could impact platforms which are not otherwise equipped to handle it. Based on the other points i mentioned, i'm not convinced this is worth adding.

@est31
Copy link
Member

est31 commented Aug 24, 2018

But as a compromise, maybe the generated webpack bundle could be included in the source tarball?

@cuviper this might mean problems for the likes of debian who want to ship source code in its "preferred form of modification", which pre-minified js code isn't. See this thread: rust-lang/mdBook#495

@shepmaster
Copy link
Member

I don't have an opinion one way or the other, but I can fill in some technical details...

the way we switch themes requires them to be in separate files [...] Will these build tools still allow us to do this?

I believe the term here is "code splitting" and the playground does this. Here's the network traffic for 3 different theme selections:

image

assets are compatible with our tiered browser support,

The playground uses a browserslist to automatically maintain compatibility.

to not require external libraries

By bundling them, they are no longer "external"

or extra packages

but no help here. I'm not sold on this benefit ("what do you mean Rust doesn't have random number generation in the standard library?!?!"), but I'm not maintaining this code.

@shepmaster
Copy link
Member

With webpack, we can benefit from:

@ishitatsuyuki can you explain why you opened this issue to start with? There are certainly benefits possible, but why do we need them? Does someone want to write ES6 or TypeScript? Is something insufficient about our current tooling?

We can spend a lot of time arguing back and forth on the abstract benefits, but I'm curious to know what the concrete benefits are to the Rust project today.

@ishitatsuyuki
Copy link
Contributor Author

Can you explain why you opened this issue to start with?

My primary motivation is to be able to require() dependencies instead of cluttering them with tags in HTML and/or committing minified version into repository.

Also that the ability of import/export, which allows to break up the code into files easier. In some place we have a ridiculous amount of indent, which makes the code hard to read (harder to match parentheses).

Does someone want to write ES6 or TypeScript?

ES6: I want, and probably others do. We already have some manual polyfills:

if (!String.prototype.startsWith) {
String.prototype.startsWith = function(searchString, position) {
position = position || 0;
return this.indexOf(searchString, position) === position;
};
}
if (!String.prototype.endsWith) {
String.prototype.endsWith = function(suffix, length) {
var l = length || this.length;
return this.indexOf(suffix, l - suffix.length) !== -1;
};
}

And ES6/TypeScript is a better language to work with, because there are more standard library helpers or language features. const/let is a more strict notation, arrow functions are more like lambda, Promise is simply nice.

TypeScript: I am a fan of TypeScript because it avoids errors you could made with Vanilla JS, but it adds some complexity to build process so I'm not sure if others like it.

@XAMPPRocky
Copy link
Member

It's worth noting that according to RFC 1985 and the ES6 compatibility table that we should be able to use ES6 directly. The only parts not supported across browsers is tail call optimisation and non ascii identifiers. The one caveat is that I couldn't find a definitive list of what UCAndroid does and doesn't support. The most I could find was it's ES6 support on caniuse.com which is hardly definitive.

@shepmaster
Copy link
Member

My primary motivation
I want
I am a fan

Are you stating that you personally want to work on the documentation JavaScript but are prevented from doing so because the language support / tooling is too out of date for you?

Perhaps you can share how you would have improved rustdoc if the improved tooling was already in place?

require() dependencies instead of cluttering them with tags
committing minified version into repository
ability of import/export
arrow functions
Promise is simply nice

These are all potential benefits, but...

  • what concrete benefits will be provided to the end consumers of the documentation?
  • what concrete benefits will be provided to the rustdoc maintainers?

It would be lovely if you could point to some bugs that have been caused by the current outdated tooling, or features that we want which would simply be too complicated to add in the current state.

I doubt that people have filed GitHub issues saying they don't want to work in this area because of the tooling, but maybe there are IRC / forum / Reddit posts that say the same thing?

@shepmaster
Copy link
Member

A bit tangential, so I split to a different comment...

In some place we have a ridiculous amount of indent

I don't know the details here, but extracting functions to reduce indent should not be predicated on the build process or upgrading to ES6.

const/let is a more strict notation

There's a surprising amount of contention about const vs let since const doesn't mean what most people intuitively think it should; it's not like Rust's immutable:

const names = [];
names.push(1); // OK

@shepmaster
Copy link
Member

And to play both sides of this conversation...

I originally rewrote the playground with new technologies (Docker, React, Webpack) because there was a problem with the original playground that I didn't care to learn how to fix in the old stack. I also did it to better learn these new technologies.

Later on, I moved to TypeScript to address some programming logic bugs and also to learn TypeScript.

However, my playground added crates — something useful to end users. It was easier for me to do this with the new technology.

I am also more inclined to maintain the playground because of the technology stack. This doesn't appear to be an issue for the current maintainers of Rustdoc.

@QuietMisdreavus
Copy link
Member

As far as what i, personally, would get out of upgrading our JavaScript setup:

I do not have much experience with JavaScript at all. I have a basic idea of the overall syntax and structure, and i know where to look to find specific documentation for certain items. But i am still basically a JavaScript novice, when it comes to writing it. This means that if we start using newer/shinier features in ES6, or if we start converting into TypeScript, i will personally be starting from scratch.

I'm sure it will be nicer from a code structure perspective, but that's what i feel like the net effect would be on me, personally. However, if doing this gets us more maintainers who have a better understanding of the language, and it helps us to make that script more understandable to more people, then it may counter-balance that. But that's my main metric that will change my mind: if people come in here and say they will start contributing to rustdoc if/when we switch to TypeScript or the like.

@JohnHeitmann
Copy link
Contributor

While I'm a big +1 on TypeScript, there are a handful of smaller conservative steps we can take that walk the codebase towards friendliness and safety without requiring a build. All are directly on the road to TypeScript.

First, a modern linter. Currently there's a bit of config for jslint [1], but there are better linters like eslint that can handle modern js, and also catch some more interesting semantic issues. I have a PR [2] that configures eslint, and exposes at least one bug (95% sure at this point), and a couple handfuls of dodgy code.

Second, perform minor language construct upgrades. Some of the dodgy code that the linter identifies (and that confuses me in practice reading the code) are places where vars are redefined in a deeply nested context. I don't see any bugs from those, but I've had to stare hard a few times. Switching var to let and doing some minor renaming will help a lot here. I don't care much about const vs let. I make the distinction in my code, but it doesn't add much value.

Third, once the UC Android browser supports modules via the script tag we can use modules natively without a build step. Before then we can do a little bit of plain old refactoring to tease apart the major bits of main.js.

I'll start making some of these changes very incrementally, and if it starts making folks anxious then it's easy to stop at any point. Most of the PRs are going to look like trivial search & replaces, so I don't think folks who haven't seen es6 or TypeScript before will be too taken aback.

[1] https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/static/main.js#L13
[2] #56523

@jonas-schievink jonas-schievink removed the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Apr 21, 2019
@jonas-schievink jonas-schievink added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed A-build labels Apr 21, 2019
@Mark-Simulacrum
Copy link
Member

I think we've basically decided that we're not doing this and further discussion is best suited for internals and such (at least until we build consensus that this is worthwhile considering the hefty dependency on node js).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests