Document removeComments can cause SSR / React hydration issues

[slorber]

For React sites using server-side rendering and client-side hydration, using the removeComments option can lead to hydration problems.

The original React SSR output is:

<div id="__docusaurus">
   <span>
      Built using the<!-- --> <a href="https://www.electronjs.org/" target="_blank">Electron</a> <!-- -->, 
      based on<!-- --> <a href="https://www.chromium.org/" target="_blank">Chromium</a>
   </span>
</div>

It is optimized into:

<div id="__docusaurus">
   <span>Built using the <a href="https://www.electronjs.org/" target="_blank">Electron</a> , 
   based on <a href="https://www.chromium.org/" target="_blank">Chromium</a>
</div>

Removing those comments in the SSR HTML leads to both links having href="https://www.chromium.org/" after React hydration.

---

This lib is agnostic but React is quite widely used and this bug is a bit nasty to pin down.

I find it valuable to document it for others but I understand if you want to close this PR (at least it will be searchable).

---

Refs:

• https://github.com/facebook/docusaurus/pull/5629
• https://twitter.com/dan_abramov/status/1443628592097898498

[DanielRuf]

<!-- -->

I've never seen this "trick" in any project so far. To be fair we've never used SSR in any projects.

Doesn't ignoreCustomComments or ignoreCustomFragments cover this use case? There are good reasons why most options like removeComments are disabled / set to false by default.

[DanielRuf]

Removing those comments in the SSR HTML leads to both links having href="https://www.chromium.org/" after React hydration.

Not sure what React does there but to me this looks like a bug in React when it tries to do it like this (or some sort of weird and faulty concept).

[DanielRuf]

Hm, seems this weird "solution" is established for a few years already. Now I know why I've never used any extra configs to remove comments in projects or never used SSR setups or never touched the generated HTML in such cases (due to too many variables which could affect the outcome). So this caveat should be known by more people.

See also https://github.com/stereobooster/react-snap/issues/142#issuecomment-366562246

Since there are other caveats and "tricks" like empty attributes and other weird things in other frameworks and setups I'm hesitant to add a note for every single option whch could cause such issues.

Interesting how fragile frontend stuff is due to such weird workarounds.

My opinion: nothing that we have to document

But we should wait for the other maintainers and their opinion.

I've tested it with ignoreCustomComments in a CodeSpace and this should cover your use case:

const result = await minify(content, {
    removeComments: true,
    ignoreCustomComments: [/^!/, /^\s*#/, /^ /] // include comment which contains only a space
});

[sibiraj-s]

Since there are other caveats and "tricks" like empty attributes and other weird things in other frameworks and setups I'm hesitant to add a note for every single option which could cause such issues.

Yes. I agree. This is one and when more comes It is not possible to mention all tricks that frameworks follow to the docs as a note beside the options.

It is common for any minifier removes comments. I have seen something like this with comments used by Angular, now React and Vue are using them. Instead of we adding it here, I would recommend the frameworks to add a note in the docs on the importance of those comments and should also provide the notes for todo's while processing, In this case minify.

[slorber]

Agree.

Going to close then :)