Skip to content

Add minification for CSS and JS files #2728

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 12, 2025

I'll add some comparison numbers when I have time.

file before after diff
chrome.css 16535 12014 -37.4%
general.css 6531 4122 -36.9%
print.css 661 505 -23.6%
variables.css 8836 6735 -23.8%
book.js 28707 17964 -37.4%

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Jun 12, 2025
@GuillaumeGomez
Copy link
Member Author

I added comparisons as well in the first comment.

@GuillaumeGomez GuillaumeGomez requested a review from ehuss June 12, 2025 19:28
@ehuss
Copy link
Contributor

ehuss commented Jun 21, 2025

Thanks! I had a few questions:

  • This isn't like a typical minifier that renames and does other compression, correct?
  • Would there be any benefit to having source maps, or some plan to getting those in the future?
  • Should this include searcher.js? Or mode-rust.js?
  • I've been wondering about combining files. Is that an option for the future? For example, there are a bunch of CSS files, would it be possible to combine them somehow? I've often wondered, as it seems like it would be pretty inefficient to have a bunch of network requests for several small files.
  • The debugging experience seems to be hindered a little by this. I'm wondering if it would be possible to turn this off while using mdbook serve?

@GuillaumeGomez
Copy link
Member Author

This isn't like a typical minifier that renames and does other compression, correct?

Indeed, it's just to remove unneeded characters (so mostly whitespace) and comments (while keeping the license). Anything more advanced would require a full JS parser, which I plan to add "some day".

Would there be any benefit to having source maps, or some plan to getting those in the future?

Never used source map so I can't answer this. If you used source maps, do you think it would be worthwhile to have them?

Should this include searcher.js? Or mode-rust.js?

searcher.js yes, but mode-rust.js seems to be already minified?

I've been wondering about combining files. Is that an option for the future? For example, there are a bunch of CSS files, would it be possible to combine them somehow? I've often wondered, as it seems like it would be pretty inefficient to have a bunch of network requests for several small files.

I suppose we could. First we need to check if some files are always on the same pages. In this case, we can merge their source code directly. However if some files are not always on the same pages, then either we generate different CSS files which include some CSS files. If there is a possibility here, it would reduce the number of files, so I think it'd be a very good thing to do.

The debugging experience seems to be hindered a little by this. I'm wondering if it would be possible to turn this off while using mdbook serve?

There isn't currently but we can definitely add an option to not run minimization (although, since we have the "pretty format" feature in all web browsers, it's mostly ok imo). Gonna add it in this PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants