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

Vitest: Add plugins from viteFinal #30105

Open
wants to merge 20 commits into
base: next
Choose a base branch
from

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Dec 18, 2024

Closes storybookjs/addon-svelte-csf#213, follow-up to #29806 (comment)

What I did

To support Svelte CSF stories, we're now also including Vite plugins collected from the viteFinal preset (and thus addons) in our Vitest plugin. So now, instead of just returning a plugin, we're returning a list of plugins (which is totally valid). With the other changes to @storybook/addon-svelte-csf landing (storybookjs/addon-svelte-csf#244, storybookjs/addon-svelte-csf#247, storybookjs/addon-svelte-csf#248) this is all that's necessary to add automatic support for stories.svelte files.

This PR also cleans up our viteFinals frameworks, because some of them were mutating configs and plugins arrays which could cause issues if the presets were loaded in another order than expected.

Things to consider:

  1. This also adds all other plugins, like Vite docgen plugins (not just for Svelte, but also React, etc.) which could incur an unnecessary perf penalty. Do we want to handle that as part of this, or later, and how? (make it work, then make it fast?) EDIT: fixed, explicitly removing a hard coded list of known unnecessary plugins.
  2. We're now also removing the enforce: 'pre' from the Vitest plugin, meaning that it would be more affected by changes to the plugin order. We need to QA this, but currently I can't see how this would cause issues.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This pull request has been released as version 0.0.0-pr-30105-sha-a2609294. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-30105-sha-a2609294
Triggered by @JReinhold
Repository storybookjs/storybook
Branch jeppe/support-svelte-csf-in-vitest
Commit a2609294
Datetime Wed Jan 8 10:19:20 UTC 2025 (1736331560)
Workflow run 12668562395

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=30105

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.8 MB 77.8 MB 0 B -0.75 0%
initSize 131 MB 131 MB 26 B -0.65 0%
diffSize 53 MB 53 MB 26 B -0.65 0%
buildSize 7.19 MB 7.19 MB 0 B 1.67 0%
buildSbAddonsSize 1.85 MB 1.85 MB 0 B - 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.87 MB 1.87 MB 0 B -1.22 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.91 MB 3.91 MB 0 B -1.22 0%
buildPreviewSize 3.28 MB 3.28 MB 0 B 1.7 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 6.6s 7.8s 1.2s -1.06 15.4%
generateTime 19.2s 21.5s 2.2s 0.64 10.5%
initTime 13.7s 14.2s 495ms 0.22 3.5%
buildTime 9s 8.9s -189ms -0.88 -2.1%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 4.9s 4.4s -452ms -1.38 🔰-10.1%
devManagerResponsive 3.7s 3.2s -453ms -1.69 🔰-13.9%
devManagerHeaderVisible 586ms 613ms 27ms -0.49 4.4%
devManagerIndexVisible 617ms 643ms 26ms -0.57 4%
devStoryVisibleUncached 2s 2s -47ms -0.2 -2.3%
devStoryVisible 618ms 644ms 26ms -0.73 4%
devAutodocsVisible 444ms 532ms 88ms -0.55 16.5%
devMDXVisible 502ms 490ms -12ms -0.98 -2.4%
buildManagerHeaderVisible 642ms 577ms -65ms -0.68 -11.3%
buildManagerIndexVisible 741ms 671ms -70ms -0.77 -10.4%
buildStoryVisible 610ms 560ms -50ms -0.7 -8.9%
buildAutodocsVisible 548ms 501ms -47ms -0.15 -9.4%
buildMDXVisible 487ms 446ms -41ms -0.97 -9.2%

Greptile Summary

Based on the provided information, I'll create a concise summary of the key changes in this pull request:

Adds support for Svelte CSF stories in Vitest by modifying the Storybook test plugin architecture.

  • Modified storybookTest in /code/addons/test/src/vitest-plugin/index.ts to return multiple plugins and include Vite plugins from viteFinal
  • Removed framework-specific plugin configuration from postinstall script, simplifying addon setup
  • Removed enforce: 'pre' setting from Vitest plugin to improve plugin compatibility
  • Updated documentation to reflect simplified plugin configuration process
  • Refactored framework presets (Next.js, React, SvelteKit) to handle plugin arrays immutably

The changes enable automatic support for .stories.svelte files while maintaining a cleaner plugin architecture, though there are potential performance implications from including additional plugins that need to be monitored.

@JReinhold JReinhold changed the title Jeppe/support-svelte-csf-in-vitest Vitest: Add plugins from viteFinal Dec 18, 2024
Copy link

nx-cloud bot commented Dec 18, 2024

View your CI Pipeline Execution ↗ for commit a260929.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 34s View ↗

☁️ Nx Cloud last updated this comment at 2025-01-08 10:54:16 UTC

@JReinhold JReinhold self-assigned this Dec 18, 2024
@JReinhold JReinhold added feature request svelte addon: test ci:daily Run the CI jobs that normally run in the daily job. labels Dec 18, 2024
@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Dec 19, 2024

Package Benchmarks

Commit: a260929, ran on 8 January 2025 at 10:34:14 UTC

No significant changes detected, all good. 👏

</If>

The above example uses the framework's plugin in a Vitest configuration file. You can also use it in a Vitest workspace file, if that is how your project is configured.
1. Adjust your Vitest configuration to include the plugin and reference the setup file. You can use the [example configuration files](#example-configuration-files) as a guide.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the How do I apply custom Vite configuration? FAQ will also need to be either rewritten to emphasize that we automatically pull everything from viteFinal or removed outright.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if that's correct and you'd prefer I do it. Happy to help!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds about right to me. Honestly, I'd love if you could do that, either in this PR or a follow-up, up to you.

@@ -51,5 +51,5 @@ export const viteFinal: NonNullable<StorybookConfig['viteFinal']> = async (confi
);
}

return config;
return { ...config, plugins };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before, this preset would discard any config set by previous viteFinal presets. We never noticed it because this would always run first in Storybook. But in the Vitest plugin it appears that main.ts runs first, so we need to handle it properly here.

@JReinhold JReinhold marked this pull request as ready for review December 20, 2024 09:50
@JReinhold JReinhold requested a review from yannbf December 20, 2024 09:50
@JReinhold JReinhold requested a review from ndelangen December 20, 2024 09:50
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

code/addons/test/src/vitest-plugin/index.ts Show resolved Hide resolved
code/frameworks/react-vite/src/preset.ts Show resolved Hide resolved
code/frameworks/sveltekit/src/preset.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: test ci:daily Run the CI jobs that normally run in the daily job. feature request svelte
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Request] Support for portable stories and experimental vitest-addon
2 participants