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]: Error when using +page.svelte files as component in stories #29636

Open
JReinhold opened this issue Nov 17, 2024 · 0 comments
Open

[Bug]: Error when using +page.svelte files as component in stories #29636

JReinhold opened this issue Nov 17, 2024 · 0 comments

Comments

@JReinhold
Copy link
Contributor

JReinhold commented Nov 17, 2024

Describe the bug

When using Svelte 5 and SvelteKit, there's an error thrown for any story that uses a +page.svelte route as the meta component:

ReferenceError: Page is not defined
    at http://localhost:6007/src/routes/+page.svelte:33:27

This happens because the Svelte Docgen plugin calculates the component name wrong, based on Svelte 4's component name algorithm, which has changed slightly in Svelte 5, especially for filenames with special characters like +.

If we add the Vite Inspect plugin to the reproduction, we'll see that the .__docgen property is added to the wrong function name:

Image

The current (broken) algorithm for calculating the component name is here:

function getNameFromFilename(filename: string) {
if (!filename) {
return null;
}
const parts = filename.split(/[/\\]/).map(encodeURI);
if (parts.length > 1) {
const indexMatch = parts[parts.length - 1].match(/^index(\.\w+)/);
if (indexMatch) {
parts.pop();
parts[parts.length - 1] += indexMatch[1];
}
}
const base = parts
.pop()
?.replace(/%/g, 'u')
.replace(/\.[^.]+$/, '')
.replace(/[^a-zA-Z_$0-9]+/g, '_')
.replace(/^_/, '')
.replace(/_$/, '')
.replace(/^(\d)/, '_$1');
if (!base) {
throw new Error(`Could not derive component name from file ${filename}`);
}
return base[0].toUpperCase() + base.slice(1);
}

But what Svelte 5 does is slightly different:

  1. get_component_name
    https://github.com/sveltejs/svelte/blob/396ea2ef370e7ea5b5d4571c4e5e14384bac3ca6/packages/svelte/src/compiler/phases/2-analyze/index.js#L208-L217

  2. Which is passed through module.scope.generate:
    https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/compiler/phases/2-analyze/index.js#L399

  3. And that generate does:
    https://github.com/sveltejs/svelte/blob/396ea2ef370e7ea5b5d4571c4e5e14384bac3ca6/packages/svelte/src/compiler/phases/scope.js#L129-L150

If you combine that whole flow into a single function it would naively be something like this:

function get_component_name(filename) {
	const parts = filename.split(/[/\\]/);
	const basename = /** @type {string} */ (parts.pop());
	const last_dir = /** @type {string} */ (parts.at(-1));
	let name = basename.replace('.svelte', '');
	if (name === 'index' && last_dir && last_dir !== 'src') {
		name = last_dir;
	}
	name = name[0].toUpperCase() + name.slice(1);
  
  // 👇 this is from generate()
   return name.replace(/[^a-zA-Z0-9_$]/g, '_').replace(/^[0-9]/, '_');
}

That last line (from generate) replaces special characters like + with _. So it turns src/routes/+page.svelte into _page, while we turn it into Page.

Reproduction link

https://github.com/simonhackler/Storybook-reproduction

Reproduction steps

  1. Install dependencies and start Storybook
  2. Open the Page story and see the error

System

Storybook Environment Info:

  System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.20.2 - ~/Library/Caches/fnm_multishells/92912_1731183850302/bin/node
    Yarn: 1.22.19 - ~/Library/Caches/fnm_multishells/92912_1731183850302/bin/yarn
    npm: 10.8.3 - ~/Library/Caches/fnm_multishells/92912_1731183850302/bin/npm <----- active
    pnpm: 9.12.0 - ~/Library/Caches/fnm_multishells/92912_1731183850302/bin/pnpm
  Browsers:
    Chrome: 130.0.6723.119
    Edge: 131.0.2903.51
    Safari: 16.2
  npmPackages:
    @storybook/addon-essentials: ^8.4.4 => 8.4.4
    @storybook/addon-interactions: ^8.4.4 => 8.4.4
    @storybook/addon-svelte-csf: ^5.0.0-next.11 => 5.0.0-next.11
    @storybook/blocks: ^8.4.4 => 8.4.4
    @storybook/svelte: ^8.4.4 => 8.4.4
    @storybook/sveltekit: ^8.4.4 => 8.4.4
    @storybook/test: ^8.4.4 => 8.4.4
    storybook: ^8.4.4 => 8.4.4

Additional context

Originally reported here: storybookjs/addon-svelte-csf#231 (reply in thread)

We probably want a condition on the Svelte version, that uses the new algorithm when Svelte 5 is detected. In the perfect world we'd not try to generate the component name, but instead find it from the default export, either via a smart regex or AST parsing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Empathy Backlog
Development

No branches or pull requests

2 participants