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

UI: Upgrade to Storybook 8.3.0-beta.0, add Vitest to UI package #4802

Merged
merged 6 commits into from
Sep 25, 2024

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Aug 30, 2024

☕️ Reasoning

Storybook maintainer here. 👋
As part of our upcoming 8.3.0 release of Storybook, we're QAing it using various open source projects like this one. I figured that since I did this upgrade I might as well open a PR with it so you can benefit from it, or not.

Feel free to immediately close this PR if it's not interesting to you.

🧢 Changes

  • Upgrade Storybook to version 8.3.0-beta.0
  • Introduce Storybook's experimental Vitest-based test runner. This will run all stories as regular unit tests (in a real browser), and report as you'd expect. If any stories fail to render (none did! 💪) they'll show up as errors. If you had any Component Tests written (you don't), they'd be part of the test suite, which would then also test the behavior of your stories.

Read more about the experiment here: https://web-git-8-3-pre-release-storybookjs.vercel.app/docs/writing-tests/test-runner-with-vitest

It worked flawlessly in your setup:

image

I added appropriate scripts which turbo picks up, but I did not add it to CI.

📌 Todos

  • Upgrade to the stable 8.3.0 release when that is published (within a week from now)?

Copy link

vercel bot commented Aug 30, 2024

@JReinhold is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

@@ -8,7 +8,8 @@ const preview: Preview = {
backgrounds: { disable: true },
controls: {
matchers: {
color: /(background|color)$/i,
// matches "background" and "color" but not "solidBackground" as that is a boolean arg
color: /(?<!solid)(background|color)$/i,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this silents a console warning that the controls addon was logging in a story that uses solidBackground. The console warning was more noticeable in the Vitest tests because it would show up in the output.

"storybook-dark-mode": "^4.0.2",
"svelte": "catalog:svelte",
"svelte-check": "catalog:svelte",
"vite": "catalog:"
"util": "^0.12.5",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

util here is a temporary workaround for storybookjs/storybook#29008. Can be removed again when that fix has been released.

@JReinhold JReinhold marked this pull request as ready for review August 30, 2024 12:18
@Caleb-T-Owens
Copy link
Contributor

Hi @JReinhold, I'm very sorry for not responding to your PR. I'm going to go and apply your changes with the release version of 8.3.0

@JReinhold
Copy link
Contributor Author

No worries @Caleb-T-Owens, I also forgot to keep this PR up-to-date when we finally released 8.3.0 stable.

Let me know if you need any help, or if you have any questions or comments in general about Storybook x Svelte, feel free to reach out, I'm jreinhold on Discord, and I'm the main Svelte responsible in the Storybook core team. 👋

packages/ui/package.json Outdated Show resolved Hide resolved
@Caleb-T-Owens
Copy link
Contributor

Hey @JReinhold,

I've now bumped all the deps and the tests are running again.

I had to replace @storybook/experimental-addon-vitest with @storybook/experimental-addon-vitest and change the imports in vitest.workspace.ts from:

import { storybookTest } from '@storybook/experimental-addon-vitest/plugin';
import { storybookSveltekitPlugin } from '@storybook/sveltekit/vite'

to:

import { storybookTest } from '@storybook/experimental-addon-test/vitest-plugin';
import { storybookSveltekitPlugin } from '@storybook/sveltekit/vite-plugin';

I'm going to go ahead and merge, but feel free to let me know if I've done something drastically wrong.

A big thanks for the contribution. We're moving rather fast as a project and have only recently started the initiative of splitting out more components into a UI library which has minimal business logic, and haven't yet gotten to writing tests on them (also in part because we've got a big UI overhaul incoming 😅). However, having validation that our components all render correctly is a great peace of mind.

@Caleb-T-Owens Caleb-T-Owens merged commit 8402b78 into gitbutlerapp:master Sep 25, 2024
12 of 13 checks passed
@Caleb-T-Owens
Copy link
Contributor

@ndom91 can we do something about the Vercel bot for users who aren't verified because all the commits look like they've got failing CI because Vercel is refusing to make preview builds?

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

Successfully merging this pull request may close these issues.

2 participants