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.

Sign up for GitHub

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

Jump to bottom

Swap uglify for terser #2873

Closed
wants to merge 2 commits into from
Closed

Swap uglify for terser #2873

wants to merge 2 commits into from

Conversation

NullVoxPopuli
Copy link

While working on: #2868
I noticed that the existing build system does not support const or let.

This PR removes uglify, which does not support ES6+, and replaces it with terser via https://www.npmjs.com/package/gulp-terser

Depending on Prism's browser support policy this will force a breaking change.

This PR is split into two commits:

  1. the meaningful changes
  2. npm run build

@github-actions
Copy link

JS File Size Changes (gzipped)

A total of 95 files have changed, with a combined diff of +466 B (+0.5%).

file master pull size diff % diff
components/prism-abnf.min.js 484 B 485 B +1 B +0.2%
components/prism-agda.min.js 482 B 492 B +10 B +2.1%
components/prism-apex.min.js 1.1 KB 1.1 KB +2 B +0.2%
components/prism-asciidoc.min.js 1.62 KB 1.62 KB -1 B -0.1%
components/prism-batch.min.js 653 B 653 B 0 Bytes 0%
components/prism-coq.min.js 1.6 KB 1.6 KB +2 B +0.1%
components/prism-core.min.js 3.08 KB 3.21 KB +129 B +4.2%
components/prism-cpp.min.js 1.19 KB 1.19 KB +2 B +0.2%
components/prism-csharp.min.js 2.34 KB 2.36 KB +14 B +0.6%
components/prism-dataweave.min.js 507 B 515 B +8 B +1.6%
components/prism-diff.min.js 400 B 401 B +1 B +0.3%
components/prism-django.min.js 567 B 569 B +2 B +0.4%
components/prism-docker.min.js 701 B 704 B +3 B +0.4%
components/prism-dot.min.js 604 B 605 B +1 B +0.2%
components/prism-ejs.min.js 262 B 263 B +1 B +0.4%
components/prism-elixir.min.js 844 B 846 B +2 B +0.2%
components/prism-erb.min.js 267 B 269 B +2 B +0.7%
components/prism-etlua.min.js 230 B 232 B +2 B +0.9%
components/prism-factor.min.js 3.54 KB 3.54 KB +1 B +0.0%
components/prism-false.min.js 260 B 272 B +12 B +4.6%
components/prism-ftl.min.js 799 B 805 B +6 B +0.8%
components/prism-gherkin.min.js 5.16 KB 5.16 KB 0 Bytes 0%
components/prism-groovy.min.js 898 B 900 B +2 B +0.2%
components/prism-haml.min.js 779 B 780 B +1 B +0.1%
components/prism-handlebars.min.js 468 B 469 B +1 B +0.2%
components/prism-http.min.js 660 B 666 B +6 B +0.9%
components/prism-icu-message-format.min.js 828 B 829 B +1 B +0.1%
components/prism-java.min.js 991 B 993 B +2 B +0.2%
components/prism-javadoc.min.js 601 B 602 B +1 B +0.2%
components/prism-javadoclike.min.js 496 B 498 B +2 B +0.4%
components/prism-jq.min.js 638 B 639 B +1 B +0.2%
components/prism-js-extras.min.js 1 KB 1.01 KB +2 B +0.2%
components/prism-js-templates.min.js 1.17 KB 1.18 KB +14 B +1.2%
components/prism-jsdoc.min.js 639 B 640 B +1 B +0.2%
components/prism-jsx.min.js 957 B 961 B +4 B +0.4%
components/prism-latte.min.js 522 B 528 B +6 B +1.1%
components/prism-lilypond.min.js 603 B 604 B +1 B +0.2%
components/prism-lisp.min.js 995 B 995 B 0 Bytes 0%
components/prism-llvm.min.js 374 B 385 B +11 B +2.9%
components/prism-markdown.min.js 1.76 KB 1.77 KB +11 B +0.6%
components/prism-markup-templating.min.js 559 B 553 B -6 B -1.1%
components/prism-markup.min.js 1.03 KB 1.03 KB +3 B +0.3%
components/prism-mongodb.min.js 1.47 KB 1.47 KB +2 B +0.1%
components/prism-naniscript.min.js 779 B 783 B +4 B +0.5%
components/prism-nginx.min.js 388 B 389 B +1 B +0.3%
components/prism-parigp.min.js 500 B 502 B +2 B +0.4%
components/prism-pascaligo.min.js 677 B 682 B +5 B +0.7%
components/prism-php.min.js 1.95 KB 1.95 KB +2 B +0.1%
components/prism-powershell.min.js 1.25 KB 1.25 KB +1 B +0.1%
components/prism-pug.min.js 950 B 952 B +2 B +0.2%
components/prism-pure.min.js 1.68 KB 1.68 KB +1 B +0.1%
components/prism-qml.min.js 600 B 602 B +2 B +0.3%
components/prism-qsharp.min.js 1.01 KB 1.01 KB +3 B +0.3%
components/prism-robotframework.min.js 598 B 599 B +1 B +0.2%
components/prism-rust.min.js 1.16 KB 1.16 KB +2 B +0.2%
components/prism-sas.min.js 3.02 KB 3.02 KB +2 B +0.1%
components/prism-scheme.min.js 1.63 KB 1.64 KB +8 B +0.5%
components/prism-shell-session.min.js 481 B 482 B +1 B +0.2%
components/prism-smarty.min.js 607 B 610 B +3 B +0.5%
components/prism-sml.min.js 770 B 770 B 0 Bytes 0%
components/prism-soy.min.js 798 B 801 B +3 B +0.4%
components/prism-t4-templating.min.js 403 B 406 B +3 B +0.7%
components/prism-textile.min.js 1.16 KB 1.16 KB +2 B +0.2%
components/prism-toml.min.js 488 B 490 B +2 B +0.4%
components/prism-tt2.min.js 641 B 643 B +2 B +0.3%
components/prism-xml-doc.min.js 193 B 192 B -1 B -0.5%
components/prism-xquery.min.js 1.66 KB 1.65 KB -3 B -0.2%
components/prism-yaml.min.js 855 B 856 B +1 B +0.1%
components/prism-zig.min.js 1.15 KB 1.15 KB 0 Bytes 0%
plugins/autolinker/prism-autolinker.min.js 595 B 598 B +3 B +0.5%
plugins/autoloader/prism-autoloader.min.js 2.21 KB 2.21 KB +5 B +0.2%
plugins/command-line/prism-command-line.min.js 944 B 973 B +29 B +3.1%
plugins/copy-to-clipboard/prism-copy-to-clipboard.min.js 712 B 716 B +4 B +0.6%
plugins/custom-class/prism-custom-class.min.js 276 B 280 B +4 B +1.4%
plugins/data-uri-highlight/prism-data-uri-highlight.min.js 590 B 595 B +5 B +0.8%
plugins/diff-highlight/prism-diff-highlight.min.js 737 B 748 B +11 B +1.5%
plugins/download-button/prism-download-button.min.js 267 B 268 B +1 B +0.4%
plugins/file-highlight/prism-file-highlight.min.js 930 B 926 B -4 B -0.4%
plugins/filter-highlight-all/prism-filter-highlight-all.min.js 440 B 447 B +7 B +1.6%
plugins/highlight-keywords/prism-highlight-keywords.min.js 123 B 125 B +2 B +1.6%
plugins/inline-color/prism-inline-color.min.js 595 B 599 B +4 B +0.7%
plugins/jsonp-highlight/prism-jsonp-highlight.min.js 1.25 KB 1.26 KB +7 B +0.6%
plugins/keep-markup/prism-keep-markup.min.js 598 B 600 B +2 B +0.3%
plugins/line-highlight/prism-line-highlight.min.js 1.43 KB 1.44 KB +9 B +0.6%
plugins/line-numbers/prism-line-numbers.min.js 1.11 KB 1.13 KB +17 B +1.5%
plugins/match-braces/prism-match-braces.min.js 848 B 862 B +14 B +1.7%
plugins/normalize-whitespace/prism-normalize-whitespace.min.js 1.1 KB 1.11 KB +6 B +0.5%
plugins/previewers/prism-previewers.min.js 2.98 KB 2.99 KB +9 B +0.3%
plugins/remove-initial-line-feed/prism-remove-initial-line-feed.min.js 238 B 239 B +1 B +0.4%
plugins/show-invisibles/prism-show-invisibles.min.js 344 B 349 B +5 B +1.5%
plugins/show-language/prism-show-language.min.js 2.32 KB 2.32 KB +1 B +0.0%
plugins/toolbar/prism-toolbar.min.js 746 B 755 B +9 B +1.2%
plugins/treeview/prism-treeview.min.js 434 B 436 B +2 B +0.5%
plugins/unescaped-markup/prism-unescaped-markup.min.js 511 B 513 B +2 B +0.4%
plugins/wpd/prism-wpd.min.js 1.28 KB 1.27 KB -4 B -0.3%

Generated by 🚫 dangerJS against 2e3811e

@RunDevelopment
Copy link
Member

I noticed that the existing build system does not support const or let.

This isn't a technical limitation but an intentional choice. Prism v1.x is an ES5 project to support older browsers such as IE11. (I know that IE11 actually supports some ES6 features (such as let and const) but it is a lot easier to enforce ES5 than it is to enforce the specific subset of ES6 supported by IE11.) However, we do plan to drop IE11 in v2.0 (see #1578) but that might still take a while.

Therefore:

  • This change cannot be part of Prism v1.x. We cannot drop support for IE11 without changing the major version.
  • As for v2.0: using Terser is only one step we have to undertake in order to modernize our codebase. This PR will not help much with these tasks.

For these reasons, I will now close this PR.

@NullVoxPopuli
I'm sorry to reject your PR but I cannot allow ES6+ features yet. Please use ES5 equivalents (such var instead of let and const) while the Prism team figures out a way to modernize this codebase.

@NullVoxPopuli
Copy link
Author

all good. do you have thoughts on transpiling down to ES5?

@mAAdhaTTah
Copy link
Member

Interesting that switching to Terser made all of the builds larger as well. If we continued to support our given syntax set but the minifier produced better results, I'd be in support but it seems like Terser produces worse results across the board for our codebase.

@NullVoxPopuli
Copy link
Author

NullVoxPopuli commented Apr 24, 2021 •
edited
Loading

Interesting that switching to Terser made all of the builds larger as well

afaict, This is only because the variable Prism is no longer abbreviated to one letter, since it's a global.

There are a bunch of options that I haven't touched, and I'm no terser expert, so 🤷

@RunDevelopment
Copy link
Member

Yes, terser is a little bit worse than uglify for ES5 but not by much. When talking about minified ES6+ code, it can do a bit more.

@joshgoebel
Copy link

joshgoebel commented Apr 24, 2021 •
edited
Loading

Well I was going to say terser can typically do much better than it's defaults (and this may still be true), but for us the difference is only 1% (300 bytes)... so I guess it all depends. It's also possible we don't have it tuned as finely as it could be as well. :) Though we also do some things to increase size like use ascii-only.

@mAAdhaTTah
Copy link
Member

I don't think that's the case because all of the languages are wrapped in IIFEs and pass in Prism, so they're mostly not referencing the global Prism. I only mention this because my general thing is keeping the bundle small, so if you wanna spend some time configuring Terser to produce smaller bundles, I'm all for it.

As for ES6, I believe we need IE11 support for our unminified code as well, so I think that's a no go until the V2 rewrite.

@RunDevelopment RunDevelopment mentioned this pull request Apr 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NullVoxPopuli @RunDevelopment @mAAdhaTTah @joshgoebel