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

$$slots with slot forwarding #6059

Closed
techniq opened this issue Mar 6, 2021 · 18 comments
Closed

$$slots with slot forwarding #6059

techniq opened this issue Mar 6, 2021 · 18 comments

Comments

@techniq
Copy link

techniq commented Mar 6, 2021

Describe the bug
If you forward a slot (ex. <slot name="label" slot="label" />) the slot is always considered passed to the parent even if not passed to the child. This breaks any conditional logic based on passed slots within the parent.

To Reproduce
Here is a REPL demonstrating the issue: https://svelte.dev/repl/d0cd92776d534349acf8e3e2deb074fa?version=3.35.0 (see console output and presence of red description <div>).

Expected behavior
Referencing the REPL, I would expect $$slots.description to not be present as a passed slot in the parent Field component, just like $$slots.description within TextField.

Severity
The only workaround is to duplicate code across components and not leverage composition with slot forward, which isn't terrible nor optimal.

@FilipJakab
Copy link

I struggle with same issue..
I would welcome that the svelte compiler would mark slots that result in empty markup (or consisting of whitespaces) as 'not provided'.
Thus allowing us to write constructs like conditional rendering of slots.

@techniq
Copy link
Author

techniq commented Mar 13, 2021

@TheMaikXX While I struggle with that issue as well, I think that is more related to #5312 where the slot is defined, but rendered empty from a {#if} conditional. This issue is the lack of slot usage with forwarding.

Given some thought, both these issues might be solved in the runtime (and not compiler) by using a MutationObserver and watching childList changes. Would be great to be handled by the compiler and $$slots, but might be at least a workaround. I haven't had time to experiment to confirm though...

@FilipJakab
Copy link

@TheMaikXX While I struggle with that issue as well, I think that is more related to #5312 where the slot is defined, but rendered empty from a {#if} conditional. This issue is the lack of slot usage with forwarding.

I mean something little bit different... Here I created very similar REPL to yours: https://svelte.dev/repl/36e864ce20704538a32a063f77e51981?version=3.35.0
But it more clearly illustrates what I was talking about.
When I inspect the generated HTML:
image
The div <div class="header"></div> is empty so the slot header must be empty as well. And I think that this case could be handled (at least partially) at compile-time to say something like (from Child's point of view): "The slot is given to me but it resolved into no content so I will treat it as not provided at all."
Or it could be handled by the user if he would have access to the inner content the slots so that the user could check if the slot is not empty..

@ecstrema
Copy link
Contributor

Or it could be handled by the user if he would have access to the inner content the slots so that the user could check if the slot is not empty..

If $$slots contained a reference to the passed HtmlElement instead of a simple true, you are right that it would be easy to check. And i see many other perks to it.

But that would still only be a workaround to fix slot forwarding.

@techniq
Copy link
Author

techniq commented Oct 6, 2021

One slight workaround for some use cases is to use :empty to apply display: none on the block (or empty:hidden class for tailwind). REPL

This doesn't solve a lot of other uses cases, and also falls apart if you have any markup within it (basically more than anything but a simple slot). Even a {#if false}{/if} causes a single space to render which breaks :empty until Selectors Level 4 is implemented by browsers.

@pzuraq
Copy link

pzuraq commented Feb 23, 2022

One slight workaround for some use cases is to use :empty to apply display: none on the block (or empty:hidden class for tailwind). REPL

This doesn't solve a lot of other uses cases, and also falls apart if you have any markup within it (basically more than anything but a simple slot). Even a {#if false}{/if} causes a single space to render which breaks :empty until Selectors Level 4 is implemented by browsers.

Formalized this into a helper component if anyone wants to use it, like @techniq said it doesn't help in all situations, but does work for many small ones: https://svelte.dev/repl/70edd01caf664f29885348853aaf59b1?version=3.46.4

@techniq
Copy link
Author

techniq commented May 6, 2022

Conditional Slots RFC by @tanhauhau would be able to handle this by checking if the slot is declared

<Field>
   {#if $$slots.label}
	<slot name="label" slot="label" />
   {/if}
   {#if $$slots.description}
	<slot name="description" slot="description" />
   {/if}
</Field>

@rodrigodagostino
Copy link

rodrigodagostino commented Mar 17, 2024

I know this is not a fix to our problem, but it’s a workaround that I’m fairly satisfied with, so I wanted to share it here in case it could help someone else.

I simply defined a new variable in the parent component (hasLabel) to store the boolean value of the slot I’m working with, and then passing it along as a prop to a child component (could be implement through context or a store too), as follows:

App.svelte

<FormControl>
  <svelte:fragment slot="label">First name</svelte:fragment>
</FormControl>

FormControl.svelte

let hasLabel = $$slots.label;
---
<script>
  let hasLabel = $$slots.label;
</script>

<div class="form-control">
  <TextField {hasLabel}>
    <slot name="label" slot="label" />
  </TextField>
</div>

TextField.svelte

{#if hasLabel}
  <label><slot name="label" /></label>
  <input type="text">
{/if}

Here’s a REPL: https://svelte.dev/repl/76fec0b94fd94861aa52f747f8ee05af?version=4.2.12

It is additional syntax that we shouldn’t be needing, but this way we can safely rely on the boolean value to show/hide our slot and any surrounding element when there is actual content assigned to it.

@opensas
Copy link
Contributor

opensas commented Mar 19, 2024

@rodrigodagostino similar solution here: https://svelte.dev/repl/1c9c3f468445408f80e7910e8cb8af36?version=3.59.2

I just let caller optionally override the child $$slots variable

a bit of explanation here: #5604 (comment)

@rodrigodagostino
Copy link

Hey, @opensas! Well... You’re not really overriding the $$slots object, you’re simply running your own variable on the side pretty much like I did, but I like how you chose to go with an object instead of a simple boolean, which allows to group multiple slots in a single variable :)

Thank you for taking the time to share that! :)

@dummdidumm
Copy link
Member

Svelte 5 will deprecate slots in favor of snippets, which achieve the desired behavior - therefore closing this issue.

@opensas
Copy link
Contributor

opensas commented Mar 19, 2024

@dummdidumm thanks for your reply and for porting the example to svelte 5

I did a quick test (repl here) and it seems like we can't mix svelte 4 slots with svelte 5 snippets, not yet at least

is such thing planned to be supported at some point?

if that would be the case porting the component that needs to conditionally forward slots to svelte 5 might be a viable alternative, but having to migrate the whole thing to snippets might be pretty hard to do in some cases.

Moreover, as far as I understand this PR from @tanhauhau would solve the issue using svelte 4 slots.

@opensas
Copy link
Contributor

opensas commented Mar 19, 2024

@techniq here's a workaround I'm using myself applied to your example: repl

basically I add an optional slots prop to override $$props of the child component (TextField overrides the $$slots of Field, in this case, telling the child component which slots to render or not)

@brandonmcconnell
Copy link
Contributor

@dummdidumm As we discussed in sveltejs/rfcs#64, this doesn't actually solve the issue at hand in terms of reverse compatibility for v4. @tanhauhau came up with an ideal solution in his RFC that would be a good stop-gap solution as we all progressively make the transition from v5

@dummdidumm
Copy link
Member

We don't have enough resources to implement a new feature for something that is going to be deprecated. We would have to get the PR up to date, and then also reimplement it in the new world, document it, etc etc - this would be a bad use of our time.

@opensas
Copy link
Contributor

opensas commented Mar 19, 2024

@brandonmcconnell as far as I understand @tanhauhau also implemented his RFC in this PR, which hasn't been merged yet

Last thing mentioned was that they were waiting for snippets to stabilize to check how good the PR behaves with svelte 5

I also agree that it would be a good solution while we migrate to svelte 5 snippets

@dummdidumm
Copy link
Member

The comment didn't refer to snippets (they were not even invented by then), it refered to us not knowing what the future looks like, and now that we know snippets will take the place of slots, it doesn't make sense to invest time in a soon-to-be-deprecated feature.

@opensas
Copy link
Contributor

opensas commented Mar 19, 2024

@dummdidumm thanks for the clarification, it's a shame, but it makes sense too

I guess the way to go is to implement some of the mentioned workarounds (they involve somehow passing an additional prop to indicate which slots should or should not be rendered) until we can migrate to svelte 5 snippets

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 a pull request may close this issue.

8 participants