feat(core): allow opting out of HTML minification

[alexandernst]

Pre-flight checklist

• I have read the Contributing Guidelines on pull requests.
• If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
• If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

The current build process is "optimizing" the HTML in a way that breaks bootstrap. More specifically, it removes some "worthless" attributes (such as type="text" from input fields), which breaks Bootstrap's [type="text"] selectors (probably selectors of other css frameworks aswell).

This change will allow users to specify the exact options that they want to be passed to the minifier. It won't introduce any breaking changes since it's merging the existing opts with any user-provided opts.

Test Plan

N/A

Test links

• https://deploy-preview-7581--docusaurus-2.netlify.app/docs/api/docusaurus-config/#buildConfig

Related issues/PRs

N/A

[netlify]

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	4347c7f
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/62a0e61f42506500084a7bda
😎 Deploy Preview	https://deploy-preview-7581--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[github-actions]

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 69	🟢 100	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 81	🟢 100	🟢 100	🟢 100	🟢 90	Report

[Josh-Cena]

When we were discussing over Discord, I'm not really sure whether we want a simple env var that toggles minification on/off, or a full config option. I'm strongly leaning towards the former because it isn't that well-used at all and making that many options for it seems like an overkill. Remember that we try to minimize the API surface as much as possible.

That's my strong opinion, not sure about @slorber's, but I think we definitely need to use env vars instead, maybe even 6 env vars, but never a config option.

[alexandernst]

a simple env var that toggles minification on/off

I get that you're trying to keep the API surface as simple as possible, but a single "on" or "off" switch that enables or disables the optimization / minification of everything (html, css and js) is just not useful. There is a bug (that I described in the PR) that happens when a particular optimization is performed. That doesn't mean that I want absolutely every optimization disabled.

Plus, the API surface is still simple, the entire buildConfig hashmap is optional; and at the same time it also providers flexibility to the users who might want to tweak the optimization step.

[Josh-Cena]

but a single "on" or "off" switch that enables or disables the optimization / minification of everything (html, css and js) is just not useful

No, only a single env var that toggles HTML minification. CSS and JS minification is controlled by another argument (--no-minify) already. Also, I'm okay with 6 env vars, just not a config option—env vars are implicit API and can get as big as we want. This is what we do with other build options: we have a bunch of env var APIs that control these optimizations already (e.g. we have DOCUSAURUS_SSR_CONCURRENCY, WEBPACK_URL_LOADER_LIMIT, USE_SIMPLE_CSS_MINIFIER).

[alexandernst]

Oh, I see. Okey! Should I use a single env var in order to skip the entire minify() call? Or do you want me to use an env var for each of the settings that are currently passed? (check code snippet below)

        {
          removeComments: false,
          removeRedundantAttributes: true,
          removeEmptyAttributes: true,
          removeScriptTypeAttributes: true,
          removeStyleLinkTypeAttributes: true,
          useShortDoctype: true,
          minifyJS: true,
        },

[Josh-Cena]

I'm fine with either, but not sure about @slorber's preference. Let's use a single env var first to skip the entire minify call, shall we?

[alexandernst]

@Josh-Cena I just force-pushed the new changes, 🙏👀

[slorber]

I'm fine with env vars with very explicit names used as escape hatches here and there 👍

To me, those things do not even have to be documented at all, as most users don't need it anyway, and not documenting it is also an opportunity for us to collect feedback on how it is or will be used. If it becomes useful to a lot of users and we have enough feedback, we can move it to a first-class option.

[Josh-Cena]

LGTM—@slorber wants to remove the documentation, so I'll see if he absolutely want that change before merge.

[slorber]

that should be fine thanks 👍