Improved snippets support

[JReinhold]

While working with the addon recently, I discovered a few areas where our support for Svelte snippets is suboptimal, and I think we can improve them. I'm listing them all out here because I think they are all related, but in essence we could approve/decline and implement each idea individually if we want to.

I think doing these would truly make snippets first class citizens of Storybook, which would be a great win for the community.

Throughout the RFC we'll be working with this Layout component, which relies heavily on snippets:

Layout.svelte

<script>
  let { header, children, footer, highlighted = false } = $props();
</script>

<div id="header-container">
  {@render header()}
</div>
<main style:font-weight={highlighted ? 'bold' : 'normal'}>
  {@render children()}
</main>
<div id="footer-container">
  {@render footer()}
</div>

and in an application it would be used like this:

App.svelte

<script>
	import Layout from './Layout.svelte';
</script>

<Layout highlighted>
	{#snippet header()}
	  This is the header snippet
	{/snippet}
	This is children
	{#snippet footer()}
	  This is the footer snippet
	{/snippet}
</Layout>

Playground: https://svelte.dev/playground/2dd31039c3144aa9b987072ea245b3de?version=5.28.2

Problem 1: Snippets can't be args

Args in stories is a great construct, because it allows you to easily set them per-story. Args are also what powers the Controls panel, making it easy for users to try out their components with different args.

The problem is, that Controls can only really work with primitive-like values - strings, numbers, objects, booleans, etc. - and snippets are functions. So while you can set an arg to be a snippet, the control for it would be a stringified function which is both confusing and unusable.

If you try and set a snippet like the header-prop to just be a primitive arg like 'My header content', Svelte will fail because it is expecting the prop to be a snippet-function, not a plain string.

The workaround today is that users define a template that converts the plain-string arg to a snippet:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Layout from './Layout.svelte';

  const { Story } = defineMeta({
    component: Layout,
    args: {
      header: 'Some header content' as any,
      children: 'Some main content' as any,
      footer: 'Some footer content' as any,
    },
    render: template,
  });
</script>

{#snippet template(args: Args<typeof Story>, context: StoryContext<typeof Story>)}
  <Layout {...args}>
	{#snippet header()}
	  {args.header}
	{/snippet}
	{args.children}
	{#snippet footer()}
	  {args.footer}
	{/snippet}
  </Layout>
{/snippet}

<Story name="Default" />

<Story name="Highlighted" args={{ highlighted: true }} />

<Story name="Custom layout">
	{#snippet template(args: Args<typeof Story>, context: StoryContext<typeof Story>)}
	  <Layout {...args}>
		{#snippet header()}
		  Prepending header args because reasons
          {args.header}
		{/snippet}
		{args.children}
		{#snippet footer()}
		  {args.footer}
		{/snippet}
	  </Layout>
	{/snippet}
</Story>

Proposal A: Convert args to snippets automatically

We could convert specific args to snippets automatically behind the scenes, so users wouldn't have to do it manually with a verbose template.

But how? We can't know upfront which args should be snippets, and which should be left as-is. You might be tempted to use our automatic argTypes inference to find all props typed as Snippet, but we should not let argTypes control how stories render. ArgTypes are purely for documentation and manager purposes, and a story should render the exact same if argTypes was defined or not.
This wasn't always the case, but we've seen that argTypes generation slows down building and rendering a lot, so we're moving towards an architecture where argTypes inference and handling is lazy, not blocking the story rendering pipeline.

This also rules out the user specifying which args should be treated as args in the argTypes definition.

Also, this is highly specific to Svelte, and the story annotations API (title, args, argTypes, decorators, etc.) is framework agnostic, so we shouldn't expand that API with Svelte-specific stuff.

I propose that we add a second options argument to defineMeta, that is specific to Svelte CSF. Down the line this could be expanded to configure more things in Svelte CSF, but right now it could contain a single snippetize (naming feedback welcome), that users would use to tell Storybook which args to convert to snippets:

const { Story } = defineMeta({
  component: Layout,
  args: {
    header: 'Some header content',
    children: 'Some main content',
    footer: 'Some footer content',
  },
  render: template,
}, {
  snippetize: {
    header: true,
    children: true,
    footer: true,
  }
});

snippetize could default to being { children: true } - and a custom snippetize should merge with that - because children is sort of a reserved word in Svelte: https://svelte.dev/docs/svelte/snippet#Passing-snippets-to-components-Implicit-children-snippet

However you can override that by setting { children: false }. if you have a component that supports <MyComp children="plain string" />. We could also say we don't support that, because it is an anti-pattern and Svelte docs state:

Note that you cannot have a prop called children if you also have content inside the component — for this reason, you should avoid having props with that name

an alternative API would be an array, snippetize: ['header', 'children', 'footer'], however the merging-heuristics for that is less obvious to me and to users.

A full solution would look like this:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Layout from './Layout.svelte';

  const { Story } = defineMeta({
    component: Layout,
    args: {
      header: 'Some header content' as any,
      children: 'Some main content' as any,
      footer: 'Some footer content' as any,
    }
  }, {
    snippetize: {
      header: true,
      footer: true,
    }
  });
</script>

<Story name="Default" />

<Story name="Highlighted" args={{ highlighted: true }} />

<Story name="Custom layout">
	{#snippet template(args: Args<typeof Story>, context: StoryContext<typeof Story>)}
	  <Layout {...args}>
		{#snippet header()}
		  Prepending header args because reasons
          {args.header}
		{/snippet}
	  </Layout>
	{/snippet}
</Story>

Two wins:

1. The default template is no longer necessary, as the args are now snippets
2. The custom template in the "Custom layout" story is now smaller, because we don't have to convert children and footer to snippets, because they are already snippets in the {...args} spread.

Unresolved questions

1. This does affect the code snippet generation (the "Show Code"-content), and I'd actually expect that to be the most complex change of this feature.
2. Is there something about inheritance we need to consider here? As snippetize isn't part of the story annotations, it won't automatically follow Storybook's inheritance-model with preview.js -> meta -> story. However I doubt it makes sense to globally define which prop names to snippetize in preview.js, as it would be a per-component thing. Similarly I doubt there is a use case for setting it at a story-level - you wouldn't have a component that supports a prop both being a snippet and a primitive at the same time, would you? Maybe I'm missing something.
3. The typings for this could be hard. They need to understand, that when you've set snippetize, it means that the arg you set in defineMeta can now be a primitive and doesn't need to match the Snippet type derived from the component's prop types. And when you define a custom template that takes in args, it needs to understand that those args are now snippets, not primitives. A simpler solution would be to always convert all Snippet-types to be Snippet | AllThePrimitives (I tried this once but failed, I'm sure it's doable though).

Proposal B: Automatically convert children arg to snippet

Only and always* convert the children arg to a snippet. An argument is that named snippets are niche, and so requiring a bit of gymnastics to make them work is okay. But we know that children should always be a snippet, so we can convert that safely. This means we don't need the snippetize-API at all, and in general makes this a lot easier to implement.

*: Unless it is already a snippet. booleans probably also should not be converted, in reality only string and number makes sense to snippetize.

Problem 2: Story children cannot be used with templates

In #228 we introduced the feature that a Story's children would be forwarded as children to the component:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Button from './Button.svelte';

  const { Story } = defineMeta({
    component: Button,
  });
</script>

<Story name="Default">
  I am forwarded to button as the children snippet
</Story>

This made it easier to write stories with children snippets, however it came with one limitation: you can't mix Story children with template. This is not allowed:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Button from './Button.svelte';

  const { Story } = defineMeta({
    component: Button,
    render: template,
  });
</script>

{#snippet template(args)}
  <Button {...args}>
    child prefix - {args.children}
  </Button>
{/snippet}

<Story name="Default">
  I should have been forwarded, but I'm throwing an error
</Story>

But it doesn't have to be that way, we can support this!

Proposal: Merge Story children into args

What we could do, is instead of making it a completely separate rendering case when Story has children, we just merge the Story-component's children into the args, overriding the existing args.children if it's there.

It's not a huge win, but it is actually easier to maintain for us, and it's one less thing to know as a user, it simplifies the mental model: "children on Story is a shortcut for args.children, because writing args.children manually with a snippet is cumbersome".

FYI if users were to do this manually today (which they shouldn't), they would achieve it like this:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Button from './Button.svelte';

  const { Story } = defineMeta({
    component: Button,
    render: template,
  });
</script>

{#snippet template(args)}
  <Button {...args}>
    child prefix - {args.children}
  </Button>
{/snippet}

{#snippet childrenForDefaultStory()}
  children arg
{/snippet}

<Story name="Default" args={{ children: childrenForDefaultStory }} />

{#snippet childrenForOtherStory()}
  other children arg
{/snippet}

<Story name="Default" args={{ children: childrenForOtherStory }} />

Again, we need to ensure that the code snippet generation logic can support this case

Problem 3: Passing snippets requires a template

Continuing from the Problem 2 above, why isn't this supported for any snippets in a Story? You can't pass any snippet to a Story and expect it to be forwarded, only children. You can't do this today:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Layout from './Layout.svelte';

  const { Story } = defineMeta({
    component: Layout,
  });
</script>

<Story name="Complex snippets">
  {#snippet header()}
    I will be completely ignored
  {/snippet}

  I am children, I will be forwarded to `Layout`

  {#snippet footer()}
    I will also be ignored
  {/snippet}
</Story>

Although it seems like a natural mental model to have from a user perspective: "any snippet passed to Story (including implicit children) will be set as args on the story, ie. forwarded to the component or template".

You might think that this is unnecessary if Problem 1 is solved, however Problem 1 is about turning primitve args into snippets, while the use case for this problem is when you have complex snippets that can't be expressed with simple strings, ie. because they contain other components or Svelte constructs, so they need to be snippets.

Proposal: Merge any snippets passed to Story into args

Expand the idea of Problem 2, to not just cover children, but any snippet passed to Story will merge into args, overriding any existing arg with the same name. However there are a few gotchas:

Firstly, Story already accepts a template-snippet. It is a reserved keyword in a sense, it won't be forwarded to args obviously. We also can't forward story annotations like play or beforeEach, because you'd get props collisions if you try to define both a prop and a snippet with that name. Should you have a component that accepts a reserved keyword snippet itself, the workaround is to use that directly (as it is the case already today):

<Story name="My Story">
  {#snippet template(args)}
    <MyComponent {...args}>
      {#snippet template()}
        This is the template being passed to MyComponent
      {/snippet}
      {#snippet play()}
        This is the play snippet being passed to MyComponent
      {/snippet}
    </MyComponent>
  {/snippet}
</Story>

Secondly, Svelte currently doesn't expose an API that allows us to check if something is a snippet or not (See sveltejs/svelte#14017, sveltejs/svelte#9774 and sveltejs/svelte#9903), it seems like the maintainers don't think the currently known use cases are valid or perhaps an anti-pattern. Maybe this will change that, I don't know. I've heard they are great people.

For us it means we can only check if any given prop to Story is a function, which doesn't necessarily mean it is a snippet. Many story annotations are functions, eg. this shouldn't become a snippet: <Story play={() => {...}} />. There are a few solutions to this:

1. Try to hack our way through Svelte's internals and find something that identifies a snippet, potentially via the function's toString. This is fragile if the internals ever change in a Svelte version.
2. Define Storybook's reserved keywords upfront (story annotations), and have that as an ignore list of prop names that should not be forwarded as snippets, even though they are functions. Eg. const IGNORED_PROPS = ['play', 'beforeEach', 'decorators', ...]. This is fragile if Storybook decides to change story annotations in the future, ie. in the near future experimental_afterEach could be renamed to afterEach, which we would need to handle. However this is within our control, so easier to maintain.
3. Try to persuade the Svelte team to expose an API that allows you to check if a given variable is a snippet or not.

If we implemented this proposal, it would make the above Just Work™:

<script module lang="ts">
  import { defineMeta, type Args, type StoryContext } from '@storybook/addon-svelte-csf';
  import Layout from './Layout.svelte';

  const { Story } = defineMeta({
    component: Layout,
  });
</script>

<Story name="Complex snippets">
  {#snippet header()}
    I am header
  {/snippet}

  I am children

  {#snippet footer()}
    I am footer
  {/snippet}
</Story>

[JReinhold]

Paraphrased feedback from someone:

snippets are not a common use case, and named snippets are even less common. I think it's okay to require a bit of gymnastics to make named snippets work, because the effort to support that is high compared to amount of users it actually helps

[JReinhold]

As a reaction to this, I've written up a Proposal B for Problem 1:

Only and always* convert the children arg to a snippet. An argument is that named snippets are niche, and so requiring a bit of gymnastics to make them work is okay. But we know that children should always be a snippet, so we can convert that safely. This means we don't need the snippetize-API at all, and in general makes this a lot easier to implement.

*: Unless it is already a snippet. booleans probably also should not be converted, in reality only string and number makes sense to snippetize.

Even if we disregard named templates (which I'm strongly considering now too), Problem 2 is still valid because it's still concerned with children and it improves the maintainability of the addon IMO.

Problem 3 could be denied because of this.