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

fix: renderToString function not render properly nested slots when they are components #13432

Merged
merged 4 commits into from
Mar 18, 2025

Conversation

P4tt4te
Copy link
Contributor

@P4tt4te P4tt4te commented Mar 14, 2025

Changes

Fixes #13067

In the initial issue, all slots passed to the renderToString function are considered strings, so when we pass them to the renderChild function, the HTML escaper breaks the component.

When a slot is passed to the renderToString function, a new SlotString is created.

I created a SlotString for now, but I don't know if this solution is the best approach (it's my first PR on Astro).
Let me know if it's correct or not. :)

Testing

I added a case inside container-with-vitest\test\Card.test.ts that reproduces the case displayed on the issue here.

Docs

The documentation is already present as mentioned by the initial issue.

P4tt4te and others added 3 commits March 14, 2025 18:34
* add force HTMLString with local tests

* remove local test & logs

* remove test counter in basic exemple

* re-add test & create SlotString instead of HTMLString
Copy link

changeset-bot bot commented Mar 14, 2025

🦋 Changeset detected

Latest commit: afeff3b

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: example Related to an example package (scope) pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release labels Mar 14, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

Copy link

codspeed-hq bot commented Mar 14, 2025

CodSpeed Performance Report

Merging #13432 will not alter performance

Comparing P4tt4te:fix/render-to-string-slots-render (afeff3b) with main (013fa87)

Summary

✅ 6 untouched benchmarks

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

This was a great fix! Thank you @P4tt4te!

@ematipico ematipico merged commit defad33 into withastro:main Mar 18, 2025
16 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Mar 18, 2025
@P4tt4te P4tt4te deleted the fix/render-to-string-slots-render branch March 18, 2025 11:53
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: example Related to an example package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AstroContainer renderToString Slots example does not render HTML
2 participants