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.

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

[Bug]: Svelte 5 docgen crashes with MyComponent is not defined if a variable has the same name as the filename #30212

Open
JReinhold opened this issue Jan 7, 2025 · 1 comment

Comments

@JReinhold
Copy link
Contributor

JReinhold commented Jan 7, 2025

Describe the bug

In Svelte 5+Vite (or SvelteKit), if you have a variable anywhere inside your Svelte component that is called the same as the filename, our internal Svelte docgen Vite plugin will crash with MyComponent is not defined.

Given the following Svelte component called Greeting.svelte:

<script>
	let Greeting = 'world';
</script>

Hello {Greeting}

It will crash with said error.

The reason for this, is that the above is compiled to something like this by Svelte:

... imports
 
export default function Greeting_1($$anchor) {
	let Greeting = 'world';

    ...
}

The default function has a _1 appended to the name, to not cause a collision with the internal variable named Greeting.

See https://svelte.dev/playground/23d895f41dc9451997d8ee7ead4e7be9?version=5.16.6

However our Svelte docgen logic adds a __docgen property to the default export, using different naming heuristics from Svelte 4, doing:

... imports
 
export default function Greeting_1($$anchor) {
	let Greeting = 'world';

    ...
}

Greeting.__docgen = ...
// 👆 whoops, wrong variable name, should have been Greeting_1

We fixed a similar issue in @storybook/addon-svelte-csf v5 by instead of trying to replicate the variable naming logic, we used AST parsing to get the name of the default export and use that as the reference. We could potentially do the same here. We could also try to update the naming logic to mimic Svelte 5 behavior too, but AFAIK that is easier said than done, because that logic is now spread out through multiple flows in Svelte internally.

Additional context

This is the naming logic that only works with Svelte 4: https://github.com/storybookjs/storybook/blob/next/code/frameworks/svelte-vite/src/plugins/svelte-docgen.ts/#L41-L71

This is where we add the __docgen property: https://github.com/storybookjs/storybook/blob/next/code/frameworks/svelte-vite/src/plugins/svelte-docgen.ts/#L228-L230

@JReinhold
Copy link
Contributor Author

I realize this is almost a duplicate of #29636 . The same underlying problem, we can kill 2 birds with one stone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant