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

Reduce browser bundle size to be comparable with alternative formatters #12144

Open
TimvdLippe opened this issue Jan 25, 2022 · 38 comments
Open
Labels
status:needs investigation Issues that need additional investigation, e.g. to understand whether the reported behavior is a bug type:meta Issues about the status of Prettier, or anything else about Prettier itself

Comments

@TimvdLippe
Copy link

Environments:

  • Prettier Version: 2.5.1
  • Running Prettier via: Browser API
  • Runtime: Chrome 99
  • Operating System: n/a
  • Prettier plugins (if any): parser-babel

Steps to reproduce:

I would like to include Prettier as part of a browser web page. In this case, I am looking at integrating Prettier into Chrome DevTools, which has a feature to automatically format web content in our sources panel. This feature is currently implemented using custom-written formatters (example).

The current formatters rely on the CodeMirror 5 tokenizers, which I am working on removing: https://bugs.chromium.org/p/chromium/issues/detail?id=1287519 As part of that work, I am analyzing the state of affairs of formatters used on the web and which ones we could potentially use. Since Prettier is an active and popular project, it is one of the first I am looking at.

I have a basic version of Prettier integration working, but I quickly discovered a blocker for us: the bundle size of Prettier greatly exceeds the size budget I have available. At Chromium, we have strict size restrictions for our projects, as users install Chrome on low-space devices. If I include only the standalone bundle + babel parser (required for JS), it clocks in at ~900Kb. I am working with a budget of around 100Kb, as we have both our CodeMirror 5 tokenizers as well as custom implementation of formatting.

At the same time, I looked at alternatives such as js-beautify, which is 40Kb in total (25Kb main bundle, 15Kb for css + html), which is well within our budget. However, js-beautify is less active than Prettier, which is why I initially would prefer to adopt Prettier.

FWIW this problem was previously mentioned at #10157 (comment) and I am not sure how much the ES Modules situation would help reducing the bundle size. Looking at the Skypack size, it shows 185Kb network size (https://cdn.skypack.dev/-/prettier@v2.5.1-2KrfQhZNe1CJFznlAR6m/dist=es2019,mode=imports/unoptimized/esm/standalone.mjs), but that is compressed with brotli. If I download the file and obtain the filesize, it is 720Kb. Since Chromium bundles files and then serves it with brotli, our network size would be more manageable, but unfortunately the disk space limitation is calculated on the original uncompressed size. Hence we are looking at the 720Kb file size rather than the 185Kb network size.

Would it be possible to reduce the bundle size of Prettier to be comparable to alternative formatters and make it feasible for Chrome DevTools to adopt Prettier?

Expected behavior:

The Prettier bundle size is manageable, ideally below 100Kb, similar to js-beautify and alternatives.

Actual behavior:

Using only the standalone bundle + babel parser (e.g. not including CSS or HTML, which we also would need) clocks in at ~900Kb.

@fisker fisker added status:needs investigation Issues that need additional investigation, e.g. to understand whether the reported behavior is a bug type:meta Issues about the status of Prettier, or anything else about Prettier itself labels Jan 25, 2022
@fisker
Copy link
Member

fisker commented Jan 25, 2022

Thanks for considering using Prettier as the JS formatted.

Possible way to reduce the file size:

standalone.js: Currently we include all print logic in this file, remove them will reduce the file size, but I'm not sure how big will that part be.

parser-babel.js: There seems no way to reduce the size, unless we remove some babel plugins (means some new syntax will not supported).

Look at all our js parser, the espree parser seems the smallest, but I think there is also no way to make it smaller.

But in meriyah parser, I know there is some code about HTML entries, it's quite big and not used.

Just a quick response, more details need investigation.

@TimvdLippe
Copy link
Author

Thanks for the quick response! I can do a quick peek into standalone.js and see what would be necessary for our use case and see how small we can make it. Thanks for your consideration 😄

@j-f1
Copy link
Member

j-f1 commented Jan 25, 2022

I did a little investigation of the parser-babel file. It contains 40kB of regexes (many of which contain Unicode character escapes — they could probably be shorter if converted to the real UTF-8 characters. There are also at least 22kB of error messages — Prettier can’t change this but it would theoretically be possible to add a compile option to Babel to have it produce generic error messages instead of specific ones. In terms of plugins, there appear to be both TypeScript and Flow plugins. Since Chrome would only need to handle regular JS (I’m assuming source mapped files are not formatted), those could be removed along with JSX and other syntax features not supported in browsers yet.

@j-f1
Copy link
Member

j-f1 commented Jan 25, 2022

Additionally, while variable names have been optimized down to single letters, object properties have remained unchanged. Maybe something like Closure Compiler could identify properties that are not exposed outside of the file and rename them too?

@fisker
Copy link
Member

fisker commented Jan 25, 2022

@TimvdLippe You can run yarn build --file=standalone.js --no-minify to debug with non-minified version.

@TimvdLippe
Copy link
Author

Additionally, while variable names have been optimized down to single letters, object properties have remained unchanged. Maybe something like Closure Compiler could identify properties that are not exposed outside of the file and rename them too?

That's an interesting observation. At DevTools, we recently adopted private class fields, which are minified by default by Terser: terser/terser#780 We have seen a consistent 8-10% bundle size win by enabling this option: https://crrev.com/c/2975285 (we later switched to the default behavior in https://crrev.com/c/3158385/3/scripts/build/rollup.config.js)

@TimvdLippe You can run yarn build --file=standalone.js --no-minify to debug with non-minified version.

Thanks, I will take a look!

@fisker
Copy link
Member

fisker commented Jan 25, 2022

Looking at this file you linked, it seems you already have acorn, if it's going to stay, we don't need parser-babel.js file, we can print ESTree AST.

TimvdLippe added a commit to TimvdLippe/prettier that referenced this issue Jan 25, 2022
By implementing a custom function for assert, we no longer rely on the
Node global polyfill for the assert module. As part of prettier#12144, I
discovered that the node global polyfill is a large contributor to the
file size.

Currently, on main I obtained the following numbers:

```
> yarn build --file=standalone.js --no-minify
> perl -le "map { \$sum += -s } @argv; print \$sum" dist/esm/standalone.mjs
< 1207295

> yarn build --file=standalone.js
> perl -le "map { \$sum += -s } @argv; print \$sum" dist/esm/standalone.mjs
< 562931
```

With this PR, the numbers change to:

```
> yarn build --file=standalone.js --no-minify
> perl -le "map { \$sum += -s } @argv; print \$sum" dist/esm/standalone.mjs
< 1136137

> yarn build --file=standalone.js
> perl -le "map { \$sum += -s } @argv; print \$sum" dist/esm/standalone.mjs
< 530682
```

This is a 5.89% and 5.72% size reduction for respectively unminified and
minified standalone bundle output.
@TimvdLippe TimvdLippe mentioned this issue Jan 25, 2022
1 task
@TimvdLippe
Copy link
Author

I discovered that one of the factors contributing to a large bundle sizes are the node polyfills that are included in the browser bundle. #12145 is a first step in the direction to remove some of these polyfills, which on their own already constitute nearly 6% size reduction. The other polyfill usages should hopefully be possible as well, so I will be working on that next.

@j-f1
Copy link
Member

j-f1 commented Jan 25, 2022

We should also be able to remove buffer (48.5kB) from the standalone bundle.

@TimvdLippe
Copy link
Author

@j-f1 Yes I spotted that as well, but it wasn't clear to me yet where that is coming from. I think it is coming from the node polyfills, but I am still attempting to dig deeper what is pulling them in.

@j-f1
Copy link
Member

j-f1 commented Jan 25, 2022

I also found https://github.com/btd/esbuild-visualizer, which can produce a visualization of bundle size (main...j-f1:esbuild-visualizer).

You’ll need to run a shell script to make The HTML files, though.

# fish (bash should be similar)
for name in dist/*.meta.json
  npx esbuild-visualizer --metadata $name --filename $name.html &
end

@TimvdLippe
Copy link
Author

@j-f1 Oh that is helpful! Interestingly enough, it claims that certain files are included, but I am not sure they are. E.g. in the build process, the parsers.js from the various languages are set to undefined, but the visualizer claims they are still used. It is not clear how the mapping works, but it is a good tool to get clearer where the biggest wins are.

@TimvdLippe
Copy link
Author

For reference, with the inclusion of both #12145 and #12146, this is the current bundle layout: EsBuild Visualizer.pdf.

@fisker
Copy link
Member

fisker commented Jan 25, 2022

You can remove other languages from here to see the size with js printer only

// JS
require("./language-js/index.js"),
// CSS
require("./language-css/index.js"),
// Handlebars
require("./language-handlebars/index.js"),
// GraphQL
require("./language-graphql/index.js"),
// Markdown
require("./language-markdown/index.js"),
// HTML
require("./language-html/index.js"),
// YAML
require("./language-yaml/index.js"),

@fisker fisker mentioned this issue Jan 27, 2022
4 tasks
@fisker
Copy link
Member

fisker commented Jan 27, 2022

@TimvdLippe Any comment on this? I'm going to add support for acorn parser.

@fisker fisker mentioned this issue Jan 27, 2022
4 tasks
@TimvdLippe
Copy link
Author

@fisker Yeah that would be great! In our case, given that we have Acorn in a special location and don't use CommonJS, I think we can integrate our custom parser? But based on https://github.com/prettier/prettier/pull/12172/files#diff-114bc4621fa54fbb70ff0442c6dbc427a114dca4cfcf798f52c11fa5493f23f3 that should be relatively straightforward.

With regards to #12144 (comment) I made that change, but sadly it does not reduce the bundle size. I am not sure what is going on, but looking at the visualization report posted on #12144 (comment) I wonder if we can split up these language printers?

E.g. can we put these language printers in separate bundles that we can import with dynamic import (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#dynamic_imports)? That way, we can choose ourselves what to load, just like the parsers.

@fisker
Copy link
Member

fisker commented Jan 27, 2022

I wonder if we can split up these language printers?

We can, but it's too hard for other users to use the standalone version of Prettier, it will requires user

import prettier from 'prettier/standalone';
import jsPrinter from 'prettier/js-printer';
import babelPlugin from 'prettier/parser-babel';

prettier.format('', {plugins: [jsPrinter, babelPlugin]})

They may use more languages.

@TimvdLippe
Copy link
Author

@fisker I see. In that case, can we preserve the existing default standalone bundle, but vendor separate chunks that users (like Chrome DevTools) can use if file size is a concern? The standalone version could then even reuse the new chunks and automatically configure all plugins for users so that the default experience is still easy to consume.

@fisker
Copy link
Member

fisker commented Jan 27, 2022

With regards to #12144 (comment) I made that change, but sadly it does not reduce the bundle size.

I tested, comment out those lines reduces the file size.

$ yarn build --file=standalone.js --report --print-size
- standalone.js...................................................510 kB    DONE
+ standalone.js...................................................372 kB    DONE

@TimvdLippe
Copy link
Author

Ah yes now I can verify these numbers. Strange, I couldn't before.

Sadly Prettier + JS + CSS + HTML printers is around 484 kB (which is a lot better than the original 900kB!), but still a bit above our budget of 100kB.

I spotted a couple of more things:

  1. The single include of @babel/code-frame pulls in nearly 30kB:
    const { codeFrameColumns } = require("@babel/code-frame");
  2. The single include of vnopts pulls in nearly 40kB:
    const vnopts = require("vnopts");
    (I wonder if we can make this part of the CLI instead, alike Move CLI related logic from normalizeOptions into src/cli #12174)
  3. The single include of esutil pulls in nearly 30kB:
    const isIdentifierName = require("esutils").keyword.isIdentifierNameES5;
    Since we only use isIdentifierNameES5 (which is a pretty small function https://github.com/estools/esutils/blob/a825f91fd1d1e3a9fff84227cb06c011d8a0b9e8/lib/keyword.js#L97-L114), can we copy the function and avoid the dependency on esutils? I think esutils is not tree-shakeable, which is why it is pulling in so much code.

With these improvements, the bundle is down from 484kB to 385kB, so another 100kB off, which seems pretty good progress.

@fisker
Copy link
Member

fisker commented Jan 27, 2022

@TimvdLippe
Copy link
Author

Thanks for the additional context. That made me realize that DevTools might need fewer pieces of Prettier than regular users, since we control the plugins and parser ourselves. Therefore, if we can hook Acorn into Prettier and only use the printer, I think we already cover the use case of DevTools: "Given a well-formed file, format it".

To that extent, we would happily ship 50-100 lines of glue code to hookup Acorn/other parsers with Prettier and call the relevant printers and retrieve the end result. Would it therefore be possible to have a separate bundle for the printers that we could import? In that case, how much work do you think it would be to hook these two components together with a minimal amount of options/plugins overhead from Prettier?

@fisker
Copy link
Member

fisker commented Jan 27, 2022

We already have this

formatAST(ast, options) {

  • skip option normalize
  • skip range format
  • remove ts/flow/jsx(they are all in seperate files) print logic
  • remove multiple parser support

I think that's all you need.

@fisker
Copy link
Member

fisker commented Jan 27, 2022

I think we can add a special entry for your case, all you need pass is:

  1. the source code
  2. acorn module

You can still use all our options, but we won't verify them.

@TimvdLippe
Copy link
Author

Ah that's good to know. I can tinker next week and see whether it is possible to create a bundle for that (or what would be necessary to make it possible to extract the function with tree-shaking). I wonder whether adopting ES modules (which you were already planning in #10157) would allow us to import only that function and Rollup would tree-shake away all other functionality that we don't use. That would prevent a separate bundle, but would require the standalone bundle to be tree-shakeable (which afaik is not possible with CommonJS, is it?).

@fisker
Copy link
Member

fisker commented Jan 27, 2022

I'll be on vacation(Chinese new year) next week, maybe @sosukesuzuki too? Good luck!

@sosukesuzuki
Copy link
Member

I don't have any vacation plans. So I can help you 👍

@daKmoR
Copy link

daKmoR commented Jan 29, 2022

  • @babel/code-frame is used to display error message, we need that, unless you don't want point where the error is, and build special version for you.

this dependency is also one of the bigger "chunks" in Web Dev Server and I look at it at some point in the past...
it's rather small itself but it has a dependency on babel-highlight (also small) but it has a dependency on chalk which is ~20kb (minified)

Dependency graph of @babel/code-frame visualized

replacing chalk with for example colorette could save around ~16kB

Is anyone up for starting a discussion at the babel repo?


"Same story" for vnopt => it's only 20kb by "itself" but uses chalk... so it's 40kb

the issue with chalk is that it's everywhere... so to get the size bonus you need to replace it everywhere which becomes an organizational nightmare 😅

--

edit: collorette is not 1kb but 3.8kb minified => code save would be ~16kB and not ~19kB as previously written

@TimvdLippe
Copy link
Author

Thanks @daKmoR for the suggestion. I opened babel/babel#14213 to that extent 😄

@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Jan 30, 2022

Babel maintainer here 👋

Unfortunately we cannot remove chalk from @babel/code-frame until Babel 8, because it's part of the public API. However, I'm experimenting with making it unused and easily tree-shakable. Does your bundle process support tree-shaking CommonJS files?

Specifically, something like this:

// @babel/code-frame

const highlight = require("@babel/highlight");

module.exports = () => highlight.red();
// @babel/highlight

exports.red = () => "RED";

exports.getChalk = function () {
  return require("chalk");
};

would it tree-shake chalk away? (I know that rollup supports some CommonJS tree-shaking, but I don't know what exactly).

@fisker
Copy link
Member

fisker commented Jan 30, 2022

Chalk is actually used, but we decide to remove it from standalone build(CLI still need colors). And it's done. #12162 and #12182

@nicolo-ribaudo
Copy link
Contributor

And you don't load @babel/code-frame in the standalone build?

@fisker
Copy link
Member

fisker commented Jan 30, 2022

And you don't load @babel/code-frame in the standalone build?

We do, but https://github.com/prettier/prettier/blob/main/scripts/build/shims/babel-highlight.cjs

@nicolo-ribaudo
Copy link
Contributor

Oh awesome, thanks 👍

@fisker
Copy link
Member

fisker commented Jan 30, 2022

Is anyone up for starting a discussion at the babel repo?

We don't really care about what babel use, we use Chalk ourself.

@TimvdLippe
Copy link
Author

While debugging the standalone.mjs output, I discovered that the usage of process.env.PRETTIER_TARGET is introducing unintended file bloat. I was able to extract a standalone reproduction case for esbuild:

function main() {
  if (true) {
    throw new Error('something')
  }
  try {
    console.log('something else');
  } catch (e) {
    // ignored
  }
}

main();

Here, the if (true) comes from

if (process.env.PRETTIER_TARGET === "universal") {
which is correctly substituted in the build process. However, since the next statement in the function is a try-catch, esbuild doesn't properly remove it. If you were to write the following, it does correctly remove the console.log:

function main() {
  if (true) {
    throw new Error('something')
  }
  console.log('something else');
}

main();

This is reported at esbuild at evanw/esbuild#1317 and listed in their caveats of https://esbuild.github.io/api/#minify-considerations

As a result of this behavior, main/parser.js is pulling in some Node globals, which would otherwise be unused.

Therefore, would it maybe make sense to move to Terser for minification (https://terser.org/)? Unfortunately, it doesn't appear to be possible to combine Terser and esbuild (https://esbuild.github.io/plugins/#plugin-api-limitations), but maybe we can use esbuild during dev and use Rollup + terser for production builds? We are using Terser at Chrome DevTools and have seen significant size reductions/tree-shaking capabilities compared to other tools.

@fisker
Copy link
Member

fisker commented Jan 31, 2022

We won't switch wo rollup, because esbuild is much faster, but we can add one extra terser call, if you are interest in minify check this #12184

@TimvdLippe
Copy link
Author

With regards to the suggestion in #12144 (comment), I made the following (temporary) changes:

  1. Change src/standalone.js to only export formatAST: core.formatAST,. This is mostly for debugging purposes, to see what we would need for a bundle that only has 1 API exposed
  2. Comment out all require-statements in src/languages.js
  3. Run yarn build --file=esm/standalone.mjs --no-babel --print-size:
$ node ./scripts/build/build.mjs --file=esm/standalone.mjs --no-babel --print-size
 Building packages 
  esm/standalone.mjs...........................................95.4 kB    DONE  
Bundled packages licenses not included in `dist/LICENSE`.
Done in 0.43s.

That's below 100 kB and a massive drop (compared to 400 kB) 🎉 This is definitely getting closer to what we are looking for!

There are some more optimizations possible, most notably those related to how options are handled. The assumption here is that in a new bundle, it would assume all option to be provided and doesn't normalize anything. If I change normalizeOptions in src/main/options-normalizer.js and comment out the const vnopts = require("vnopts") from the top, I get these results:

$ node ./scripts/build/build.mjs --file=esm/standalone.mjs --no-babel --print-size
 Building packages 
  esm/standalone.mjs...........................................76.5 kB    DONE  
Bundled packages licenses not included in `dist/LICENSE`.
Done in 0.41s.

The caveat with this approach is that I disabled all languages, to get a baseline value. If I re-enable the language-js require in src/languages.js, I get the following:

$ node ./scripts/build/build.mjs --file=esm/standalone.mjs --no-babel --print-size
 Building packages 
  esm/standalone.mjs............................................255 kB    DONE  
Bundled packages licenses not included in `dist/LICENSE`.
Done in 0.46s.

Therefore, we are getting closer to feasibility, but there are some further cleanups required to get below 100kB officially. For that to work, I think migrating to ES Modules (#10157) is the biggest bang for the buck, given that it should (if I understand correctly) allow for more granular tree-shaking with esbuild.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs investigation Issues that need additional investigation, e.g. to understand whether the reported behavior is a bug type:meta Issues about the status of Prettier, or anything else about Prettier itself
Projects
None yet
Development

No branches or pull requests

6 participants