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]: Legacy Template option breaks Vitest integration #262

Closed
JReinhold opened this issue Jan 15, 2025 · 1 comment
Closed

[Bug]: Legacy Template option breaks Vitest integration #262

JReinhold opened this issue Jan 15, 2025 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@JReinhold
Copy link
Collaborator

JReinhold commented Jan 15, 2025

Describe the bug

When using the new Vitest integration via @storybook/experimental-addon-test, having the legacyTemplate: true option set will break all tests.

This is because the order of the Vite plugins are wrong when using Storybook's Vitest plugin.

The Vite plugins must run in the following order:

  1. storybook:addon-svelte-csf-legacy-api-support - has enforce: 'pre'. Transforms the Svelte source to new API, source-to-source transformation
  2. vite-plugin-svelte - has enforce: 'pre'. Transform Svelte source to JS compiled output
  3. storybook:addon-svelte-csf. Transformes JS compiled output to JS that Storybook can understand (regular CSF)
  4. vite-plugin-storybook-test. Transforms regular CSF to Vitest tests

Due to the enforce: 'pre' on vite-plugin-svelte it means that storybook:addon-svelte-csf-legacy-api-support must come before vite-plugin-svelte in the plugins array. This is fine in Storybooks dev server where it controls the plugin loading and injects them in the right order.

However when Storybook Test is just a regular Vite plugin builder, it can't modify the existing order of plugins, it can only add plugins in the place where it is added. In reality this means that in a typical setup where storybookTest() is added in the test config - after - regular Vite plugins, the order will be as follows:

  1. vite-plugin-svelte - has enforce: 'pre'
  2. storybook:addon-svelte-csf-legacy-api-support - has enforce: 'pre'
  3. storybook:addon-svelte-csf
  4. vite-plugin-storybook-test

This causes storybook:addon-svelte-csf-legacy-api-support to fail because it's expecting to get source passed in but in reality it gets the compiled output from vite-plugin-svelte.

Steps to reproduce the behavior

  1. Have a basic Svelte CSF setup and add the addon with npx storybook@next add @storybook/experimental-addon-test@next
  2. Have the legacyTemplate option enabled as described in the README: https://github.com/storybookjs/addon-svelte-csf/tree/next?tab=readme-ov-file#legacy-api
  3. Run Vitest with something like npm run test --project storybook
  4. See errors all over the place

Workaround

The workaround is to manually put storybookTest() before svelte() in the Vitest plugin array.
We generally advice users to use Vitest's workspace feature, to define a Storybook workspace: https://storybook.js.org/docs/8.5/writing-tests/test-addon#example-configuration-files . A default setup would look something like this:

// ./vite.config.js

import { defineConfig } from 'vite';
import { svelte } from '@sveltejs/vite-plugin-svelte';
import inspect from 'vite-plugin-inspect';

export default defineConfig({
  plugins: [
    svelte(),
    inspect(),
    ... more plugins
  ],
});
// ./vitest.workspace.js

import { defineWorkspace, mergeConfig } from 'vitest/config';
import { storybookTest } from '@storybook/experimental-addon-test/vitest-plugin';

export default defineWorkspace([
  // 👇 define the default "unit tests" Vitest workspace
  {
    extends: './vite.config.ts', // 👈 extend base Vite config
    test: {
      name: 'unit',
      dir: './src/',
    },
  },
  // 👇 define the Storybook workspace to test stories
  {
    extends: './vite.config.ts', // 👈 extend base Vite config
    plugins: [
      storybookTest({
        storybookScript: 'pnpm run storybook --no-open',
      }),
    ],
    test: {
      name: 'storybook',
      browser: {
        enabled: true,
        name: 'chromium',
        provider: 'playwright',
        headless: true,
      },
      setupFiles: ['./.storybook/vitest.setup.ts'],
    },
  },
]);

In the Storybook workspace, if we instead of extending the base Vite config, we copy it into the workspace definition (or import the config, however smart you want to be), that will allow us to put storybookTest() before svelte():

// ./vitest.workspace.js

import { defineWorkspace, mergeConfig } from 'vitest/config';
import { storybookTest } from '@storybook/experimental-addon-test/vitest-plugin';

export default defineWorkspace([
  // 👇 define the default "unit tests" Vitest workspace
  {
    extends: './vite.config.ts', // 👈 extend base Vite config
    test: {
      name: 'unit',
      dir: './src/',
    },
  },
  // 👇 define the Storybook workspace to test stories
  {
-   extends: './vite.config.ts', // 👈 extend base Vite config
    plugins: [
      storybookTest({
        storybookScript: 'pnpm run storybook --no-open',
      }),
      // Storybook is now BEFORE Svelte
+     svelte(),
+     inspect(),
    ],
+   // ... and any extra Vite config you have in vite.config.js
    test: {
      name: 'storybook',
      browser: {
        enabled: true,
        name: 'chromium',
        provider: 'playwright',
        headless: true,
      },
      setupFiles: ['./.storybook/vitest.setup.ts'],
    },
  },
]);

You can see this workaround being applied in this repo: https://github.com/storybookjs/addon-svelte-csf/blob/next/vitest.workspace.ts#L23-L52

Proposed solution

I don't have a good solution to this problem, this just feels like a limitation of the Vite plugin system that we're running into. Maybe the best we can do is document this and throw a nicer error from storybook:addon-svelte-csf-legacy-api-support

@dominikg do you perhaps have any ideas here with your Vite knowledge - maybe there's something I'm missing?

@dominikg
Copy link
Collaborator

not sure if it helps but have you tried using rollup hook reordering?

https://rollupjs.org/plugin-development/#build-hooks

sth like

function runMeFirst() {
	return {
		name: 'me-first',
		transform: {
			order: 'pre',
			handler: ()=>{} // your transform here
		}
	};
}

but i guess you'd have to put the oder: 'pre' on all hooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants