Vitest: Implement add command for vitest addon #28920
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings
| storybookTest(),${vitestInfo.frameworkPluginCall ? '\n' + vitestInfo.frameworkPluginCall : ''} | ||
| ], | ||
| test: { | ||
| include: ['**/*.{stories}.?(m)[jt]s?(x)'], |
There was a problem hiding this comment.
style: The glob pattern for stories files might be too permissive
| const builderPackageJson = await fs.readFile( | ||
| `${typeof builder === 'string' ? builder : builder.name}/package.json`, | ||
| 'utf8' | ||
| ); |
There was a problem hiding this comment.
style: Wrap this in a try/catch to handle potential file read errors
☁️ Nx Cloud ReportCI is running/has finished running commands for commit dc9432a. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
There was a problem hiding this comment.
3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings
| storybookTest(),${vitestInfo.frameworkPluginCall ? '\n' + vitestInfo.frameworkPluginCall : ''} | ||
| ], | ||
| test: { | ||
| include: ['**/*.stories.?(m)[jt]s?(x)'], |
There was a problem hiding this comment.
style: The glob pattern for stories files might be too permissive
| const builderPackageJson = await fs.readFile( | ||
| `${typeof builder === 'string' ? builder : builder.name}/package.json`, | ||
| 'utf8' | ||
| ); |
There was a problem hiding this comment.
style: Wrap this in a try/catch to handle potential file read errors
|
Failed to publish canary version of this pull request, triggered by @yannbf. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/10473560827 |
There was a problem hiding this comment.
Nice work!! We should revamp all of these messages, hopefully Kyle can assist. It's via the CLI where we will introduce the feature to people, so we should make sure they have a great experience through it.
Here are some suggestions for messages:
- The Vitest addon is going to set up its plugin for you
- Configuring Vitest, Playwright and Vitest browser mode dependencies
- We detected that you're using Next.js. We will configure the
vite-plugin-storybook-nextjsplugin to allow you to run tests in Vitest. More info at: [some-link-here] - We detected that you have Vitest and workspaces configured, so we couldn't automatically set the plugin for you. Please refer to the documentation on how to complete the set up: [some-link]
- There were some issues when setting up the plugin, please follow the steps to manually set up the plugin instead: [some-link]
- The Vitest addon is now configured and you're ready to run your tests! Check the documentation for more information about its features and configurations at [some-link]
Co-authored-by: Kyle Gach <[email protected]>
Co-authored-by: Kyle Gach <[email protected]>
Co-authored-by: Kyle Gach <[email protected]>
Co-authored-by: Kyle Gach <[email protected]>
Co-authored-by: Kyle Gach <[email protected]>
|
|
||
| const project = setProjectAnnotations(${previewExists ? 'projectAnnotations' : '[]'}) | ||
|
|
||
| beforeAll(project.beforeAll) |
There was a problem hiding this comment.
Please make sure the snippet matches what we have in the docs for portable stories, or change the docs for portable stories to match the snippet
There was a problem hiding this comment.
I'm not sure what you mean?
import { beforeAll } from 'vitest';
import { render as testingLibraryRender } from '@testing-library/svelte';
import { setProjectAnnotations } from '@storybook/svelte';
// 👇 Import the exported annotations, if any, from the addons you're using; otherwise remove this
import * as addonAnnotations from 'my-addon/preview';
import * as previewAnnotations from './.storybook/preview';
const annotations = setProjectAnnotations([
previewAnnotations,
addonAnnotations,
// You MUST provide this option to use portable stories with vitest
{ testingLibraryRender },
]);
// Run Storybook's beforeAll hook
beforeAll(annotations.beforeAll);I can remove the testingLibraryRender indeed. Do you mean that?
There was a problem hiding this comment.
@kylegach Let's discuss this for another PR
| const { builder, renderer } = core; | ||
|
|
||
| if (!builder || !renderer) { | ||
| throw new Error('Could not detect your Storybook framework.'); |
There was a problem hiding this comment.
Please tell the user what to do in this situation, I'd assume they passed the wrong config dir?
There was a problem hiding this comment.
They will get an error earlier when we validate the framework:
validateFrameworkName(frameworkName);
I think this can only happen if you have a mal-configured community framework. So I think it is very unlikely.
There was a problem hiding this comment.
Also, in the dev command, we don't throw anything here. As typescript is okay with it.
|
|
||
| const extensions = ['.ts', '.mts', '.cts', '.js', '.mjs', '.cjs']; | ||
|
|
||
| export default async function postInstall(options: PostinstallOptions) { |
There was a problem hiding this comment.
Do we need to do any version checks here? Given that these packages will only fully work in Storybook 8.3? Specially sveltekit
There was a problem hiding this comment.
But this postinstall will only be available in 8.3 right?
| } | ||
| logger.info(c.bold('Installing packages...')); | ||
| logger.info(packages.join(', ')); | ||
| await packageManager.addDependencies({ installAsDevDependencies: true }, packages); |
There was a problem hiding this comment.
What if this or any of the other operations fail? Should we wrap in a try/catch to provide some narrative about the failure? Perhaps we may want to add telemetry to track these?
| ` | ||
| ); | ||
|
|
||
| const configFiles = extensions.map((ext) => 'vitest.config' + ext); |
There was a problem hiding this comment.
Maybe we should do this mapping where extensions are defined as well?
| logger.info(c.bold('Writing vitest.config.ts file...')); | ||
| await writeFile( | ||
| resolve('vitest.config.ts'), | ||
| dedent` |
There was a problem hiding this comment.
I think these code snippets should probably be in a separate file somewhere, rather than inline.
| validateFrameworkName(frameworkName); | ||
| const frameworkPackageName = extractProperFrameworkName(frameworkName); | ||
|
|
||
| console.log(configDir); |
There was a problem hiding this comment.
I think this needs to be removed.
|
Failed to publish canary version of this pull request, triggered by @kasperpeulen. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/10526145473 |
kasperpeulen
added
needs qa
Closes #28807
What I did
Implement add command for vitest addon
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.tsMake 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-28920-sha-cd8f1f27. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-28920-sha-cd8f1f27kasper/vitest-addcd8f1f271724419799)To request a new release of this pull request, mention the
@storybookjs/coreteam.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=28920Greptile Summary
This PR enhances the Vitest addon installation process for Storybook, focusing on improved framework support and configuration setup.
code/addons/vitest/src/postinstall.tsfor automatic Vitest setup across various frameworksvitest.workspace.tsorvitest.config.tsbased on existing project configurationcode/lib/cli-storybook/src/add.tsto includeconfigDirin PostinstallOptions for more flexible addon installation