-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix: avoid migrating slots in custom elements #13406
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
Conversation
🦋 Changeset detectedLatest commit: 1af292c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
{/if} | ||
|
||
{#if $$slots.bar} | ||
{$$slots} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This begs an interesting question ... what about $$slots
in the context of custom elements? $$slots
is also legacy/deprecated syntax and as such shouldn't be used anymore. Is it even feasible/necessary to use that in the context of custom elements? @Theo-Steiner any real world use cases from your end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had exactly the same question but when i tried to use $$slots
in a custom element it's actually populated so it might be used? I don't think anyone uses it but i would still leave it untouched in case of custom elements because the migration removes it in favor of checking the prop for falsy values which will not be here if we don't migrate slots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unfortunate because it may mean that after $$slots
are gone people will have to migrate away from it, using something like https://developer.mozilla.org/en-US/docs/Web/API/HTMLSlotElement/assignedNodes instead. As you say it's probably super rare in practise, but it's something to keep in mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this just means
$$slots.bar
will become
$host().shadowRoot.querySelector("slot[name=bar]").assignedNodes()
which is not too bad either IMO
EDIT: forgot the assignedNodes
Svelte 5 rewrite
No issue, it was reported by Theo on discord.
Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (
main
).If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is
svelte-4
and notmain
.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint