search

storybookjs / addon-svelte-csf

[Incubation] CSF using Svelte components.
MIT License
98 stars 29 forks source link

Experimental support for Svelte 5 #181

Open xeho91 opened 1 month ago

xeho91 commented 1 month ago

[!CAUTION] This is still a work in progress 🚧. Breaking changes may occur at any time, so do not use it for other purposes than experimenting yet.

Credits

Additional goals

  1. Simplify the usage - align closer to regular CSF format, without extras
  2. Take advantage of the Svelte 5 features
  3. Improve examples demonstration

Resources

[!TIP] Call for contribution on the demo (by add missing or improving examples) to help us find potential issues. Some of them we can fix the next major version 🙏.

Breaking changes

New features

  1. Improved TypeScript experience - defineMeta({ ... }) - works in TypeScript and with JSDoc
  2. Added a function setTemplate to allow adding a default {#snippet children()} to all of the <Story> in the stories file.
  3. Added type helpers Args<typeof Story> and StoryContext<typeof Story> to get typings while creating Svelte v5 snippets outside the <Story />

TODO

Issues

Legend:

Related to tests

  1. 🟡 Tests emit error that the components should use render from svelte/render instead of mount - I'm not sure how to deal with this one yet. Perhaps using environment variable TEST?
  2. 🟡 Related to above. It seems we can't use setContext during the tests - we used' the render instead of mount, it will break while running vitest, AFAIK.
  3. 🟡 I can't find a way to inject other Vite plugin's transformation (from @storybook/vite-builder & @storybook/svelte-vite). Based the compiled output, I want to test the snippets which analyse/extract AST nodes from the compiled output. To see if creating appendix to the compiled output works as expected.

Related to runtime

  1. Actions tab doesn't work, I couldn't figure out whether is related to the breaking change that on:* should be replaced with on* (removed colon)
  2. Typings for args used in {#snippet children(args)} - no idea yet how to crack this one
  3. Setting the layout in export meta parameters is broken
  4. Importing the zimmerframe fails due to vite not being able to get exports correctly - current workaround to continue developing: manual override import to default in ./node_modules/zimmerframe/package.json.
  5. 🔴 Inserting HTML comments as description & <Story> doesn't work in the development mode - see: https://github.com/storybookjs/addon-svelte-csf/pull/181#issuecomment-2143379765

Related to TypeScript

  1. 🟡 Stories with required children snippet, and not being explicitly set. See comment: https://github.com/storybookjs/addon-svelte-csf/pull/181#issuecomment-2130744977
  2. 🟡 https://github.com/storybookjs/addon-svelte-csf/pull/181#issuecomment-2132124184
📦 Published PR as canary version: 5.0.0--canary.181.168fccb.0
:sparkles: Test out this PR locally via: ```bash npm install @storybook/addon-svelte-csf@5.0.0--canary.181.168fccb.0 # or yarn add @storybook/addon-svelte-csf@5.0.0--canary.181.168fccb.0 ```
xeho91 commented 1 month ago

Turns out I was wrong before. I am not sure how I made the wrong assumption in my research. I think confused object destructuring & fields renaming, with setting a type. You can actually set a parameter type in snippet's function, e.g.:

{#snippet template({ args, context }: Template<Component>)}
<!-- ... -->
{/snippet}

Full example on how we could use templating, by removing the <Template /> component. At the moment is type-safe by using TypeScript only for now.

<script context="module" lang="ts">
  import type { Meta } from '@storybook/svelte';

  import Text from './Text.svelte';

  /**
   * Demonstration on how to use multiple templates in one stories file,
   * powered by Svelte's **snippets**.
   */
  export const meta: Meta<Text> = {
    title: 'Templates',
    tags: ['autodocs'],
  };
</script>

<script lang="ts">
  import { Story, type Template } from '../src/index.js';
</script>

{#snippet template1({ args }: Template<Text>)}
  <h2 style="color: lightgreen">Template 1</h2>
  <p>{args.text}</p>
{/snippet}

{#snippet template2({ args }: Template<Text>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args.text}</p>
{/snippet}

<Story
  name="Story with template 1"
  children={template1}
  args={{ text: 'This story uses the first template' }}
/>

<Story
  name="Story with template 2"
  children={template2}
  args={{ text: 'This story uses second template' }}
/>

<Story
  name="Custom template"
  args={{ text: 'This story uses custom template passed as children' }}
>
  {#snippet children({ args })}
    <h2 style="color: orange; font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>{args.text}</p>
    <hr>
  {/snippet}
</Story>

Questions

  1. Currently the snippet accepts only one argument - which is an object:

    export type Template<
Component extends SvelteComponent,
Props = NonNullable<Partial<ComponentProps<Component>>>,
> = {
args: Props;
context: StoryContext<Props>;
};

I'm thinking if this is the good direction? Because if we'd like to follow the type of known function render(args, storyContext) function, then in order to obtain the storyContext only, we would have to write like this:

{#snippet template(_args: Args, context: StoryContext))}
<!-- ... -->
{/snippet}

which is less convenient in the usage. Not to mention it doesn't really do the 'rendering', in my understanding.

JReinhold commented 1 month ago

This is interesting, I like this.

  1. You don't need to import the Story and Template type in a normal script right, you could have done it all in the context="module" script?

  2. With this setup, if you wanted a static story that doesn't react to args, you could ditch the snippet altogether right? Would be cool.

<Story
  name="static template"
>
    <h2 style="color: orange; font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>some text</p>
    <hr>
</Story>
  1. I think being consistent with the core render function parameters would be better than making it slightly easier to get StoryContext, so I'd say this is better:
{#snippet template(args: ComponentProps<MyComponent>, context: StoryContext)}
<!-- ... -->
{/snippet}

It's rare that users need to read the context in a template, so giving them just args is easier for users in 95 % of cases. it's even rarer to not need args and only context, so I don't think we should optimize for that. In fact that used to be what the render function received, just one big context, but in reality most users just wanted args so it was split up into two parameters back in SB 6 IIRC.

We could perhaps make a helper type for getting the Args type, which would essentially (for now) just alias to ComponentProps. Bringing it all together:

<script context="module" lang="ts">
  import type { Meta } from '@storybook/svelte';
  import { Story, type Args } from '../src/index.js';

  import Text from './Text.svelte';

  /**
   * Demonstration on how to use multiple templates in one stories file,
   * powered by Svelte's **snippets**.
   */
  export const meta: Meta<Text> = {
    title: 'Templates',
    tags: ['autodocs'],
  };
</script>

{#snippet template1(args: Args<Text>)}
  <h2 style="color: lightgreen">Template 1</h2>
  <p>{args.text}</p>
{/snippet}

{#snippet template2(args: Args<Text>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args.text}</p>
{/snippet}

<Story
  name="Story with template 1"
  children={template1}
  args={{ text: 'This story uses the first template' }}
/>

<Story
  name="Story with template 2"
  children={template2}
  args={{ text: 'This story uses second template' }}
/>

<Story
  name="Custom template"
  args={{ text: 'This story uses custom template passed as children' }}
>
  {#snippet children(args)}
    <h2 style="color: orange; font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>{args.text}</p>
    <hr>
  {/snippet}
</Story>
JReinhold commented 1 month ago

Did you find a way to make <Story /> typesafe from just inferring from the children snippet, or have you just omitted that part here?

Eg. for:

{#snippet template(args: Args<Text>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args.text}</p>
{/snippet}

<Story
  name="Story with template"
  children={template}
  args={{ text: 'This story uses the first template' }}
/>

Will the args prop on the story be typesafe based on the Args<Text> type from the template snippet?

xeho91 commented 1 month ago
  1. You don't need to import the Story and Template type in a normal script right, you could have done it all in the context="module" script?

I haven't had a chance to update the parser by the time you asked this question. I've removed completely parsing of the "instance" which is the normal <script> tag in the svelte file, and moved it to "module" parsing aka <script context="module">.

  1. With this setup, if you wanted a static story that doesn't react to args, you could ditch the snippet altogether right? Would be cool.
<Story
  name="static template"
>
    <h2 style="color: orange; font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>some text</p>
    <hr>
</Story>

That's correct! The property children is optional.

At the moment, this is the way how it works:

  1. If the <Story> has nested children, it will use nested children
  2. If the <Story> has {children} prop, it will use the template from this snippet
  3. If the setTemplate(snippet) was used in the Stories file, then the <Story> will use the template set by setTemplate via Svelte context.
  1. I think being consistent with the core render function parameters would be better than making it slightly easier to get StoryContext, so I'd say this is better:
{#snippet template(args: ComponentProps<MyComponent>, context: StoryContext)}
<!-- ... -->
{/snippet}

Alright, no problem. I'll save it on the TODO list. EDIT: ✅

It's rare that users need to read the context in a template, so giving them just args is easier for users in 95 % of cases. it's even rarer to not need args and only context, so I don't think we should optimize for that. In fact that used to be what the render function received, just one big context, but in reality most users just wanted args so it was split up into two parameters back in SB 6 IIRC.

We could perhaps make a helper type for getting the Args type, which would essentially (for now) just alias to ComponentProps. Bringing it all together:

<script context="module" lang="ts">
  import type { Meta } from '@storybook/svelte';
  import { Story, type Args } from '../src/index.js';

  import Text from './Text.svelte';

  /**
   * Demonstration on how to use multiple templates in one stories file,
   * powered by Svelte's **snippets**.
   */
  export const meta: Meta<Text> = {
    title: 'Templates',
    tags: ['autodocs'],
  };
</script>

{#snippet template1(args: Args<Text>)}
  <h2 style="color: lightgreen">Template 1</h2>
  <p>{args.text}</p>
{/snippet}

{#snippet template2(args: Args<Text>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args.text}</p>
{/snippet}

<Story
  name="Story with template 1"
  children={template1}
  args={{ text: 'This story uses the first template' }}
/>

<Story
  name="Story with template 2"
  children={template2}
  args={{ text: 'This story uses second template' }}
/>

<Story
  name="Custom template"
  args={{ text: 'This story uses custom template passed as children' }}
>
  {#snippet children(args)}
    <h2 style="color: orange; font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>{args.text}</p>
    <hr>
  {/snippet}
</Story>

Will it be a problem if this Args type helper will conflict with the type Args from @storybookjs/svelte?

EDIT: I've created temporary TArgs<Meta> and TContext<Meta> helper types

xeho91 commented 1 month ago

I couldn't get rid of the idea you planted in my mind, by making defineMeta instead. It's possible and it works on examples so far - I'll test on my hobby project tomorrow.

There's the recent example (includes templating as well) :

<script context="module" lang="ts">
  import { defineMeta, type TArgs } from '../src/index.js';

  import Text from './Text.svelte';

  /**
   * Demonstration on how to use multiple templates in one stories file,
   * powered by Svelte's **snippets**.
   */
  const { Story, meta } = defineMeta({
    title: 'Templates',
    component: Text,
    tags: ['autodocs'],
    args: { text: "" },
    argsTypes: {
      text: { control: "text" },
    },
  });
</script>

{#snippet template1(args: TArgs<typeof meta>)}
  <h2 style="color: lightgreen">Template 1</h2>
  <p>{args?.text}</p>
{/snippet}

{#snippet template2(args: TArgs<typeof meta>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args?.text}</p>
{/snippet}

<Story
  name="Story with template 1"
  children={template1}
  args={{ text: 'This story uses the first template' }}
/>

<Story
  name="Story with template 2"
  children={template2}
  args={{ text: 'This story uses second template' }}
/>

<Story name="Static template">
  <h2 style="color: aqua">Static template</h2>
  <hr>
  <p>{"Static template."}</p>
</Story>

<Story
  name="Custom template"
  args={{ text: 'This story uses custom template passed as children' }}
>
  {#snippet children(args)}
    <h2 style="color: orange;font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>{args?.text}</p>
    <hr>
  {/snippet}
</Story>

Drawbacks

  1. At the moment I found one gotcha. The linters (TypeScript, ESLint or Biome) will point out that there's unused meta destructured from the defineMeta({ ... }) expression call. Is not really "unused", at least not in the final output after parsing the *.stories.file (via Vite's code transformation). meta is used as argument in parser - collect-stories.

    • Possible solution(s)
      1. use export const - meh
      2. I'm optimistic that I can achieve extra magic with parser (especially with zimmerframe), where it will automatically destructure meta. Right during the transformation of the *.stories.svelte file, and passing it forward for stories collecting.
xeho91 commented 1 month ago

Did you find a way to make <Story /> typesafe from just inferring from the children snippet, or have you just omitted that part here?

Eg. for:

{#snippet template(args: Args<Text>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args.text}</p>
{/snippet}

<Story
  name="Story with template"
  children={template}
  args={{ text: 'This story uses the first template' }}
/>

Will the args prop on the story be typesafe based on the Args<Text> type from the template snippet?

Last time I checked - (I need to double/triple/multiple 😆 confirm it again) - if the template had incorrect types, then TypeScript was showing me error on the children={template} prop passed to <Story />

JReinhold commented 1 month ago

Will it be a problem if this Args type helper will conflict with the type Args from @storybookjs/svelte?

Hmm I don't think so, as long as the user doesn't import both. The user could be confused about which package to import the types from, but I'm not sure there's a way around that.

defineMeta

I do think it's confusing from a user's perspective that the meta property is unused. "Why do I need to destructure it?" Exporting it is the least magical, but also more verbose than it needs to be. If we just omitted meta entirely, then the mental model would be that defineMeta is not just a type-helper, but an actual factory function that will set the meta when called. I think that's a fine mental model for the user. Then we'd have to extract the content and export it default in the parser to make SB happy.

And the types for the snippet would then be snippet(args: Args<typeof Story>)

xeho91 commented 1 month ago

Will it be a problem if this Args type helper will conflict with the type Args from @storybookjs/svelte?

Hmm I don't think so, as long as the user doesn't import both. The user could be confused about which package to import the types from, but I'm not sure there's a way around that.

defineMeta

I do think it's confusing from a user's perspective that the meta property is unused. "Why do I need to destructure it?" Exporting it is the least magical, but also more verbose than it needs to be. If we just omitted meta entirely, then the mental model would be that defineMeta is not just a type-helper, but an actual factory function that will set the meta when called. I think that's a fine mental model for the user. Then we'd have to extract the content and export it default in the parser to make SB happy.

And the types for the snippet would then be snippet(args: Args<typeof Story>)

Totally agreed on the mental model. I refactored the vite plugin, and split into two:

Following the feedback, there's an updated example:

<script context="module" lang="ts">
  import { defineMeta, type Args } from '../src/index.js';

  import Text from './Text.svelte';

  /**
   * Demonstration on how to use multiple templates in one stories file,
   * powered by Svelte's **snippets**.
   */
  const { Story } = defineMeta({
    title: 'Templates',
    component: Text,
    tags: ['autodocs'],
    args: { text: "" },
    argsTypes: {
      text: { control: "text" },
    },
  });
</script>

{#snippet template1(args: Args<typeof Story>)}
  <h2 style="color: lightgreen">Template 1</h2>
  <p>{args?.text}</p>
{/snippet}

{#snippet template2(args: Args<typeof Story>)}
  <h2 style="color: fuchsia">Template 2</h2>
  <hr>
  <p>{args?.text}</p>
{/snippet}

<Story
  name="Story with template 1"
  children={template1}
  args={{ text: 'This story uses the first template' }}
/>

<Story
  name="Story with template 2"
  children={template2}
  args={{ text: 'This story uses second template' }}
/>

<Story name="Static template">
  <h2 style="color: aqua">Static template</h2>
  <hr>
  <p>{"Static template."}</p>
</Story>

<Story
  name="Custom template"
  args={{ text: 'This story uses custom template passed as children' }}
>
  {#snippet children(args)}
    <h2 style="color: orange;font-weight: 700;">Custom template</h2>
    <hr style="border-style: dashed">
    <p>{args?.text}</p>
    <hr>
  {/snippet}
</Story>

Changes

  1. No need to destructure meta anymore, the vite plugin transform will handle the case even if it was destructured already
  2. Renamed type helpers from TArgs to Args and from TContext to StoryContext
  3. The above type helpers accepts typeof Story - Args<typeof Story> & StoryContext<typeof Story>
JReinhold commented 1 month ago

Great work! This is really starting to look promising, like something we can prerelease and gather feedback on.

What's left to do here?

xeho91 commented 1 month ago

What's left to do here?

Apart from the obvious need to update the documentation, and the tests. I'll list separate issue in each comment of what is not seen in the code changes, and the rest I'll make comments on where I need the help.

xeho91 commented 1 month ago

Issue ✅ resolved

A warning appears while running the Storybook (command pnpm storybook):

[!WARNING] The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.

Using VITE_CJS_TRACE=true to debug:

``` Trace: The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details. at warnCjsUsage (/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/node_modules/.pnpm/vite@5.2.11_@types+node@20.12.12/node_modules/vite/index.cjs:33:3) at Object. (/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/node_modules/.pnpm/vite@5.2.11_@types+node@20.12.12/node_modules/vite/index.cjs:3:1) at Module._compile (node:internal/modules/cjs/loader:1369:14) at Module._extensions..js (node:internal/modules/cjs/loader:1427:10) at Object.newLoader (/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/node_modules/.pnpm/esbuild-register@3.5.0_esbuild@0.20.2/node_modules/esbuild-register/dist/node.js:2262:9) at extensions..js (/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/node_modules/.pnpm/esbuild-register@3.5.0_esbuild@0.20.2/node_modules/esbuild-register/dist/node.js:4838:24) at Module.load (node:internal/modules/cjs/loader:1206:32) at Module._load (node:internal/modules/cjs/loader:1022:12) at Module.require (node:internal/modules/cjs/loader:1231:19) at require (node:internal/modules/helpers:179:18) ```

I can't figure out whether the issue lies on the Storybook's internal configuration of Vite or the addon.

xeho91 commented 1 month ago

Issue ✅ Resolved

Build breaks (command pnpm build), without the workaround - see the bottom of this comment.

```txt SB_CORE-SERVER_0002 (CriticalPresetLoadError): Storybook failed to load the following preset: ./.storybook/main.ts. Please check whether your setup is correct, the Storybook dependencies (and their peer dependencies) are installed correctly and there are no package version clashes. If you believe this is a bug, please open an issue on Github. SB_CORE-SERVER_0002 (CriticalPresetLoadError): Storybook failed to load the following preset: ./dist/preset.js. Please check whether your setup is correct, the Storybook dependencies (and their peer dependencies) are installed correctly and there are no package version clashes. If you believe this is a bug, please open an issue on Github. Error: No "exports" main defined in ./node_modules/zimmerframe/package.json at exportsNotFound (node:internal/modules/esm/resolve:304:10) at packageExportsResolve (node:internal/modules/esm/resolve:594:13) at resolveExports (node:internal/modules/cjs/loader:590:36) at Module._findPath (node:internal/modules/cjs/loader:667:31) at Module._resolveFilename (node:internal/modules/cjs/loader:1129:27) at Module._resolveFilename (./node_modules/.pnpm/esbuild-register@3.5.0_esbuild@0.20.2/node_modules/esbuild-register/dist/node.js:4799:36) at Module._load (node:internal/modules/cjs/loader:984:27) at Module.require (node:internal/modules/cjs/loader:1231:19) at require (node:internal/modules/helpers:179:18) at Object. (./dist/parser/walkers/fragment.js:37:26) at loadPreset (./node_modules/.pnpm/@storybook+core-common@8.1.0_prettier@3.2.5/node_modules/@storybook/core-common/dist/index.js:12:59) at loadPreset (./node_modules/.pnpm/@storybook+core-common@8.1.0_prettier@3.2.5/node_modules/@storybook/core-common/dist/index.js:12:59) at async Promise.all (index 1) at async loadPresets (./node_modules/.pnpm/@storybook+core-common@8.1.0_prettier@3.2.5/node_modules/@storybook/core-common/dist/index.js:12:483) at async getPresets (./node_modules/.pnpm/@storybook+core-common@8.1.0_prettier@3.2.5/node_modules/@storybook/core-common/dist/index.js:12:1503) at async buildDevStandalone (./node_modules/.pnpm/@storybook+core-server@8.1.0_prettier@3.2.5_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@storybook/core-server/dist/index.js:65:2024) at async withTelemetry (./node_modules/.pnpm/@storybook+core-server@8.1.0_prettier@3.2.5_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@storybook/core-server/dist/index.js:28:3599) at async dev (./node_modules/.pnpm/@storybook+cli@8.1.0_@babel+preset-env@7.24.5_@babel+core@7.24.5__react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@storybook/cli/dist/generate.js:705:563) at async Command. (./node_modules/.pnpm/@storybook+cli@8.1.0_@babel+preset-env@7.24.5_@babel+core@7.24.5__react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@storybook/cli/dist/generate.js:707:250) WARN Broken build, fix the error above. WARN You may need to refresh the browser. WARN Failed to load preset: {"type":"presets","name":"/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/dist/preset.js"} on level 1 Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in ./node_modules/zimmerframe/package.json at exportsNotFound (node:internal/modules/esm/resolve:304:10) at packageExportsResolve (node:internal/modules/esm/resolve:594:13) at resolveExports (node:internal/modules/cjs/loader:590:36) at Module._findPath (node:internal/modules/cjs/loader:667:31) at Module._resolveFilename (node:internal/modules/cjs/loader:1129:27) at Module._resolveFilename (./node_modules/.pnpm/esbuild-register@3.5.0_esbuild@0.20.2/node_modules/esbuild-register/dist/node.js:4799:36) at Module._load (node:internal/modules/cjs/loader:984:27) at Module.require (node:internal/modules/cjs/loader:1231:19) at require (node:internal/modules/helpers:179:18) at Object. (./dist/parser/walkers/fragment.js:3:22) ```

What I know:

  1. This is because zimmerframe is an ESM-only package.
  2. svelte-package is somehow(?) trying to import this package as CJS format. I can't locate the options/settings on where I can change this behaviour.

Might be(?) related to this issue: https://github.com/storybookjs/addon-svelte-csf/pull/181#issuecomment-2129023551

Workaround

This is what I do to not get blocked on further development of this addon

  1. Open the ./node_modules/zimmerframe/package.json
  2. Make changes
    "exports": {
        ".": {
            "types": "./types/index.d.ts",
-           "import": "./src/walk.js"
+           "default": "./src/walk.js"
        }
    },
JReinhold commented 1 month ago

Vite CJS warning

This is a limitation of Storybook. Currently, Storybook loads any presets (like main.ts, or addon presets) with esbuild-register in a way that compiles them to CJS, which triggers this warning. The workaround is to turn any Vite imports in presets to dynamic imports, because those will stay as import('vite') instead of require('vite'). So the imports defined in pre-transform.ts and post-transform.ts needs to be dynamic imports within the functions.

Build breaks (command pnpm build)

I think you confused yourself (and me 😅) here a bit. I tried it out, it's not the build command that's failing, it's build-storybook, which makes sense given it's a Storybook build error being thrown. This is the exact same problem as above, however this is the first time that I've experienced this "forced" CJS becoming a real problem beyond Vite's warning.

I've pushed commits that fixes both issues above.

However now building Storybook throws a new error from Rollup:

Error ``` x Build failed in 194ms => Failed to build the preview RollupError: [vite:build-import-analysis] [plugin vite:build-import-analysis] stories/Actions.stories.svelte (38:0): Failed to parse source for import analysis because the content contains invalid JS syntax. If you are using JSX, make sure to name the file with the .jsx or .tsx extension. file: ./stories/Actions.stories.svelte:38:0 36: $.append($anchor, fragment); 37: $.pop(); 38: } ^ 39: 40: $.delegate(["click"]);;Actions_stories.__docgen = {"version":3,"name":"Actions.stories.svelte","data":[],"computed":[... at getRollupError (file://./node_modules/.pnpm/rollup@4.17.2/node_modules/rollup/dist/es/shared/parseAst.js:394:41) at error (file://./node_modules/.pnpm/rollup@4.17.2/node_modules/rollup/dist/es/shared/parseAst.js:390:42) at Object.error (file://./node_modules/.pnpm/rollup@4.17.2/node_modules/rollup/dist/es/shared/node-entry.js:19593:20) at Object.error (file://./node_modules/.pnpm/rollup@4.17.2/node_modules/rollup/dist/es/shared/node-entry.js:18703:42) at Object.transform (file://./node_modules/.pnpm/vite@5.2.11_@types+node@20.12.12/node_modules/vite/dist/node/chunks/dep-cNe07EU9.js:66777:22) at process.processTicksAndRejections (node:internal/process/task_queues:95:5) ```

I haven't looked into this yet, but it seems like we or Svelte 5 is transforming the stories files to invalid syntax, possibly only when in build/rollup mode.

xeho91 commented 1 month ago

Issue - ✅ Resolved

TypeScript fails on writing stories for components with required snippet(s) e.g. `children`. > [!NOTE] > I've added an example with this case under `./stories/test/RequiredSnippet.stories.svelte`. **I need a guidance on this one.** I suspect it might be related to the [`Meta`](https://github.com/storybookjs/storybook/blob/8e3e06b34386d4c5b20621772a028bfd9961a087/code/renderers/svelte/src/public-types.ts#L21-L28) & [`StoryObj`](https://github.com/storybookjs/storybook/blob/8e3e06b34386d4c5b20621772a028bfd9961a087/code/renderers/svelte/src/public-types.ts#L38-L59) type helpers inside `@storybook/svelte` package. Following the Svelte `v5` snippet feature: https://svelte-5-preview.vercel.app/docs/snippets There are two ways how we can provide any required snippet, especially `children`. 1. As identifier to a snippet which exists at the root of fragment, like this: ```svelte {#snippet children()} {/snippet} ``` ✅ **This case is covered already**. We don't have to do anything with this one, because, passing it to `args` like in example below works: ```svelte {#snippet children()} {/snippet} ``` 2. Defining `children` as inner nodes, like this: ```svelte {#snippet children()} {/snippet} ``` which is also equivalent to: ```svelte ``` ❌ **This case is not covered.** I don't know how to. How do we make `StoryObj["args"]["children"]` understand the following scenario, where `` has required `children` provided as inner content? Like in the example below: ```svelte ``` ## Workaround to shut up TypeScript > [!CAUTION] > **This is invalid** _(throws error at runtime)_. ```diff const { Story } = defineMeta({ + args: { + children: "", +. }, }); ``` > [!CAUTION] > **REMEMBER!** You still have to actually provide a `children` from outer snippet at the root of fragment to the `Story` args. Or define it inner content _(children)_. **Things to remember**: 1. This below doesn't work: ```svelte {#snippet children()} {/snippet} ``` Why? Because you can't forward the snippet to module tag `context="module"`. Only to the normal instance tag.
xeho91 commented 1 month ago

EDIT Resolved! ✅

I am to blame 😆 .it was caused by this addon removing the whole line of `export default ...`. Now it removes just the `export default`. @JReinhold I've inspected the `build-storybook` issue. This is what I found, I picked one file to see what it's being transformed into - in this case - `./stories/templates.stories.svelte`. ---
Output for build-storybook ```js import "svelte/internal/disclose-version"; import * as $ from "svelte/internal/client"; var root_1 = $.template(`

Template 1

`, 1); var root_2 = $.template(`

Template 2

`, 1); var Story_default = $.template(`

Static template

`, 1); var root_3 = $.template(`

Custom template

`, 1); var root = $.template(` `, 1); import { defineMeta } from '@storybook/addon-svelte-csf'; import Text from './Text.svelte'; /** * Demonstration on how to use multiple templates in one stories file, * powered by Svelte's **snippets**. */ const { Story, meta } = defineMeta({ title: 'Templates', component: Text, tags: ['autodocs'], args: { text: "" }, argsTypes: { text: { control: "text" } } }); $.push($$props, false); var template1 = ($$anchor, args = $.noop) => { var fragment = root_1(); var h2 = $.first_child(fragment); var p = $.sibling($.sibling(h2, true)); var text = $.child(p); $.template_effect(() => $.set_text(text, args()?.text)); $.append($$anchor, fragment); }; var template2 = ($$anchor, args = $.noop) => { var fragment_1 = root_2(); var h2_1 = $.first_child(fragment_1); var hr = $.sibling($.sibling(h2_1, true)); var p_1 = $.sibling($.sibling(hr, true)); var text_1 = $.child(p_1); $.template_effect(() => $.set_text(text_1, args()?.text)); $.append($$anchor, fragment_1); }; $.init(); var fragment_2 = root(); var node = $.first_child(fragment_2); Story(node, { name: "Story with template 1", children: template1, args: { text: 'This story uses the first template' } }); var node_1 = $.sibling($.sibling(node, true)); Story(node_1, { name: "Story with template 2", children: template2, args: { text: 'This story uses second template' } }); var node_2 = $.sibling($.sibling(node_1, true)); Story(node_2, { name: "Static template", children: ($$anchor, $$slotProps) => { var fragment_3 = Story_default(); var h2_2 = $.first_child(fragment_3); var hr_1 = $.sibling($.sibling(h2_2, true)); var p_2 = $.sibling($.sibling(hr_1, true)); var text_2 = $.child(p_2); text_2.nodeValue = $.stringify("Static template."); $.append($$anchor, fragment_3); }, $$slots: { default: true } }); var node_3 = $.sibling($.sibling(node_2, true)); { var children = ($$anchor, args = $.noop) => { var fragment_4 = root_3(); var h2_3 = $.first_child(fragment_4); var hr_2 = $.sibling($.sibling(h2_3, true)); var p_3 = $.sibling($.sibling(hr_2, true)); var text_3 = $.child(p_3); var hr_3 = $.sibling($.sibling(p_3, true)); $.template_effect(() => $.set_text(text_3, args()?.text)); $.append($$anchor, fragment_4); }; Story(node_3, { name: "Custom template", args: { text: 'This story uses custom template passed as children' }, children }); } $.append($$anchor, fragment_2); $.pop(); };Templates_stories.__docgen = {"keywords":[],"data":[],"name":"templates.stories.svelte"} import parser from '/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/dist/parser/collect-stories.js'; const __parsed = parser(Templates_stories, {"defineMeta":{"description":"Demonstration on how to use multiple templates in one stories file,\npowered by Svelte's **snippets**.","title":"Templates","tags":["autodocs"]},"stories":{"Story with template 1":{"id":"StoryWithTemplate1","name":"Story with template 1"},"Story with template 2":{"id":"StoryWithTemplate2","name":"Story with template 2"},"Static template":{"id":"StaticTemplate","name":"Static template","rawSource":"

Static template

\n
\n

{\"Static template.\"}

"},"Custom template":{"id":"CustomTemplate","name":"Custom template","rawSource":"

Custom template

\n
\n

{args?.text}

\n
"}}}, meta); export default __parsed.meta; export const __exports = ["Story with template 1","Story with template 2","Static template","Custom template"]; export const Story_with_template_1 = __parsed.stories["Story with template 1"]; export const Story_with_template_2 = __parsed.stories["Story with template 2"]; export const Static_template = __parsed.stories["Static template"]; export const Custom_template = __parsed.stories["Custom template"]; ```
The issue is seen here: ```js };Templates_stories.__docgen = {"keywords":[],"data":[],"name":"templates.stories.svelte"} ``` Something internally adds the invalid `}` at the start. I suspect the issue might be on the Storybook side, not sure which one package yet. AFAIK, this piece of code is not added/transformed by this addon. ---
Output for dev - storybook ```js import "svelte/internal/disclose-version"; $.mark_module_start(); Templates_stories.filename = "stories/templates.stories.svelte"; import * as $ from "svelte/internal/client"; var root_1 = $.add_locations($.template(`

Template 1

`, 1), Templates_stories.filename, [[26, 2], [27, 2]]); var root_2 = $.add_locations($.template(`

Template 2

`, 1), Templates_stories.filename, [[31, 2], [32, 2], [33, 2]]); var Story_default = $.add_locations($.template(`

Static template

`, 1), Templates_stories.filename, [[49, 2], [50, 2], [51, 2]]); var root_3 = $.add_locations($.template(`

Custom template

`, 1), Templates_stories.filename, [ [59, 4], [60, 4], [61, 4], [62, 4] ]); var root = $.add_locations($.template(` `, 1), Templates_stories.filename, []); import { defineMeta } from '@storybook/addon-svelte-csf'; import Text from './Text.svelte'; /** * Demonstration on how to use multiple templates in one stories file, * powered by Svelte's **snippets**. */ const { Story, meta } = defineMeta({ title: 'Templates', component: Text, tags: ['autodocs'], args: { text: "" }, argsTypes: { text: { control: "text" } } }); function Templates_stories($$anchor, $$props) { if (new.target === Templates_stories) throw new Error("Instantiating a component with `new` is no longer valid in Svelte 5. See https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes for more information"); $.push($$props, false, Templates_stories); var template1 = $.wrap_snippet(($$anchor, args = $.noop) => { var fragment = root_1(); var h2 = $.first_child(fragment); var p = $.sibling($.sibling(h2, true)); var text = $.child(p); $.template_effect(() => $.set_text(text, args()?.text)); $.append($$anchor, fragment); }); var template2 = $.wrap_snippet(($$anchor, args = $.noop) => { var fragment_1 = root_2(); var h2_1 = $.first_child(fragment_1); var hr = $.sibling($.sibling(h2_1, true)); var p_1 = $.sibling($.sibling(hr, true)); var text_1 = $.child(p_1); $.template_effect(() => $.set_text(text_1, args()?.text)); $.append($$anchor, fragment_1); }); $.init(); var fragment_2 = root(); var node = $.first_child(fragment_2); $.validate_component(Story)(node, { name: "Story with template 1", children: template1, args: { text: 'This story uses the first template' } }); var node_1 = $.sibling($.sibling(node, true)); $.validate_component(Story)(node_1, { name: "Story with template 2", children: template2, args: { text: 'This story uses second template' } }); var node_2 = $.sibling($.sibling(node_1, true)); $.validate_component(Story)(node_2, { name: "Static template", children: $.wrap_snippet(($$anchor, $$slotProps) => { var fragment_3 = Story_default(); var h2_2 = $.first_child(fragment_3); var hr_1 = $.sibling($.sibling(h2_2, true)); var p_2 = $.sibling($.sibling(hr_1, true)); var text_2 = $.child(p_2); text_2.nodeValue = $.stringify("Static template."); $.append($$anchor, fragment_3); }), $$slots: { default: true } }); var node_3 = $.sibling($.sibling(node_2, true)); { var children = $.wrap_snippet(($$anchor, args = $.noop) => { var fragment_4 = root_3(); var h2_3 = $.first_child(fragment_4); var hr_2 = $.sibling($.sibling(h2_3, true)); var p_3 = $.sibling($.sibling(hr_2, true)); var text_3 = $.child(p_3); var hr_3 = $.sibling($.sibling(p_3, true)); $.template_effect(() => $.set_text(text_3, args()?.text)); $.append($$anchor, fragment_4); }); $.validate_component(Story)(node_3, { name: "Custom template", args: { text: 'This story uses custom template passed as children' }, children }); } $.append($$anchor, fragment_2); return $.pop({ ...$.legacy_api() }); } if (import.meta.hot) { const s = $.source(Templates_stories); const filename = Templates_stories.filename; Templates_stories = $.hmr(s); Templates_stories.filename = filename; if (import.meta.hot.acceptExports) { import.meta.hot.acceptExports(["default"], (module) => { $.set(s, module.default); }); } else { import.meta.hot.acceptExports(["default"],(module) => { $.set(s, module.default); }); } } $.mark_module_end(Templates_stories);;Templates_stories.__docgen = {"keywords":[],"data":[],"name":"templates.stories.svelte"} import parser from '/Users/xeho91/Nextcloud/Projects/oss/addon-svelte-csf/dist/parser/collect-stories.js'; const __parsed = parser(Templates_stories, {"defineMeta":{"description":"Demonstration on how to use multiple templates in one stories file,\npowered by Svelte's **snippets**.","title":"Templates","tags":["autodocs"]},"stories":{"Story with template 1":{"id":"StoryWithTemplate1","name":"Story with template 1"},"Story with template 2":{"id":"StoryWithTemplate2","name":"Story with template 2"},"Static template":{"id":"StaticTemplate","name":"Static template","rawSource":"

Static template

\n
\n

{\"Static template.\"}

"},"Custom template":{"id":"CustomTemplate","name":"Custom template","rawSource":"

Custom template

\n
\n

{args?.text}

\n
"}}}, meta); export default __parsed.meta; export const __exports = ["Story with template 1","Story with template 2","Static template","Custom template"]; export const Story_with_template_1 = __parsed.stories["Story with template 1"]; export const Story_with_template_2 = __parsed.stories["Story with template 2"]; export const Static_template = __parsed.stories["Static template"]; export const Custom_template = __parsed.stories["Custom template"]; ```
This piece of transformed code looks weird, although is not invalid, and not the case: ```js $.mark_module_end(Templates_stories);;Templates_stories.__docgen = {"keywords":[],"data":[],"name":"templates.stories.svelte"} ``` By weird part I meant the double semicolons - `;;` I know the related Storybook addons appends the `;` at the start of every code where they do the transformation, not the reason of why it fails. --- **EDIT** I've spotted the difference that during `build-storybook` something eats missing code in comparison from `storybook` _(dev)_, which is the cause of the extra `}`. I've scanned through the code of the following plugins, to see if any of them could be the suspect for the invalid transformation.
Used plugins ```js Plugins { plugins: [ Promise { }, [ [Object], [Object], [Object] ], { name: 'storybook:code-generator-plugin', enforce: 'pre', configureServer: [Function: configureServer], config: [Function: config], configResolved: [Function: configResolved], resolveId: [Function: resolveId], load: [AsyncFunction: load], transformIndexHtml: [AsyncFunction: transformIndexHtml] }, { name: 'unplugin-csf', transformInclude: [Function: transformInclude], transform: [Function (anonymous)] }, { name: 'storybook:inject-export-order-plugin', enforce: 'post', transform: [AsyncFunction: transform] }, { name: 'storybook:strip-hmr-boundary-plugin', enforce: 'post', transform: [AsyncFunction: transform] }, { name: 'storybook:allow-storybook-dir', enforce: 'post', config: [Function: config] }, { name: 'storybook:external-globals-plugin', enforce: 'post', config: [AsyncFunction: config], transform: [AsyncFunction: transform] }, { name: 'storybook:rollup-plugin-webpack-stats', enforce: 'post', moduleParsed: [Function: moduleParsed], storybookGetStats: [Function: storybookGetStats] }, { name: 'storybook:svelte-docgen-plugin', transform: [AsyncFunction: transform] }, Promise { } ] } ```
Now I suspect the issue is on the `@sveltejs/vite-plugin-svelte`, or even `svelte` - because as far I could invegistate, they could be responsible for generating the code of where is the below issue I found: ```diff - function Templates_stories($$anchor, $$props) { - if (new.target === Templates_stories) throw new Error("Instantiating a component with - `new` is no longer valid in Svelte 5. See https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes for more information"); - $.push($$props, false, Templates_stories); + $.push($$props, true); ``` The possible location of what causes this issue: https://github.com/sveltejs/svelte/blob/d1f5d5d33d228106236089d062d27c7d4696c930/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js#L379
xeho91 commented 1 month ago

Issue - Resolved ✅

Broken type helpers from this addon: - `Args` - `StoryContext` Not sure since when, I found out now that **right now TypeScript is not happy with the type parameters provided - `typeof Story`**. You can see the error in: `./stories/Example.stories.svelte`. Even though, the extracted types for further usage seems to be correct _(e.g. it infers the component props in `args`, or any field that should be in `StoryContext`)_ It could be related to `@storybook/svelte` - I saw that it still uses deprecated type, for example: - `ComponentConstructorOptions` - source: https://github.com/storybookjs/storybook/blob/b94ad4473564d8ca16f252f3b038e6aedf2df08e/code/renderers/svelte/src/types.ts#L28 and the deprecation source: https://github.com/sveltejs/svelte/blob/d1f5d5d33d228106236089d062d27c7d4696c930/packages/svelte/src/index.d.ts#L11 I'm out of clues on how I made it work before _(unless my LSP was broken then...?)_ 🤔
xeho91 commented 1 month ago

I'm close, damn it! 😅

I've created a test snippet: https://github.com/storybookjs/addon-svelte-csf/blob/experimental-support-svelte-v5/test/compile.ts

It almost gives me what I need.

Currently the output from the initial test file: https://github.com/storybookjs/addon-svelte-csf/blob/experimental-support-svelte-v5/src/parser/extract/compiled/nodes.test.ts

... is:

mark_module_start();
Virtual_stories.filename = "/@virtual:vite-plugin-virtual/virtual:stories.svelte";

function Virtual_stories($$anchor, $$props) {
    if (new.target === Virtual_stories) throw new Error("Instantiating a component with `new` is no longer valid in Svelte 5. See https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes for more information");
    push($$props, false, Virtual_stories);

    const { Story } = defineMeta();

    init();
    bind_prop($$props, "Story", Story);

    return pop({
        get Story() {
            return Story;
        },
        ...legacy_api()
    });
}

mark_module_end(Virtual_stories);

Which is close to what I need in order to produce tests for the functions related to compiled AST & transformation.

I've encountered one blocker for further parsing/analysing the AST. The output didn't include the imports at all.

I'm talking about:

import { defineMeta } from "@storybook/addon-svelte-csf";
// ... rest of the compiled code

And I ran out of ideas how to get it included in the compiled output. Perhaps some of vite's plugin is missing(?), or we need to include vite plugins from Storybook?

JReinhold commented 1 month ago

Perhaps the imports are treeshaken out? Have you tried with a more real-life example, where you use context="module" and actually define a <Story/>?

xeho91 commented 1 month ago

Perhaps the imports are treeshaken out? Have you tried with a more real-life example, where you use context="module" and actually define a <Story/>?

Good thought, totally forgot about context="module"! Slight progress. Changing entry to:

<script context="module">
  import { defineMeta } from "@storybook/addon-svelte-csf";
  export const { Story } = defineMeta();
</script>
<Story name="Default" />

and adding treeshake: false to rollupOptions:

... outputs:

mark_module_start();
Virtual_stories.filename = "/@virtual:vite-plugin-virtual/virtual:stories.svelte";

const { Story } = defineMeta();

function Virtual_stories($$anchor, $$props) {
    if (new.target === Virtual_stories) throw new Error("Instantiating a component with `new` is no longer valid in Svelte 5. See https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes for more information");
    push($$props, false, Virtual_stories);
    init();

    var fragment = comment();
    var node = first_child(fragment);

    validate_component(Story)(node, { name: "Default" });
    append($$anchor, fragment);
    return pop({ ...legacy_api() });
}

mark_module_end(Virtual_stories);

What changed as expected:

const { Story } = defineMeta();

... got moved outside the function.

But still, import { defineMeta } from "@storybookjs/addon-svelte-csf" not included.

xeho91 commented 1 month ago

Issue

Currently we're using svelte-package to build this addon. I've explored this package source: https://github.com/sveltejs/kit/tree/main/packages/kit

I don't see a way how we can use glob pattern or any similar option to omit tests files from being included in the build output.

Possible solutions

  1. [difficulty - easy peasy 🔰] Separate all of the tests files *.test.ts from ./src/ to ./test/ directory
  2. [difficulty - hard 🏋️] Explore if we can use vite for building - using the lib option, and include *.svelte files without being compiled - as in produce the same results as svelte-package do right now
xeho91 commented 1 month ago

Issue - ✅ Resolved

In the **development mode** _(when running with `pnpm storybook`)_: 1. HTML comments above `` component, which are supposed to be automatically inserted into `parameters.docs.description.story` doesn't work 2. Property `source` - e.g.`` doesn't work either **Reason**: The compiled output differs between production and development mode. Hence current [function for extracting the compiled `Stories`](https://github.com/storybookjs/addon-svelte-csf/blob/experimental-support-svelte-v5/src/parser/extract/compiled/stories.ts) needs to do some extra advanced work for **development** mode. 😞 I will take care of it after weekend.
JReinhold commented 1 month ago

Currently we're using svelte-package to build this addon.

I came to the same conclusion. Let's just separate tests into their own files like it was 2022 😅 Not a big deal, not worth the effort trying to change our bundling system.

  1. HTML comments above <Story> component, which are supposed to be automatically inserted into parameters.docs.description.story doesn't work

Good catch!

  1. Property source - e.g.<Story source="code..." /> doesn't work either

Don't worry about this for now, I have an alternative solution to the whole Source Code situation. 👇

JReinhold commented 1 month ago

Issue

Source snippet generation is pretty broken, always producing <StoryRenderer allTheArgs... />.

This is because we're relying on SB core's built in source generator, which isn't clever enough to understand this syntax. it basically does some string concatenation like <COMPONENT_NAME ARGS_AS_ATTRIBUTES... />.

See https://github.com/storybookjs/storybook/blob/4bd5ef3549a514a60ad0683df5fb85896895a92c/code/renderers/svelte/src/docs/sourceDecorator.ts#L101-L136

The stable version of this addon is a bit more clever at extracting the source snippets, but we've disabled/removed that in this branch, and it wouldn't work with the new template/snippet syntax anyway, but we can draw inspiration from it:

https://github.com/storybookjs/addon-svelte-csf/blob/0c31f56cbe0595b6c2a07b5f1188d6493f504aff/src/parser/extract-stories.ts#L204-L214

https://github.com/storybookjs/addon-svelte-csf/blob/0c31f56cbe0595b6c2a07b5f1188d6493f504aff/src/parser/collect-stories.ts#L149-L171

Possible solution

For starters, I think we can remove the source prop on Story.

And then I want to explore this flow:

1. Get the original source of the X.stories.svelte file.

Our Vite plugin currently transforms after Svelte has compiled the files. There might be a built-in way in the Vite API to get the original source in our transform-hook, but if not, we could either:

A. Read the source from the file system directly. This probably wouldn't work with virtual files (is that even a use case?), and it could be a bit slow. This is what this addon currently does at https://github.com/storybookjs/addon-svelte-csf/blob/0c31f56cbe0595b6c2a07b5f1188d6493f504aff/src/plugins/vite-svelte-csf.ts#L25-L28 B. Add a pre plugin, where all it does is store the source to a resolve id -> source map, that our post plugin can then read from and do whatever it want with.

2. For each story, get the source snippet string from the original source.

This is the tricky part. I'm not sure if we should do this with the AST and string positions, or just with raw string parsing. I see multiple cases we need to support, ranked here from simplest to most complex:

1. No template, no children, just Story This is the simplest, it should just be about getting the component name and the args. **Input**: ```svelte ``` **Expected source snippet**: ```svelte ```
2. No template, just Story with static content Here we need to get the whole child content from the ``, but we don't have to worry about `args` at all. **Input**: ```svelte ``` **Expected source snippet**: ```svelte ```
3. No template, Story with snippet content Very similar to 2. above, except we need to get the content from _within_ the snippet. **Input**: ```svelte {#snippet children(args)}

A header

 {/snippet} ``` **Expected source snippet**: ```svelte

A header

 ```
4. Explicit template as children Similar to 3. above, except we need to find the template that the `children` prop is referencing, and get the content from there. **Input**: ```svelte {#snippet myTempalte(args)}

A header

 {/snippet} ``` **Expected source snippet**: ```svelte

A header

 ```
5. Implicit template via setTemplate Similar to 4. above, except we need to find the template that has been set via `setTemplate(myTemplate)` in the instance script. **Input**: ```svelte {#snippet myTempalte(args)}

A header

 {/snippet} ``` **Expected source snippet**: ```svelte

A header

 ```

3. Add the source snippets to the compiled output

Similar to what we do in the "create appendix" logic now, I think we can add these extracted source snippets as variables to the compiled output, so they can be read at runtime. I don't yet know what the exact structure should be for this, but I don't think that's too crucial.

These strings then need to be passed down to the StoryRenderer as props here https://github.com/storybookjs/addon-svelte-csf/blob/0b0e5c41da873d72d9d04a2d94b0ce9d21e84dd7/src/runtime/create-runtime-stories.ts#L51-L59

4. At runtime, replace args in strings and emit to the channel

Eg. given the string 

<MyComponent {...args} somethingElse={args.someArg}>
  <h2>A header</h2>
</MyComponent>

It should expand current args to

<MyComponent myProp="A" other={false} somethingElse={42}>
  <h2>A header</h2>
</MyComponent>

There are also some edge cases, eg. if a snippet references a variable from somewhere else like the instance script, we can't expand that to be what it actually is. I don't think we should aim to support that right out the gate, There is always the transform parameter that users can use to fix this in their use case.

And then we emit the final string to the SB channel.

We can draw inspiration from https://github.com/storybookjs/storybook/blob/4bd5ef3549a514a60ad0683df5fb85896895a92c/code/renderers/svelte/src/docs/sourceDecorator.ts#L101-L136

Step 2 is definitely the hardest part and I think I could use your help there @xeho91 with your AST expertise. The rest of the steps looks pretty straightforward to me.

benoitf commented 1 week ago

hello, can we test the module published on npmjs like 5.0.0--canary.181.04f02dc.0

it seems to fail early on my end. What should be the other dependencies of storybook ?

JReinhold commented 1 week ago

@benoitf thanks for trying it out! We're hoping it's ready for broader feedback soon.

Which error are you getting? It should work if you follow the version requirements listed here: https://github.com/storybookjs/addon-svelte-csf/blob/experimental-support-svelte-v5/README.md#latest

benoitf commented 1 week ago

@JReinhold here is the error I got (it's about loading src files while there is only a dist folder in the published npmjs module)

$ storybook build
@storybook/cli v8.1.11

info => Cleaning outputDir: storybook-static
info => Loading presets
SB_CORE-SERVER_0002 (CriticalPresetLoadError): Storybook failed to load the following preset: ./.storybook/main.ts.

Please check whether your setup is correct, the Storybook dependencies (and their peer dependencies) are installed correctly and there are no package version clashes.

If you believe this is a bug, please open an issue on Github.

SB_CORE-SERVER_0002 (CriticalPresetLoadError): Storybook failed to load the following preset: node_modules/@storybook/addon-svelte-csf/dist/preset.js.

Please check whether your setup is correct, the Storybook dependencies (and their peer dependencies) are installed correctly and there are no package version clashes.

If you believe this is a bug, please open an issue on Github.

Error: Cannot find module 'node_modules/@storybook/addon-svelte-csf/src/compiler/transform/shared/description.ts'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1181:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1169:15)
    at Module._resolveFilename (node:internal/modules/cjs/loader:1106:16)
    at Module._resolveFilename (node_modules/esbuild-register/dist/node.js:4799:36)
    at Module._load (node:internal/modules/cjs/loader:985:27)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (node_modules/@storybook/addon-svelte-csf/dist/compiler/transform/story/insert-description.js:37:26)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at extensions..js (node_modules/esbuild-register/dist/node.js:4845:15)
    at loadPreset (node_modules/@storybook/core-common/dist/index.js:12:59)
    at loadPreset (node_modules/@storybook/core-common/dist/index.js:12:59)
    at async Promise.all (index 2)
    at async loadPresets (node_modules/@storybook/core-common/dist/index.js:12:483)
    at async getPresets (node_modules/@storybook/core-common/dist/index.js:12:1503)
    at async buildStaticStandalone (node_modules/@storybook/core-server/dist/index.js:36:3268)
    at async withTelemetry (node_modules/@storybook/core-server/dist/index.js:40:3599)
    at async build (node_modules/@storybook/cli/dist/generate.js:705:1409)
    at async Command.<anonymous> (node_modules/@storybook/cli/dist/generate.js:708:150)
WARN   Failed to load preset: {"type":"presets","name":"node_modules/@storybook/addon-svelte-csf/dist/preset.js"} on level 1
Error: Cannot find module 'node_modules/@storybook/addon-svelte-csf/src/compiler/transform/shared/description.ts'
    at createEsmNotFoundErr (node:internal/modules/cjs/loader:1181:15)
    at finalizeEsmResolution (node:internal/modules/cjs/loader:1169:15)
    at Module._resolveFilename (node:internal/modules/cjs/loader:1106:16)
    at Module._resolveFilename (node_modules/esbuild-register/dist/node.js:4799:36)
    at Module._load (node:internal/modules/cjs/loader:985:27)
    at Module.require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at Object.<anonymous> (node_modules/@storybook/addon-svelte-csf/dist/compiler/transform/story/insert-description.js:3:246)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at extensions..js (node_modules/esbuild-register/dist/node.js:4845:15)
error Command failed with exit code 1.

it comes from https://github.com/storybookjs/addon-svelte-csf/blob/04f02dc3dc1ec0e656586be3f233cfd72e23d65a/package.json#L17-L19

benoitf commented 1 week ago

replacing it by "#*": "./dist/*.js" I made progress

JReinhold commented 6 days ago

Thanks for the fix @benoitf ❤️. I added a "source" import condition to be used internally, because the dist map would cause the "Go To Definition" functionality to go to .d.ts files instead of the source files which were quite annoying. Can you check that it still works on your end?

benoitf commented 6 days ago

@JReinhold I confirm your fix is better and it's still working on my end 🎉

benoitf commented 1 day ago

Hi, FYI with more recent versions of svelte5, (updating from 5.0.0-next.164 to 5.0.0-next.174) I'm now receiving the error 

node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24
CompileError: `p` has already been declared

$ storybook build
@storybook/cli v8.1.11

info => Cleaning outputDir: storybook-static
info => Loading presets
info => Building manager..
info => Manager built (182 ms)
info => Building preview..
WARN No story files found for the specified pattern: src/stories/**/*.mdx
vite v5.3.3 building for production...

./sb-common-assets/fonts.css doesn't exist at build time, it will remain unchanged to be resolved at runtime
transforming...
The TypeScript option verbatimModuleSyntax is now required when using Svelte files with lang="ts". Please add it to your tsconfig.json.
x Build failed in 1.16s
✓ 101 modules transformed.
=> Failed to build the preview
[vite-plugin-svelte] [plugin vite-plugin-svelte] ../node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte (12:24): /home/runner/work/podman-desktop/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24 `p` has already been declared
file: /home/runner/work/podman-desktop/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24
CompileError: `p` has already been declared
    at e (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/errors.js:61:8)
    at Module.declaration_duplicate (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/errors.js:130:2)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:98:6)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:88:24)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:88:24)
    at VariableDeclaration (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:490:34)
    at visit (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:121:13)
    at next (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:63:23)
    at create_block_scope (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:[28](https://github.com/containers/podman-desktop/actions/runs/9806646693/job/27078814364?pr=7959#step:7:29)0:3)
    at visit (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:121:13)

attention => Storybook now collects completely anonymous telemetry regarding usage.
This information is used to shape Storybook's roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://storybook.js.org/telemetry
xeho91 commented 1 day ago

Hi, FYI with more recent versions of svelte5, (updating from 5.0.0-next.164 to 5.0.0-next.174) I'm now receiving the error 

node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24
CompileError: `p` has already been declared

$ storybook build
@storybook/cli v8.1.11

info => Cleaning outputDir: storybook-static
info => Loading presets
info => Building manager..
info => Manager built (182 ms)
info => Building preview..
WARN No story files found for the specified pattern: src/stories/**/*.mdx
vite v5.3.3 building for production...

./sb-common-assets/fonts.css doesn't exist at build time, it will remain unchanged to be resolved at runtime
transforming...
The TypeScript option verbatimModuleSyntax is now required when using Svelte files with lang="ts". Please add it to your tsconfig.json.
x Build failed in 1.16s
✓ 101 modules transformed.
=> Failed to build the preview
[vite-plugin-svelte] [plugin vite-plugin-svelte] ../node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte (12:24): /home/runner/work/podman-desktop/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24 `p` has already been declared
file: /home/runner/work/podman-desktop/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:12:24
CompileError: `p` has already been declared
    at e (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/errors.js:61:8)
    at Module.declaration_duplicate (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/errors.js:130:2)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:98:6)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:88:24)
    at Scope.declare (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:88:24)
    at VariableDeclaration (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:490:34)
    at visit (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:121:13)
    at next (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:63:23)
    at create_block_scope (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/svelte/src/compiler/phases/scope.js:[28](https://github.com/containers/podman-desktop/actions/runs/9806646693/job/27078814364?pr=7959#step:7:29)0:3)
    at visit (file:///home/runner/work/podman-desktop/podman-desktop/node_modules/zimmerframe/src/walk.js:121:13)

attention => Storybook now collects completely anonymous telemetry regarding usage.
This information is used to shape Storybook's roadmap and prioritize features.
You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
https://storybook.js.org/telemetry

From the error trace, it doesn't look like there's an issue on our side. I can be wrong. Does it fails as well, when you downgrade version down to 172? There's also latest one 174.

For now, I can't upgrade version to the latest one, because there were some breaking changes to the types of svelte/compiler (since version 165). I've reached out to maintainers on Discord to find a solution.

Let me know if your issue still persist in other versions than 173 🙏

benoitf commented 1 day ago

with 172 it was the same error (see the log trace) https://github.com/containers/podman-desktop/actions/runs/9777005225/job/26990739363

it's a FYI if you already saw this.

xeho91 commented 1 day ago

5.0.0--canary.181.17d1807.0

Ok, so I've tried to reproduce the error in two places:

In both, I've cleaned cache, and I'm not getting this error 🤔 I'll dive in into your repository to see if I can reproduce locally.

xeho91 commented 1 day ago

Alright, I was able to reproduce in your repository on my machine.

I've managed to narrow it down by modifying Story.svelte inside node_modules/. It breaks when destructuring ...restProps: https://github.com/storybookjs/addon-svelte-csf/blob/17d18078bf20ae36e526ddf73a19e4fc4ef58aca/src/runtime/Story.svelte#L70

However, this case is way too complicated to introduce to Svelte maintainers. For now I can't figure out a simple case to reproduce this issue.

Using Svelte 5 REPL didn't help. I suspect it be problem with @sveltejs/vite-plugin-svelte optimization part. Because its the one who gives the following error which I was able to reproduce. And using dev mode, there's a little more info:

The TypeScript option verbatimModuleSyntax is now required when using Svelte files with lang="ts". Please add it to your tsconfig.json.
✘ [ERROR] /Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:6:24 `p` has already been declared [plugin vite-plugin-svelte:optimize-svelte]

    ../node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:6:0:
      6 │
        ╵ ^

  The plugin "vite-plugin-svelte:optimize-svelte" was triggered by this import

    ../node_modules/@storybook/addon-svelte-csf/dist/index.js:2:27:
      2 │ import StoryComponent from './runtime/Story.svelte';
        ╵                            ~~~~~~~~~~~~~~~~~~~~~~~~

/Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/@storybook/cli/bin/index.js:23
  throw error;
  ^

Error: Build failed with 1 error:
../node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:6:24: ERROR: [plugin: vite-plugin-svelte:optimize-svelte] /Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:6:24 `p` has already been declared
    at failureErrorWithLog (/Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/vite/node_modules/esbuild/lib/main.js:1472:15)
    at /Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/vite/node_modules/esbuild/lib/main.js:945:25
    at /Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/vite/node_modules/esbuild/lib/main.js:1353:9
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  errors: [Getter/Setter],
  warnings: [Getter/Setter]
}

Node.js v20.12.2
 ELIFECYCLE  Command failed with exit code 7.

I'll highlight the most important part, so I can investigate it further later on.

[!CAUTION] ERROR: [plugin: vite-plugin-svelte:optimize-svelte] /Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/@storybook/addon-svelte-csf/dist/runtime/Story.svelte:6:24 p has already been declared at failureErrorWithLog (/Users/xeho91/Nextcloud/Projects/oss/podman-desktop/node_modules/vite/node_modules/esbuild/lib/main.js:1472:15)

benoitf commented 1 hour ago

Thanks @xeho91 for the investigation 🙏