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

Pass children to client components even if they do not render them #2588

Merged
merged 5 commits into from
Feb 16, 2022

Conversation

matthewp
Copy link
Contributor

Changes

Testing

Tests added.

Docs

Not needed, bug fix.

@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2022

🦋 Changeset detected

Latest commit: ad69806

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

This PR includes changesets to release 1 package
Name Type
astro Patch

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: astro Related to the core `astro` package (scope) test labels Feb 15, 2022
@matthewp matthewp marked this pull request as ready for review February 15, 2022 15:36
Copy link
Contributor

@jonathantneal jonathantneal left a comment

Choose a reason for hiding this comment

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

Interesting, clever fix.

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Woah. Clever fix! LGTM.

if(fragment == null) {
// If there is no child fragment, check to see if there is a template.
// This happens if children were passed but the client component did not render any.
let template = roots[0].querySelector(`template[data-astro-template]`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self, this is going to check for this template any time there is not an astro-fragment. We can probably short circuit this check so it's only done when needed.

@matthewp matthewp marked this pull request as draft February 15, 2022 22:09
@matthewp
Copy link
Contributor Author

Put this back to draft as I want to make one small change but need to test it locally first.

Copy link
Contributor

@tony-sull tony-sull left a comment

Choose a reason for hiding this comment

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

Nice, that's a very cool fix!

@matthewp matthewp marked this pull request as ready for review February 16, 2022 14:06
Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looks good!

@matthewp matthewp merged commit 1021617 into main Feb 16, 2022
@matthewp matthewp deleted the donut-no-ssr branch February 16, 2022 15:11
This was referenced Feb 16, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
…ithastro#2588)

* Pass children to client components even if they do not render them

* Handle when no children are provided

* Adds a changeset

* Use roots directly i guess

* Use an attribute to signal that the template is needed
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Component children aren't hydrated if they were conditionally hidden during the build
4 participants