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.

Sign up for GitHub

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

fix: <Route> received an unexpected slot "default" #3115

Merged
merged 2 commits into from
Dec 28, 2021
Merged

fix: <Route> received an unexpected slot "default" #3115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cf1dee
fix: <Route> received an unexpected slot "default"
bfanger Dec 27, 2021
a75eda8
Update .changeset/moody-students-report.md
Rich-Harris Dec 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/moody-students-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix `<Route> received an unexpected slot "default"` warning
10 changes: 6 additions & 4 deletions packages/kit/src/core/create_app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,13 @@ function generate_app(manifest_data) {

while (l--) {
pyramid = `
<svelte:component this={components[${l}]} {...(props_${l} || {})}>
{#if components[${l + 1}]}
{#if components[${l + 1}]}
<svelte:component this={components[${l}]} {...(props_${l} || {})}>
${pyramid.replace(/\n/g, '\n\t\t\t\t\t')}
{/if}
</svelte:component>
</svelte:component>
{:else}
<svelte:component this={components[${l}]} {...(props_${l} || {})} />
{/if}
Comment on lines +133 to +139
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to do this trick in my other projects, but a caveat I found is that this would also rerender the component if only the child components change, which is sometimes undesirable, especially for route transitions. And so, I think we should fix the warning in Svelte side, and leave the code as is for now to prevent a potential breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does that happen? Are there situations where the upper level component stays the same but the underlying level changes to falsy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also curious about this — if we can fix it here rather than in Svelte, that would be preferable I think, since the warning is useful.

The components 'branch' is an array of nodes, where every node except the leaf is a layout. Layouts always have a slotted child, leaves never do. Which means that I don't think there's ever a scenario in which a component could move between the consequent and the alternate — layouts will always be in the consequent, leaves will always be in the alternate.

In other words this is a brilliantly simple (and in hindsight, obvious!) fix for this issue as far as I can see — thank you @bfanger! But I'd love to better understand @bluwy's concern before merging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks for the explanation. I wasn't sure what the components array would contain, but if the quirk I mention never happens. This looks good to me!

`
.replace(/^\t\t\t/gm, '')
.trim();
Expand Down