HMR unmount fails for Vue islands with fragments instead of a single root node

davidlueder commented 2 years ago

[x] I have read the troubleshooting section before opening an issue.
[x] I have tried upgrading iles and vite.

Description 📖

Changing anything within a components setup script, that was used in a page with the client:load attribute results in an error in the browser console.

The error contains This is likely a Vue internals bug. but when removing the client:load attribute from the component tag everything works fine. This is why i think that might be an issue in iles.

Reproduction 🐞

Open Project https://stackblitz.com/edit/iles-2xnpyn?file=src%2Fcomponents%2FTestComponent.vue
Click "Open in New Window"
Open DevTools in that window
Change something within the "TestComponent.vue" script in on stackblitz.com

Logs 📜

[vite] hot updated: /src/components/TestComponent.vue
runtime-core.esm-bundler.js:38 [Vue warn]: Unhandled error during execution of scheduler flush. This is likely a Vue internals bug. Please open an issue at https://new-issue.vuejs.org/?repo=vuejs/core 
  at <Island component= {__hmrId: '7734c30b', __file: '/home/projects/iles-2xnpyn/src/components/TestComponent.vue', setup: ƒ, render: ƒ} componentName="TestComponent" importName="default"  ... > 
  at <Index > 
  at <DefaultLayout key=1 > 
  at <RouterView> 
  at <îles>
warn2 @ runtime-core.esm-bundler.js:38
logError @ runtime-core.esm-bundler.js:212
handleError @ runtime-core.esm-bundler.js:204
callWithErrorHandling @ runtime-core.esm-bundler.js:158
flushJobs @ runtime-core.esm-bundler.js:394
Promise.then (async)
queueFlush @ runtime-core.esm-bundler.js:285
queueJob @ runtime-core.esm-bundler.js:279
reload @ runtime-core.esm-bundler.js:528
(anonymous) @ runtime-core.esm-bundler.js:566
(anonymous) @ TestComponent.vue:12
(anonymous) @ client.ts:428
(anonymous) @ client.ts:349
(anonymous) @ client.ts:209
queueUpdate @ client.ts:209
await in queueUpdate (async)
(anonymous) @ client.ts:78
handleMessage @ client.ts:76
(anonymous) @ client.ts:50
dispatchEvent @ .localservice@runtime.0e61f5616522419418be8a0cc53f5e2e645f635f.js:1
_handleMessage @ .localservice@runtime.0e61f5616522419418be8a0cc53f5e2e645f635f.js:7
_0x461884 @ .localservice@runtime.0e61f5616522419418be8a0cc53f5e2e645f635f.js:7
runtime-dom.esm-bundler.js:36 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'nextSibling')
    at nextSibling (runtime-dom.esm-bundler.js:36:31)
    at removeFragment (runtime-core.esm-bundler.js:5571:20)
    at remove2 (runtime-core.esm-bundler.js:5537:13)
    at unmount (runtime-core.esm-bundler.js:5521:17)
    at unmountComponent (runtime-core.esm-bundler.js:5593:13)
    at unmount (runtime-core.esm-bundler.js:5494:13)
    at patchKeyedChildren (runtime-core.esm-bundler.js:5376:21)
    at patchChildren (runtime-core.esm-bundler.js:5205:21)
    at patchElement (runtime-core.esm-bundler.js:4721:13)
    at processElement (runtime-core.esm-bundler.js:4569:13)

Screenshots 📷

ElMassimo commented 2 years ago

After inspecting your example, it seems that this reliably happens when unmounting and island component that has a fragment template (several sibling nodes at the root), failing at removeFragment.

This is likely to be a Vue bug which is very rare in practice, since it requires:

A Vue app that has a fragment template (more than one node at the root) (have in mind each island is a Vue app)
The Vue app is unmounted

Most Vue apps are never unmounted, and typically have a single root node, which is why I say it's rare in practice.

If you can investigate and provide a small replication for Vue that doesn't depend on iles, it's likely that the Vue core team would fix it.

In the meantime, I'd suggest using a single root node in Vue islands, as in:

<template>
  <div>
    <p>This is the component</p>
    <span ref="scriptAddition"></span>
  </div>
</template>

davidlueder commented 2 years ago

Thank you, using a single root node is of course absolutely no problem.

davidlueder commented 2 years ago

I have tried to reproduce this just using vue & vite, but maybe i did not understand your explanation completely: https://stackblitz.com/edit/vitejs-vite-s4qyl1

Until now it seems like unmounting a fragmented root template doesn't result in an error

ElMassimo commented 2 years ago

Hi David, thanks for looking into it.

After doing some experiments, I also could not replicate the problem outside of HMR, as it seems to be specific to how HMR reloads the component by unmounting and remounting. The conflict occurs because for Vue islands the component is rendered first before the hydration script kicks in. This double-rendering seems to break the assumptions of unmount, which is heavily optimized to avoid rework, and expects the sibling elements to be present when clearing fragments.

I think a reasonable workaround will be not to pre-render Vue islands in development (similar to how it works for other framework islands), and let the inline script do the hydration + rendering on its own. This would also avoid confusing behavior such as #110.

I'll see if I can implement this while wrapping up a new CSS splitting feature once Vite 2.9 is released.

ElMassimo / iles