withastro / compiler

The Astro compiler. Written in Go. Distributed as WASM.
Other
502 stars 59 forks source link

Issue with slots containing multiple items / conditions #950

Closed spacedawwwg closed 10 months ago

spacedawwwg commented 10 months ago

Astro Info

Astro                    v4.2.3
Node                     v20.10.0
System                   macOS (arm64)
Package Manager          pnpm
Output                   server
Adapter                  none
Integrations             astro-dynamic-import
                         adds-to-head
                         @astrojs/vue
                         astro-purgecss
                         astro-compress

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

as of v4.2.2, slots are not longer working correctly. When a slot has a loops, only the last item is being rendered. From what I can tell, its seems to be something to do with wrapping a slot in a condition (see reproduction)

This does not work (only last item in items is rendered)

---
import Layout from '../layouts/Layout.astro';
import Test from '../components/Test.vue';

const items = [1,2,3,4,5];
const test = false;

---
<Layout title="Welcome to Astro.">
    Outside
    <Test>
        {test && (
            <p slot={test ? 'test' : undefined}>
                Test name slot
            </p>
        )}
        {items.map(item => (
            <p>Item {item}</p>
        ))}
    </Test>
</Layout>

This does work (all items rendered)

---
import Layout from '../layouts/Layout.astro';
import Test from '../components/Test.vue';

const items = [1,2,3,4,5];
const test = false;
---

<Layout title="Welcome to Astro.">
    Outside
    <Test>
      <p slot={test ? 'test' : undefined}>
        Test name slot
      </p>
      {items.map(item => (
        <p>Item {item}</p>
      ))}
    </Test>
</Layout>

What's the expected result?

all items to be rendered in correct slots

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-vzmcxd?file=src%2Fpages%2Findex.astro,src%2Flayouts%2FLayout.astro,src%2Fcomponents%2FTest.vue

Participation

florian-lefebvre commented 10 months ago

Probably a regression introduced in withastro/astro#9605? @MoustaphaDev what do you think? I don't know anything about slots or the compiler, I hope you don't mind the ping 😅

spacedawwwg commented 10 months ago

Previously, we had to use both:

{test && (
  <p slot={test ? 'test' : undefined}>
    Test name slot
  </p>
)}

as the named slot was always showing, no matter if test was defined. Not sure what changes were made to slots, but an example of "best practice" for the above would also be appreciated

MoustaphaDev commented 10 months ago

Thanks for the ping @florian-lefebvre! Yeah that's probably a regression, I'll take a look today!

MoustaphaDev commented 10 months ago

As a temporary workaround, you can wrap the second expression in a fragment @spacedawwwg

---
import Layout from '../layouts/Layout.astro';
import Test from '../components/Test.vue';

const items = [1,2,3,4,5];
const test = false;

---
<Layout title="Welcome to Astro.">
    Outside
    <Test>
        {test && (
            <p slot={test ? 'test' : undefined}>
                Test name slot
            </p>
        )}
        <>
            {items.map(item => (
                <p>Item {item}</p>
            ))}
        </>
    </Test>
</Layout>

Sorry for the trouble! I'll try to push a fix as soon as possible!

spacedawwwg commented 10 months ago

get in, @MoustaphaDev 🚀

ZeldOcarina commented 9 months ago

Kindly please reopen this. See #959.