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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unmount framework components when islands are destroyed #8264

Merged
merged 16 commits into from
Aug 29, 2023
Merged

Conversation

natemoo-re
Copy link
Member

Changes

  • Adds an astro:unmount event when islands are destroyed
  • Improves view transitions lifecycle because navigating to a new page without an island now properly runs framework lifecycle hooks
  • Updates all the framework integrations

Testing

Tested manually but I would love to add some E2E tests

Docs

We don't really document how to build a new renderer, but this would be a detail for renderers to handle.

@changeset-bot
Copy link

changeset-bot bot commented Aug 29, 2023

🦋 Changeset detected

Latest commit: 7b1c4f3

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: react Related to React (scope) pkg: preact Related to Preact (scope) pkg: solid Related to Solid (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Aug 29, 2023
Comment on lines -40 to -47
render(
h(
Wrapper,
null,
h(Component, props, children != null ? h(StaticHtml, { value: children }) : children)
),
parent,
element
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a hacky solution instead of just using the built-in hydrate function, so I refactored to use that.

Comment on lines +177 to +179
// Only replace the existing body *AFTER* persistent elements are moved over
// This avoids disconnecting `astro-island` nodes multiple times
document.body.replaceWith(doc.body);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was actually important! We replace the persistent islands first, then swap the <body> element. Previously, we would trigger multiple disconnectedCallback in a row when using transition:persist.

@natemoo-re natemoo-re merged commit 1f58a7a into main Aug 29, 2023
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants