DOM shuffle after SSR hydration

[Xowap]

Vue version

3.3.4

Link to minimal reproduction

https://github.com/Xowap/vue-hydration-bug

https://stackblitz.com/edit/stackblitz-starters-lgtp6t?file=app.js

Steps to reproduce

The example is straight out of the Vue SSR doc, just with a different template. Just run example.js and visit localhost:3000.

What is expected?

It should display blocks in "AAA"/"BBB"/"CCC"/"AAA"/"BBB"/"CCC" order.

What is actually happening?

The SSR renders the HTML correctly but as soon as hydration kicks the order is kind of randomized, usually "AAA"/"BBB"/"CCC"/"CCC"/"AAA"/"BBB" on my machine.

System Info

System:
    OS: Linux 6.2 Ubuntu 23.04 23.04 (Lunar Lobster)
    CPU: (24) x64 AMD Ryzen 9 7900X 12-Core Processor
    Memory: 9.83 GB / 30.49 GB
    Container: Yes
    Shell: 3.6.0 - /usr/bin/fish
  Binaries:
    Node: 18.15.0 - ~/.nvm/versions/node/v18.15.0/bin/node
    npm: 9.5.0 - ~/.nvm/versions/node/v18.15.0/bin/npm
    pnpm: 8.1.0 - ~/.local/share/pnpm/pnpm
  Browsers:
    Chrome: 116.0.5845.140
  npmPackages:
    vue: ^3.3.4 => 3.3.4 

Any additional comments?

This bug popped in a complex Nuxt app, I've been able to dumb it down to this example which outlines its Vue origin. Presumably it affects all Vue 3 versions, at least those that I have tested (3.1 to 3.3 if I'm not mistaken).

To be noted as well that if you drop the .prod from the import map, the bug goes away, at least with this example (not sure if in the absolute).

[Xowap]

I am extremely inexperienced with Vue build pipelines and so forth, however so far I've been able to build the esm-browser format by tweaking a bit the dev.js script:

diff --git a/scripts/dev.js b/scripts/dev.js
index 8342f523..dae98834 100644
--- a/scripts/dev.js
+++ b/scripts/dev.js
@@ -15,7 +15,7 @@ const require = createRequire(import.meta.url)
 const __dirname = dirname(fileURLToPath(import.meta.url))
 const args = minimist(process.argv.slice(2))
 const target = args._[0] || 'vue'
-const format = args.f || 'global'
+const format = args.f || 'esm-browser'
 const inlineDeps = args.i || args.inline
 const pkg = require(`../packages/${target}/package.json`)
 
@@ -109,7 +109,7 @@ esbuild
     define: {
       __COMMIT__: `"dev"`,
       __VERSION__: `"${pkg.version}"`,
-      __DEV__: `true`,
+      __DEV__: `false`,
       __TEST__: `false`,
       __BROWSER__: String(
         format !== 'cjs' && !pkg.buildOptions?.enableNonBrowserBranches

Turns out if I force __DEV__ to false then the bug happens even in the non-minified build. I guess I'll just have to check the 400 ifs around the code 🔍

[Xowap]

So I've done a bisection to figure which flags are causing the issue, it looks like when those two are set together it will cause the bug to happen.

diff --git a/packages/compiler-core/src/parse.ts b/packages/compiler-core/src/parse.ts
index b72ad028..4d1c5316 100644
--- a/packages/compiler-core/src/parse.ts
+++ b/packages/compiler-core/src/parse.ts
@@ -77,7 +77,7 @@ export const defaultParserOptions: MergedParserOptions = {
     rawText.replace(decodeRE, (_, p1) => decodeMap[p1]),
   onError: defaultOnError,
   onWarn: defaultOnWarn,
-  comments: __DEV__
+  comments: false
 }
 
 export const enum TextModes {
diff --git a/packages/runtime-core/src/hydration.ts b/packages/runtime-core/src/hydration.ts
index 89a00886..4926a3fb 100644
--- a/packages/runtime-core/src/hydration.ts
+++ b/packages/runtime-core/src/hydration.ts
@@ -315,7 +315,7 @@ export function createHydrationFunctions(
     const forcePatchValue = (type === 'input' && dirs) || type === 'option'
     // skip props & children if this is hoisted static nodes
     // #5405 in dev, always hydrate children for HMR
-    if (__DEV__ || forcePatchValue || patchFlag !== PatchFlags.HOISTED) {
+    if (false || forcePatchValue || patchFlag !== PatchFlags.HOISTED) {
       if (dirs) {
         invokeDirectiveHook(vnode, null, parentComponent, 'created')
       }

Not sure about the role of comments here, but since the faulty line is already mentioned in #5405 I'm gonna guess that this is a strong hint.

Beyond that I'm not familiar enough with the Vue internals to interpret this correctly.

[Xowap]

Just to be clear on the above comment, in order to produce the bug in dev mode you need to apply the patch that forces __DEV__ to false in the two location mentioned then build esm-browser in dev mode. This will produce a packages/vue/dist/vue.esm-browser.js file which once loaded as vue import in my demo above will trigger the bug. On the other hand if you load a dev build without this patch, the bug will not be triggered. And finally if you load a prod build, the bug will also occur. Hope it's clear enough!

[linghaoSu]

You can set app.config.compilerOptions.comments as true, and then you will get the right hydrate.

The comments option in compilerOptions defaults to false in production, and when generating a vnode with html, the comments are removed by default.
but in this case, the html content still contains comments in the result of renderToString.
Therefore, when some vnode whose type is not comment encounters comment element, mismatch occurs.

Not quite sure this counts as a problem.

Maybe vue needs to make the renderToString method in production mode remove comments by default, like it does when generating a vnode.

[Xowap]

Thanks for having a look.

In terms of keeping the comments, this is really not the issue here: they are not needed nor wanted, merely an artifact in my app (which is obviously much larger than this example).

However it seems to me than randomly shuffling DOM elements is not an expected behavior, especially without any explicit warning.

If I might give my input on possible solutions:

• If this raises a performance concern, having a development-only warning that comments will result in unexpected behavior when in production might have saved me several days of work from myself and my team investigating into this issue (going from "random bug that happens only in production within 100kLoC" to "here's the reproducible bug" has been a trip) but would definitely be frustrating
• My preferred solution would indeed be that comments be simply ignored

Tell me what you think

[edison1105]

This is because your client and server use different versions of vue.esm-browser.js

• server-side: vue.esm-browser.js (keep comments in dev)
• client-side: vue.esm-browser.prod.js(remove comments in prod)

<script type="importmap">
{
	"imports": {
		"vue": "/node_modules/vue/dist/vue.esm-browser.prod.js" // should use vue.esm-browser.js if in dev
	}
}

[Xowap]

Could be, however this bug manifested in Nuxt initially, all I did was reproduce the use case. Should I bring the issue to them then?

[Xowap]

Just to be clear, what should I do different in my published example? Let's say that the documentation is far from being exhaustive on that topic.

[edison1105]

Could be, however this bug manifested in Nuxt initially, all I did was reproduce the use case. Should I bring the issue to them then?

please provide a runnable reproduction with nuxt, let's see further.

[Xowap]

Sure, here it is: https://github.com/Xowap/nuxt-hydration-bug

[Xowap]

Indeed, after digging into the package source code, I understand that the import varies depending on whether NODE_ENV=production or not and if I set it the bug goes away. My mistake from reading the Nuxt doc and expecting that this variable was set automatically for some reason.

However this is a highly surprising behavior (and another breaking change from Vue 2), I get the feeling that there should at least be some kind of warning somewhere?

[edison1105]

Closing this issue as it has been identified not to be a bug based on the discussion. Thank you for your contribution!