Fix various issues with slots #933
Conversation
🦋 Changeset detectedLatest commit: 97ce7ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…t/dynamic-slot-names
…t/dynamic-slot-names
MoustaphaDev
force-pushed
the
feat/dynamic-slot-names
branch
from
c5ebcf5 to
bb95980
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
MoustaphaDev
force-pushed
the
feat/dynamic-slot-names
branch
from
a4fffd8 to
0810a11
Compare
…t/dynamic-slot-names
MoustaphaDev
changed the title
There was a problem hiding this comment.
Phenomenal work! Tests look great and the code makes sense.
My only open question is about the mergeSlots function in core because there might be projects with an older Astro runtime but this release of the compiler. If everything is backwards compatible that's great, otherwise we might need to figure out the best way to handle this migration.
|
Yes, this is backwards compatible as it doesn't break any current slot functionality. Only users on older Astro versions attempting to use this feature will encounter issues (which they of course did even before this release if they tried to). I can make a PR to docs to communicate that clearly |
This reverts commit db13db9.
…" (#963) * test(#955): add failing test * test(#955): add another failing test * test: whoops it actually passes * Revert "Fix slot regression when there are multiple expressions (#952)" This reverts commit 418558c. * Revert "Fix various issues with slots (#933)" This reverts commit db13db9. * chore: changeset * chore: remove slot parens in test * test: update test * chore: remove slot parens in test * test: add test for #959 --------- Co-authored-by: Nate Moore <[email protected]>
Changes
undefinedslot name issue will be tackled in another PRPreviously, using variables as slot names within loops was not possible because the slot object (specifically, the slot key) wasn't printed in the same location as the slotted elements.
This PR addresses this issue by refactoring the slot printing logic to ensure that the slot object is printed alongside the slotted elements when a dynamic slot name is used.
The other issues were fortuitously fixed with the refactor
Testing
Added test cases covering each issue
Docs
This wasn't possible before so we should probably document this. I'll make a docs PR