Skip to content

Fix slot regression when there are multiple expressions#952

Merged
MoustaphaDev merged 13 commits intomainfrom
fix-slot-regression
Jan 30, 2024
Merged

Fix slot regression when there are multiple expressions#952
MoustaphaDev merged 13 commits intomainfrom
fix-slot-regression

Conversation

@MoustaphaDev
Copy link
Copy Markdown
Member

@MoustaphaDev MoustaphaDev commented Jan 29, 2024

Changes

A regression in how we print slot was introduced in my PR in #933.

Normally, if there is more than one slotted element or any slotted element with dynamic slot prop in an expression, we consider it a nested slot. However, instead of doing the count per expression, we aggregated it for all expressions.

Because of that, if there were any slotted element in an expression, any subsequent slotted element of the same kind would be considered a nested slot

To fix that, we localized each of those states per expression

Testing

  • Adapted and added a few printer tests to reflect the changes and ensure a correct output
  • Added tests to Astro core to ensure a correct result

Docs

N/A bug fix only

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jan 29, 2024

🦋 Changeset detected

Latest commit: 336021b

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

This PR includes changesets to release 1 package
Name Type
@astrojs/compiler 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

@MoustaphaDev
Copy link
Copy Markdown
Member Author

!preview mk-slot-expr-fix

@github-actions
Copy link
Copy Markdown
Contributor

 > root@0.0.0 release /home/runner/work/compiler/compiler > changeset publish "--tag" "next--mk-slot-expr-fix" 🦋 warn ===============================IMPORTANT!=============================== 🦋 warn Packages will be released under the next--mk-slot-expr-fix tag 🦋 warn ---------------------------------------------------------------------- 🦋 info npm info @astrojs/compiler 🦋 info @astrojs/compiler is being published because our local version (0.0.0-mk-slot-expr-fix-20240129183218) has not been published on npm 🦋 info Publishing "@astrojs/compiler" at "0.0.0-mk-slot-expr-fix-20240129183218" 🦋 success packages published successfully: 🦋 @astrojs/compiler@0.0.0-mk-slot-expr-fix-20240129183218 🦋 Creating git tag... 🦋 New tag: @astrojs/compiler@0.0.0-mk-slot-expr-fix-20240129183218

@MoustaphaDev
Copy link
Copy Markdown
Member Author

!preview mk-slot-expr-fix

@github-actions
Copy link
Copy Markdown
Contributor

 > root@0.0.0 release /home/runner/work/compiler/compiler > changeset publish "--tag" "next--mk-slot-expr-fix" 🦋 warn ===============================IMPORTANT!=============================== 🦋 warn Packages will be released under the next--mk-slot-expr-fix tag 🦋 warn ---------------------------------------------------------------------- 🦋 info npm info @astrojs/compiler 🦋 info @astrojs/compiler is being published because our local version (0.0.0-mk-slot-expr-fix-20240130083013) has not been published on npm 🦋 info Publishing "@astrojs/compiler" at "0.0.0-mk-slot-expr-fix-20240130083013" 🦋 success packages published successfully: 🦋 @astrojs/compiler@0.0.0-mk-slot-expr-fix-20240130083013 🦋 Creating git tag... 🦋 New tag: @astrojs/compiler@0.0.0-mk-slot-expr-fix-20240130083013

@MoustaphaDev MoustaphaDev marked this pull request as ready for review January 30, 2024 09:14
@MoustaphaDev MoustaphaDev merged commit 418558c into main Jan 30, 2024
@MoustaphaDev MoustaphaDev deleted the fix-slot-regression branch January 30, 2024 16:12
MoustaphaDev added a commit that referenced this pull request Feb 5, 2024
Princesseuh pushed a commit that referenced this pull request Feb 6, 2024
…" (#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 <nate@astro.build>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with slots containing multiple items / conditions

3 participants