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

Svelte: Fix usage of deprecated <svelte:component> #29405

Closed
wants to merge 7 commits into from
+8 −48
Closed

Svelte: Fix usage of deprecated <svelte:component> #29405

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b259f5e
fix(renderer/svelte): Resolve usage of deprecated `<svelte:component>`
xeho91 Oct 19, 2024
8d3dcce
Merge branch 'next' into svelte-component-warning
JReinhold Oct 24, 2024
4b76394
Merge branch 'next' into svelte-component-warning
JReinhold Oct 24, 2024
398acfb
Merge branch 'next' into svelte-component-warning
JReinhold Oct 24, 2024
ecf3a55
update snapshots
JReinhold Oct 24, 2024
2f53a0b
update snapshots again
JReinhold Oct 25, 2024
f028acb
Merge branch 'next' into svelte-component-warning
JReinhold Oct 28, 2024
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
24 changes: 0 additions & 24 deletions .../renderers/svelte/src/__test__/composeStories/__snapshots__/portable-stories.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
exports[`Renders CSF2Secondary story 1`] = `
<body>
<div>
<!---->
<!---->
<button
class="storybook-button storybook-button--medium storybook-button--secondary"
Expand All @@ -15,23 +14,19 @@ exports[`Renders CSF2Secondary story 1`] = `
</button>



</div>
</body>
`;

exports[`Renders CSF2StoryWithParamsAndDecorator story 1`] = `
<body>
<div>
<!---->
<!---->
<!---->
<!---->
<div
data-testid="local-decorator"
style="margin: 3em;"
>
<!---->
<button
class="storybook-button storybook-button--medium storybook-button--secondary"
style=""
Expand All @@ -40,22 +35,18 @@ exports[`Renders CSF2StoryWithParamsAndDecorator story 1`] = `
foo
<!---->
</button>

<!---->
</div>





</div>
</body>
`;

exports[`Renders CSF3Button story 1`] = `
<body>
<div>
<!---->
<!---->
<button
class="storybook-button storybook-button--medium storybook-button--secondary"
Expand All @@ -67,15 +58,13 @@ exports[`Renders CSF3Button story 1`] = `
</button>



</div>
</body>
`;

exports[`Renders CSF3ButtonWithRender story 1`] = `
<body>
<div>
<!---->
<!---->
<div>
<p
Expand All @@ -96,15 +85,13 @@ exports[`Renders CSF3ButtonWithRender story 1`] = `
</div>



</div>
</body>
`;

exports[`Renders CSF3InputFieldFilled story 1`] = `
<body>
<div>
<!---->
<!---->
<input
data-testid="input"
Expand All @@ -113,15 +100,13 @@ exports[`Renders CSF3InputFieldFilled story 1`] = `
/>



</div>
</body>
`;

exports[`Renders CSF3Primary story 1`] = `
<body>
<div>
<!---->
<!---->
<button
class="storybook-button storybook-button--large storybook-button--primary"
Expand All @@ -133,15 +118,13 @@ exports[`Renders CSF3Primary story 1`] = `
</button>



</div>
</body>
`;

exports[`Renders LoaderStory story 1`] = `
<body>
<div>
<!---->
<!---->
<div>
<div
Expand All @@ -158,23 +141,19 @@ exports[`Renders LoaderStory story 1`] = `
</div>



</div>
</body>
`;

exports[`Renders NewStory story 1`] = `
<body>
<div>
<!---->
<!---->
<!---->
<!---->
<div
data-testid="local-decorator"
style="margin: 3em;"
>
<!---->
<button
class="storybook-button storybook-button--large storybook-button--primary"
style=""
Expand All @@ -183,14 +162,11 @@ exports[`Renders NewStory story 1`] = `
foo
<!---->
</button>

<!---->
</div>





</div>
</body>
`;
8 changes: 4 additions & 4 deletions code/renderers/svelte/src/components/SlotDecorator.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@
</script>

{#if decorator}
<svelte:component this={decorator.Component} {...decorator.props} bind:this={decoratorInstance}>
<svelte:component this={Component} {...propsWithoutDocgenEvents} bind:this={instance} />
</svelte:component>
<decorator.Component {...decorator.props} bind:this={decoratorInstance}>
<Component {...propsWithoutDocgenEvents} bind:this={instance} />
</decorator.Component>
{:else}
<svelte:component this={Component} {...propsWithoutDocgenEvents} bind:this={instance} />
<Component {...propsWithoutDocgenEvents} bind:this={instance} />
{/if}
24 changes: 4 additions & 20 deletions ...storybooks/portable-stories-kitchen-sink/svelte/stories/__snapshots__/Button.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,14 @@ exports[`Renders CSF2Secondary story 1`] = `

</button>


</div>
</body>
`;

exports[`Renders CSF2StoryWithParamsAndDecorator story 1`] = `
<body>
<div>
<div
style="margin: 3em;"
>
<decorator.component>
Copy link
Contributor

Choose a reason for hiding this comment

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

This exposes a bug in Testing Library Svelte and/or Svelte 4.

When you use <something.theComponent />, it will snapshot as that directly instead of what the component actually is, as we see in the diff here. I confirmed that it is unrelated to Storybook/Portable Stories and that it's only a problem in Svelte 4. I wrote this simple test setup:

// Wrapper.svelte
<div style="margin: 3em;">
  <slot />
</div>
// Inner.svelte
<script>
  export let stuff;
</script>

<stuff.Component> hello </stuff.Component>
// the.test.js
import { it, expect } from 'vitest';
import { render } from '@testing-library/svelte';
import Inner from './Inner.svelte';
import Wrapper from './Wrapper.svelte';

it('barebone', () => {
  render(Inner, { stuff: { Component: Wrapper }});
  expect(document.body).toMatchInlineSnapshot();
});

Using <stuff.Component> hello </stuff.Component> it had this weird flaw in the snapshot, using <svelte:component this={stuff.Component}>hello</svelte:component> it didn't.

I think this is okay honestly, not much we can do about it other than not fixing the warning here. Thoughts @yannbf @kasperpeulen?

Copy link
Member

Choose a reason for hiding this comment

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

It was discussed with @JReinhold that he will try to apply version-dependent logic so that the decorator.Component is only used in Svelte 5

<button
class="storybook-button storybook-button--medium storybook-button--secondary"
style=""
Expand All @@ -31,10 +28,7 @@ exports[`Renders CSF2StoryWithParamsAndDecorator story 1`] = `
foo

</button>

</div>


</decorator.component>


</div>
Expand All @@ -53,7 +47,6 @@ exports[`Renders CSF3Button story 1`] = `

</button>


</div>
</body>
`;
Expand All @@ -78,7 +71,6 @@ exports[`Renders CSF3ButtonWithRender story 1`] = `
</button>
</div>


</div>
</body>
`;
Expand All @@ -90,7 +82,6 @@ exports[`Renders CSF3InputFieldFilled story 1`] = `
data-testid="input"
/>


</div>
</body>
`;
Expand All @@ -107,7 +98,6 @@ exports[`Renders CSF3Primary story 1`] = `

</button>


</div>
</body>
`;
Expand All @@ -129,17 +119,14 @@ exports[`Renders LoaderStory story 1`] = `
</div>
</div>


</div>
</body>
`;

exports[`Renders NewStory story 1`] = `
<body>
<div>
<div
style="margin: 3em;"
>
<decorator.component>
<button
class="storybook-button storybook-button--large storybook-button--primary"
style=""
Expand All @@ -148,10 +135,7 @@ exports[`Renders NewStory story 1`] = `
foo

</button>

</div>


</decorator.component>


</div>
Expand Down