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

Named slots do not work in MDX sometimes #4610

Closed
1 task
zaha opened this issue Sep 3, 2022 · 9 comments · Fixed by #5080
Closed
1 task

Named slots do not work in MDX sometimes #4610

zaha opened this issue Sep 3, 2022 · 9 comments · Fixed by #5080
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@zaha
Copy link

zaha commented Sep 3, 2022

What version of astro are you using?

1.1.5

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

When the text for the default slot is not wrapped in a HTML tag (e.g. <p>) then the named slots' content is rendered in the default slots' location.

MyComponent.astro

<h1>My Component</h1>
Before slot: <slot name="before" /><br/>
Text inbetween before and default.<br/>
Default slot: <slot /><br/>
Text inbetween default and after.<br/>
After slot: <slot name="after" /><br/>
The end

Test.mdx

import MyComponent from "../../components/MyComponent.astro"

<MyComponent>
    <p slot="before" style="color: blue;">Before slot text.</p>
    Default slot text.
    <p slot="after" style="color: red;">After slot text.</p>
</MyComponent>

leads to

grafik

But when I do it like this it's working as expected.

<MyComponent>
    <p slot="before" style="color: blue;">Before slot text.</p>
    <p>Default slot text.</p>
    <p slot="after" style="color: red;">After slot text.</p>
</MyComponent>

grafik

Not sure if I'm missing something, but I would've expected both examples to work the same. It's reproducible on my local machine, but I wasn't able to reproduce it on StackBlitz. There, both work.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-21chqj?file=src/pages/example.mdx

Participation

  • I am willing to submit a pull request for this issue.
@matthewp matthewp added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Sep 6, 2022
@matthewp matthewp self-assigned this Sep 12, 2022
@matthewp
Copy link
Contributor

@zaha if you download this exact stackblitz locally and run it you see the problem?

If that's the case what OS are you using?

@matthewp matthewp added the needs response Issue needs response from OP label Sep 12, 2022
@zaha
Copy link
Author

zaha commented Sep 14, 2022

@matthewp I'm afraid I cannot reproduce it either with astro 1.1.5 in the downloaded stackblitz example neither in my own project running astro 1.2.4 where it originally appeared. Could be something that only happened on my laptop, will have to check that. My laptops' running on Win 11 whereas my desktop PC is running win 10.

@matthewp matthewp removed the needs response Issue needs response from OP label Sep 14, 2022
@zaha
Copy link
Author

zaha commented Sep 15, 2022

Okay, just tested it on my laptop but cannot reproduce it there, either. Seems like a happenstance with my dev server instance back then, maybe it was in a messed up state. What is curious, though, is that the styled p tag is now coming through like this:

<h1 id="doesnt-work">Doesn’t work</h1>
<h1>My Component</h1>
Before slot: <p style="color: blue;"></p>
<p>Before slot text.</p>
<p></p><br>
Text inbetween before and default.<br>
Default slot: <p>Default slot text.</p><br>
Text inbetween default and after.<br>
After slot: <p style="color: red;"></p>
<p>After slot text.</p>
<p></p><br>
The end

The "Before slot text" and "After slot text" are outside of the slot p tag it looks like, so they don't get the color. This also happens in the stackblitz example.

@matthewp
Copy link
Contributor

@zaha That's an HTML issue, you cannot nest <p> elements inside of each other.

@zaha
Copy link
Author

zaha commented Sep 15, 2022

@matthewp That makes sense, but there is no wrapping p tag around it in the component:

<h1>My Component</h1>
Before slot: <slot name="before" /><br/>
Text inbetween before and default.<br/>
Default slot: <slot /><br/>
Text inbetween default and after.<br/>
After slot: <slot name="after" /><br/>
The end

and in the generated HTML there are 2 extra p tags:

Before slot: <p style="color: blue;"></p>
<p>Before slot text.</p>
<p></p><br>

Shouldn't the resulting HTML be like this given the component?

Before slot: <p style="color: blue;">Before slot text.</p><br>

@matthewp
Copy link
Contributor

Hm, I assumed it was because Markdown wraps lines with <p> but maybe that's not what's going on here. Worth looking into to confirm.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P4: important Violate documented behavior or significantly impacts performance (priority) labels Sep 15, 2022
@matthewp matthewp removed their assignment Sep 15, 2022
@matthewp matthewp self-assigned this Sep 23, 2022
@darusk
Copy link

darusk commented Sep 30, 2022

I've recently come across this which is stopping an update from an old beta version (with old astro markdown) to latest using mdx, though I am nesting a component. I've done a repro which has a cut-down version of the code in use, with the effective same components and layout, where as a .astro it works, but as a .mdx it doesn't.

https://stackblitz.com/edit/github-2pttkh?file=src/pages/index.mdx

@matthewp
Copy link
Contributor

Looking into this now, here's a reproduction, seems like the slot attribute causes the components not to render. https://stackblitz.com/edit/github-2pttkh-6w3dbx?file=src%2Fpages%2Findex.mdx,astro.config.mjs

@matthewp
Copy link
Contributor

The problem seems to be here:

const toSlotName = (str: string) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w.toUpperCase());

This converts a slot name to a camel-cased version which breaks usage in Astro components. I'm not sure why this code exists. It's possibly so that you can pass these to JSX framework components. But I would think that if a framework component wanted a camel-case name that you would just use slot="camelCased" which should pass through just fine.

Going to write some tests and assume that I can fix this just by not doing that part. cc-ing @natemoo-re in case he has insight.

@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants