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

🐛 BUG: Dev server "panic"s and gives scary/unhelpful error when using slots that aren't at the root level #3253

Closed
1 task
Jutanium opened this issue May 1, 2022 · 6 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@Jutanium
Copy link
Contributor

Jutanium commented May 1, 2022

What version of astro are you using?

1.0.0-beta.19

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

None

What package manager are you using?

yarn

What operating system are you using?

Mac

Describe the Bug

A user might want to dynamically send named slots to an Astro component by looping over an array:

<ComponentWithSlots>
	{slots.map(name => (<div slot={name}>hi</div>))}
</ComponentWithSlots>

This breaks the compiler because the slots aren't considered at the root of the component. It's similar to this, which also breaks the compiler:

<ComponentWithSlots>
    <div>
	    <div slot={"name"}>
		    hi
	    </div>
    </div>
</ComponentWithSlots>

In either case, the compiler literally panics:

panic: Element with a slot='...' attribute must be a child of a component or a descendant of a custom element

goroutine 9 [running]:
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x50e5a0, {0x0, 0x0, 0x5, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:423 +0x536
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x50e3c0, {0x0, 0x0, 0x4, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:563 +0x45e
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x50e1e0, {0x0, 0x0, 0x3, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:538 +0x4c4
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x4fdd10, {0x0, 0x0, 0x2, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:563 +0x45e
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x4fd1d0, {0x0, 0x0, 0x1, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:563 +0x45e
github.com/withastro/compiler/internal/printer.render1(0x517b08, 0x4fcf00, {0x1, 0x0, 0x0, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, ...}})
        github.com/withastro/compiler/internal/printer/print-to-js.go:110 +0x33
github.com/withastro/compiler/internal/printer.printToJs(0x517b08, 0x4fcf00, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, {0x4541c8, 0x16}, {0x42e380, ...}, ...})
        github.com/withastro/compiler/internal/printer/print-to-js.go:74 +0x2
github.com/withastro/compiler/internal/printer.PrintToJS({0x4f2600, 0x172}, 0x4fcf00, 0x0, {{0x411750, 0x8}, {0x4b05a0, 0x4f}, {0x4541c8, 0x16}, ...})
        github.com/withastro/compiler/internal/printer/print-to-js.go:49 +0xc
main.Transform.func1.1.1({0x4f2600, 0x172}, 0x448360, {{}, 0x7ff8000400000040, 0x4117c8})
        ./astro-wasm.go:240 +0x29
created by main.Transform.func1.1
        ./astro-wasm.go:183 +0x3
exit code: 2
 error   Error: Uh oh, the Astro compiler encountered an unrecoverable error!
      
      Please open
      a GitHub issue using the link below:
      https://github.com/withastro/astro/issues/new?labels=compiler&title=%F0%9F%90%9B+BUG%3A+%60%40astrojs%2Fcompiler%60+panic&body=%23%23%23+Describe+the+Bug%0A++++%0A++++%60%40astrojs%2Fcompiler%60+encountered+an+unrecoverable+error+when+compiling+the+following+file.%0A++++%0A++++**src%2Fpages%2Findex.astro**%0A++++%60%60%60astro%0A++++---%0Aimport+TestAstro+from+%22..%2Fcomponents%2Fglobal%2Fastro%2FTestAstro.astro%22%0Aconst+slots+%3D+%5B%22name%22%5D%3B%0A---%0A%3Chtml+lang%3D%22en%22%3E%0A%09%3Chead%3E%0A%09%09%3Cmeta+charset%3D%22utf-8%22+%2F%3E%0A%09%09%3Cmeta+name%3D%22viewport%22+content%3D%22width%3Ddevice-width%22+%2F%3E%0A%09%09%3Ctitle%3EAstro%3C%2Ftitle%3E%0A%09%3C%2Fhead%3E%0A%09%3Cbody%3E%0A%09%09%3Ch1%3EAstro%3C%2Fh1%3E%0A%09%09%3CTestAstro%3E%0A%09%09%09%3Cdiv%3E%0A%09%09%09%3Cdiv+slot%3D%7B%22name%22%7D%3E%0A%09%09%09%09eeee%0A%09%09%09%3C%2Fdiv%3E%0A%09%09%09%3C%2Fdiv%3E%0A%09%09%3C%2FTestAstro%3E%0A%09%3C%2Fbody%3E%0A%3C%2Fhtml%3E%0A++++%60%60%60%0A++++
    at /Users/jutanium/Documents/Dev/DxD-Astro/DataByDesign-Next/src/pages/index.astro
04:06:02 PM [astro] reload /src/pages/index.astro
 error   Go program has already exited

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-idj9xd?file=src%2Fpages%2Findex.astro&on=stackblitz

Participation

  • I am willing to submit a pull request for this issue.
@tony-sull
Copy link
Contributor

Dynamically named slots aren't supported, any named slot needs to be given a string rather than a variable like <div slot={name}>hi</div>. That's an interesting use case though, could make for a great discussion or RFC!

I'm seeing the panic in the reproduction link as well, but it's actually due to the slot index.astro being conditional - removing the {true && ...} fixes the issue. Closing this as a duplicate of withastro/compiler#383 since that's specific to the conditional slot issue 👍

@Jutanium
Copy link
Contributor Author

Jutanium commented May 3, 2022

@tony-sull That's not correct. Even basic examples without a variable break the compiler:

<h1>Welcome to <span class="text-gradient">Astro</span></h1>
<ComponentWithSlots>
	<div>
		<div slot="name">
			This breaks the compiler
		</div>
	<div>
</ComponentWithSlots>

@tony-sull
Copy link
Contributor

Sorry @Jutanium, looks like I closed this one too early! Not actually sure what happened the first time, I tested the Stackblitz without the conditional slot and it did actually work somehow, now I'm seeing the same panic again

Reopening for further investigation

@tony-sull tony-sull reopened this May 3, 2022
@natemoo-re natemoo-re added s3-large - P3: minor bug An edge case that only affects very specific usage (priority) and removed bb:investigate labels May 10, 2022
@natemoo-re
Copy link
Member

So this is definitely not ideal compiler behavior—conditional slots will never work because we need to be able to statically analyze them, but we need to surface a more friendly error.. Unforuntatley reworking the compiler to surface errors instead of panicking will be a pretty large undertaking. Something we need to prioritize.

As for the nested slots, how should this work? Slots by definition project their children into certain areas. Would content be plucked out of the parent element?

@Jutanium
Copy link
Contributor Author

@FredKSchott assured me it was a small issue but I had my doubts <3
Nested slots shouldn't work. Unlike conditional slots, they're not something you'd expect to work, so I feel good about that.
As for conditional slots: this gets into a broader discussion of meeting the users' expectations. Astro's named slots work super differently to Vue and Svelte's slots, and we should talk about that!

@matthewp
Copy link
Contributor

matthewp commented Oct 6, 2022

@matthewp matthewp closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

No branches or pull requests

5 participants