Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page island is rendered in slot of layout island #193

Closed
2 tasks done
davidlueder opened this issue Sep 11, 2022 · 4 comments
Closed
2 tasks done

Page island is rendered in slot of layout island #193

davidlueder opened this issue Sep 11, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@davidlueder
Copy link

davidlueder commented Sep 11, 2022

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:

return `ile-${++idNumber}`

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
Copy link
Owner

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
Copy link
Author

davidlueder commented Sep 11, 2022

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?

router.afterEach(resetHydrationId) // reset island identifiers to match ssg.

@ElMassimo
Copy link
Owner

ElMassimo commented Sep 11, 2022

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 ElMassimo added the bug Something isn't working label Sep 11, 2022
@ElMassimo
Copy link
Owner

ElMassimo commented Sep 12, 2022

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:

Screen Shot 2022-09-11 at 22 37 34

Thanks again for reporting! @davidlueder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants