Page island is rendered in slot of layout island

davidlueder commented 1 year ago

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

Description 📖

When islands are rendered within a layout file and within a page file, the page island hydration ids start with ile-1 again. The second component from the page is rendered in the slot of the layout component (see stackblitz example)

It seems to have something to do with the newHydrationId() function but i was not able to find the root cause: https://github.com/ElMassimo/iles/blob/df8b7935ab1afd2a8b238a3cb93a885c4634f6d1/packages/iles/src/client/app/hydration.ts#L22

Reproduction 🐞

https://stackblitz.com/edit/iles-ytnhui

npm run dev - bug reproducible npm run now - no bug (as expected because newHydrationId() uses a different implementation for SSR during build)

Logs 📜

[Vue warn]: There is already an app instance mounted on the host container. If you want to mount another app on the same host container, you need to unmount the previous app by calling app.unmount() first.

ElMassimo commented 1 year ago

Hi David, thanks for reporting.

This could be another case of modules being duplicated by Vite, or a bug in how the counter is reset after navigation.

Would you check if there are two copies of client/app/hydration.ts being sent to the browser during development in your site?

davidlueder commented 1 year ago

Looks like .../node_modules/iles/dist/client/app/hydration.js is loaded only once but each navigation call triggers the resetHydrationId() function and therefore causes the bug.

But that seems to be the wanted behavior? https://github.com/ElMassimo/iles/blob/ccbb0019e67f6bf82a0bfb21ae030e34869ef786/packages/iles/src/client/app/index.ts#L94

ElMassimo commented 1 year ago

Gotcha, then it's option 2, thanks for checking.

But that seems to be the wanted behavior?

The idea behind resetting was to simulate the behavior that would occur during build, where each page starts from 1.

I don't understand why this bug does not manifest in the docs site, need to check.

Perhaps this has to do with how Vue reuses DOM elements when re-rendering the layout (if layout does not change between pages, islands in layout are not re-rendered, so newHydrationId is not called until the first page island, which then gets id that overlaps with the layout islands).

Since ids are internal, a potential fix is to avoid resetting ids in development.

The downside is that when using the dev tools, ids for islands would continue increasing until a full reload occurs. Currently, each island within a page should always have the same id.

ElMassimo commented 1 year ago

Perhaps this has to do with how Vue reuses DOM elements when re-rendering the layout

That theory turned out to be correct, released a fix in iles@0.8.5.

Fixed by changing the way ids are recycled during dev: instead of resetting to zero on navigation, we now mark ids as available once islands are unmounted.

As a result, we prevent using ids which are currently taken, but ids still match what you would see during build, even as you navigate across pages:

Thanks again for reporting! @davidlueder

ElMassimo / iles