Upgrade Storybook to 8.x (next) #2373

jandrade · 2024-11-27T13:52:29Z

Summary:

Upgrading to v8.5.x

Upgrading Storybook to experiment with the new test addon.

We are doing this so we can test the new UI testing capabilities that work with
vitest + Playwright under the hood.

It will also get us closer to test the new a11y tooling that will be available
in the next days.

NOTE: This new feature is still under development by the Storybook team, so we
are not yet ready to use it in production.

You can see more information about this new testing approach here:

Integrating the new testing approach in CI

Now that we have upgraded to the experimental version, we can take advantage of its new features.

This PR adds a new step to our GH workflows: Running Storybook component tests in CI.

Issue: XXX-XXXX

Test plan:

Run yarn storybook and check if the new test addon is available.
Also check that the Chromatic storybook environment still works.

Also verify how the storybook tests failed here:

https://github.com/Khan/wonder-blocks/actions/runs/12057096671/job/33620982112

Now check that these tests were fixed here:

https://github.com/Khan/wonder-blocks/actions/runs/12074721182/job/33673231130#step:9:1024

changeset-bot · 2024-11-27T13:52:33Z

⚠️ No Changeset found

Latest commit: 0393fdb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2024-11-27T14:41:30Z

Size Change: 0 B

Total Size: 97.3 kB

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.78 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.53 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.77 kB
`packages/wonder-blocks-breadcrumbs/dist/es/index.js`	887 B
`packages/wonder-blocks-button/dist/es/index.js`	4.04 kB
`packages/wonder-blocks-cell/dist/es/index.js`	2.01 kB
`packages/wonder-blocks-clickable/dist/es/index.js`	3.06 kB
`packages/wonder-blocks-core/dist/es/index.js`	3.48 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.24 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	19.2 kB
`packages/wonder-blocks-form/dist/es/index.js`	6.28 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.36 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	2.95 kB
`packages/wonder-blocks-icon/dist/es/index.js`	871 B
`packages/wonder-blocks-labeled-field/dist/es/index.js`	72 B
`packages/wonder-blocks-layout/dist/es/index.js`	1.82 kB
`packages/wonder-blocks-link/dist/es/index.js`	2.28 kB
`packages/wonder-blocks-modal/dist/es/index.js`	5.43 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.65 kB
`packages/wonder-blocks-popover/dist/es/index.js`	4.87 kB
`packages/wonder-blocks-progress-spinner/dist/es/index.js`	1.52 kB
`packages/wonder-blocks-search-field/dist/es/index.js`	1.38 kB
`packages/wonder-blocks-switch/dist/es/index.js`	1.94 kB
`packages/wonder-blocks-testing-core/dist/es/index.js`	3.74 kB
`packages/wonder-blocks-testing/dist/es/index.js`	1.07 kB
`packages/wonder-blocks-theming/dist/es/index.js`	693 B
`packages/wonder-blocks-timing/dist/es/index.js`	1.8 kB
`packages/wonder-blocks-tokens/dist/es/index.js`	2.36 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	905 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	7.08 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.23 kB

_{compressed-size-action}

github-actions · 2024-11-27T14:42:22Z

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-cfoaqzhznw.chromatic.com/

Chromatic results:

Metric	Total
Captured snapshots	377
Tests with visual changes	0
Total stories	513
Inherited (not captured) snapshots [TurboSnap]	0
Tests on the build	377

jandrade · 2024-11-27T16:49:32Z

.storybook/main.ts

    ],
    staticDirs: ["../static"],
    core: {
        disableTelemetry: true,
    },
-    framework: getAbsolutePath("@storybook/react-vite"),
-    async viteFinal(config, {configType}) {


note: I'm moving this to a separate vite.config.ts file for better integration with vitest.

y'all are switching to vitest?

I see, it's just for running storybook tests.

correct! I'm thinking that we should probably write an ADR if we want to adopt this more widely.

Just curious.... I know this is a big change, but I wonder if you have any opinions about switching from Jest to vitest for unit tests?

Jest is definitely a more well-trodden path as it's been around for longer. But Vitest is a solid testing framework! I was able to do everything I needed with Vitest and Testing Library. It's supposedly faster and easier to configure than Jest, too.

jandrade · 2024-11-27T16:49:58Z

.storybook/main.ts

-        if (configType === "PRODUCTION") {
-            config.plugins?.push(
-                turbosnap({rootDir: config.root || process.cwd()}),
-            );
-        }


note: Dropping this as it is no longer needed as a separate package. SB 8 now includes it internally.

jandrade · 2024-11-28T19:44:19Z

__docs__/wonder-blocks-tooltip/tooltip.stories.tsx

@@ -413,6 +396,23 @@ export const AutoUpdate: StoryComponentType = {
                >
                    Click to update trigger position (fixed)
                </Button>
+                <Tooltip


note: Moved this instance to the right so it can work more consistently in different environments (browser and JSDOM).

The issue with the previous story was that the assertion in the play function was not working properly in headless browsers (e.g. running npx vitest) because the viewport/document size was different from the browser one.

This was causing that the Tooltip was not positioned horizontally, causing the test to fail in CI. With this change, we can now ensure that there will be a horizontal reposition.

jandrade · 2024-11-28T19:45:16Z

vite.config.ts

note: Moved this to a separate config file so it can be reused by Storybook and vitest.

jandrade · 2024-11-28T20:03:16Z

vitest.config.ts

note: This file was generated when I added the experimental addon. More info here: https://storybook.js.org/docs/writing-tests/test-addon#automatic-setup

jandrade · 2024-11-28T20:03:25Z

.storybook/vitest.setup.ts

note: This file was generated when I added the experimental addon. More info here: https://storybook.js.org/docs/writing-tests/test-addon#automatic-setup

jandrade · 2024-11-28T20:04:45Z

__docs__/wonder-blocks-link/link.stories.tsx

-    // compare different color formats, so hex was converted to RGB.
-    await expect(computedStyle.outline).toBe("rgb(24, 101, 242) solid 1px");
+    // rgb(24, 101, 242) is the same as Color.blue
+    await expect(link).toHaveStyle("outline: rgb(24, 101, 242) solid 1px");


note: This and other similar assertions where changed b/c getComputedStyle doesn't work with the new testing approach.

I got these errors the first time I integrated the new SB test CLI in CI, and made these changes to fix the failures.

What do we think about tests that check for specific styles? I had started removing jest tests that were asserting styles since it would be visually covered by Chromatic (especially with the "All Variants" stories + pseudostates add on!). Is it helpful to also have these component test checks for styles?

Kinda related - In general, when would it be helpful for us to reach for component tests in Storybook? I'm thinking if there's functionality we want to test for that relies on browser behaviour! Curious to see what other think!

100% agree with you. We should move this to "All variants" but the issue is that Link (and Button) still rely on Clickable for setting pseudo states styles. As you know, I'd love to get us to a point where we don't use Clickable for those styles, and we let CSS handle that for us, but that's a tricky change that would be definitively nice to address.

Once we migrate to CSS pseudoclasses, I'm totally in favor of moving this to visual regression tests instead of interaction tests.

jandrade · 2024-11-28T20:05:23Z

__docs__/wonder-blocks-switch/switch.stories.tsx

-export const Controlled: StoryComponentType = () => {
-    const [checkedOne, setCheckedOne] = React.useState(false);
-    const [checkedTwo, setCheckedTwo] = React.useState(false);
+export const Controlled: StoryComponentType = {


note: This component doesn't have any big changes, just moved the code around to support CSF3.

…ndler' for module resolution

khan-actions-bot · 2024-11-28T20:36:42Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to package.json, tsconfig-common.json, vite.config.ts, vitest.config.ts, yarn.lock, .storybook/main.ts, .storybook/preview.tsx, .storybook/vitest.setup.ts, .github/workflows/chromatic-build.yml, .github/workflows/chromatic-pr.yml, __docs__/wonder-blocks-dropdown/combobox.stories.tsx, __docs__/wonder-blocks-switch/switch.stories.tsx, __docs__/wonder-blocks-tooltip/tooltip.stories.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2024-11-28T20:36:48Z

npm Snapshot: NOT Published

🤕 Oh noes!! We couldn't find any changesets in this PR (10b4b74). As a result, we did not publish an npm snapshot for you.

jandrade · 2024-11-28T20:38:39Z

Here's an example of how the new Test addon works locally:

Screen.Recording.2024-11-28.at.3.37.08.PM.mov

jeremywiebe

Very neat! We'll have to see if component tests can be used in Perseus.

jeremywiebe · 2024-11-28T23:52:11Z

.storybook/main.ts

@@ -9,52 +6,17 @@ const config: StorybookConfig = {
        "../__docs__/**/*.mdx",
    ],
    addons: [
-        getAbsolutePath("@storybook/addon-essentials"),


Storybook upgrades always want to drop this back in. I finally searched when it's needed. Apparently in Yarn PnP situations module resolution can fail without this. Good to know we can remove this in Perseus too then.

https://storybook.js.org/docs/faq#how-do-i-fix-module-resolution-in-special-environments

oh I see, thanks for the context.

jeremywiebe · 2024-11-28T23:54:39Z

.storybook/preview.tsx

@@ -1,6 +1,6 @@
 import * as React from "react";
 import wonderBlocksTheme from "./wonder-blocks-theme";
-
+import {Decorator} from "@storybook/react";


Totally out of the blue and unrelated to this PR, but it looks like you don't have import/order enabled in Wonder Blocks. I (not sure about others Perseus) really love how it automatically orders imports so I don't think about it... although, maybe you don't think about it either. :D

https://github.com/Khan/perseus/blob/main/.eslintrc.js#L220..L236

Great point! I'll add this rule in a separate PR. I'll probably wait until WB is in a more "quiet" state to apply the changes :)

jeremywiebe · 2024-11-29T00:01:37Z

__docs__/wonder-blocks-dropdown/combobox.stories.tsx

@@ -332,7 +332,7 @@ export const ControlledMultilpleCombobox: Story = {
    ],

    play: async ({canvasElement}) => {
-        const canvas = within(canvasElement);
+        const canvas = within(canvasElement.ownerDocument.body);


Interesting, this differs from the examples for these play() functions. :( They always show how you originally had it.

Ahh yeah, this is useful for when we are using portals. I should add a comment here stating that.

jeremywiebe · 2024-11-29T00:04:23Z

package.json

-    "@storybook/react": "^8.2.1",
-    "@storybook/react-vite": "^8.2.1",
-    "@storybook/test": "^8.2.1",
+    "@storybook/addon-a11y": "^8.5.0-alpha.10",


-alpha.10 😱

Are you planning to wait till this is released before landing? 😁

hehe living on the edge! j/k

I can totally wait until it is officially released :). I mainly wanted to start this PR to be able to unlock their upcoming Accessibility addon (that uses this new testing tooling), so @marcysutton can try it out and give us some feedback, and also give feedback to the Storybook team as well, which she's planning to do :).

kevinb-khan · 2024-11-29T16:45:07Z

Here's an example of how the new Test addon works locally:

In the screen recording it looked like there were three errors, but when you clicked on each test individually it looked like they were all passing. Am I misinterpreting what I was seeing?

kevinb-khan

LGTM. You may want to wait until 8.5 is no longer alpha as Jeremy suggested.

kevinb-khan · 2024-11-29T16:58:51Z

.storybook/main.ts

    ],
    staticDirs: ["../static"],
    core: {
        disableTelemetry: true,
    },
-    framework: getAbsolutePath("@storybook/react-vite"),
-    async viteFinal(config, {configType}) {


y'all are switching to vitest?

kevinb-khan · 2024-11-29T17:00:10Z

.storybook/main.ts

    ],
    staticDirs: ["../static"],
    core: {
        disableTelemetry: true,
    },
-    framework: getAbsolutePath("@storybook/react-vite"),
-    async viteFinal(config, {configType}) {


I see, it's just for running storybook tests.

kevinb-khan · 2024-11-29T17:02:13Z

__docs__/wonder-blocks-tooltip/tooltip.stories.tsx

-                <Tooltip
-                    content="This is a tooltip that auto-updates its position when the trigger element changes."
-                    opened={true}
-                    autoUpdate={true}
-                >
-                    <View
-                        style={[
-                            position && {
-                                position: "absolute",
-                                top: position.y,
-                                left: position.x,
-                            },
-                        ]}
-                    >
-                        Trigger element
-                    </View>
-                </Tooltip>


Why was this removed?

It was moved to be included after the buttons. See my previous comment here: https://github.com/Khan/wonder-blocks/pull/2373/files#r1862635270

kevinb-khan · 2024-11-29T17:02:52Z

__docs__/wonder-blocks-switch/switch.stories.tsx

-    );
-};
-
-Controlled.play = async ({canvasElement}) => {


In previous versions of Storybook you had to manually navigate to each story and press "play" to run the tests, right?

Right! and that made it hard to determine when we introduced a regression with these interaction tests. I also added the CI step to ensure that we don't introduce issues in the future.

NOTE: Storybook uses the test-runner for this, but it will be replaced by this new test addon.

The previous runner used Jest + Playwright, and they wanted to switch to Vitest specially because they can run these tests in browsers as well as in headless mode. This is a new vitest feature that is also being refined in collaboration between the Storybook and vitest teams: https://vitest.dev/guide/browser/

More context here: https://storybook.js.org/blog/storybook-test-sneak-peek/

jandrade · 2024-11-29T17:28:28Z

Here's an example of how the new Test addon works locally:

In the screen recording it looked like there were three errors, but when you clicked on each test individually it looked like they were all passing. Am I misinterpreting what I was seeing?

@kevinb-khan good question. I added the video to use as a visual reference when we see failures in the browser. You can see the failures being reported in headless mode in CI here: https://github.com/Khan/wonder-blocks/actions/runs/12057096671/job/33620982112

What you see in the individual views, those tests were passing, but were considered flaky as it seemed that the results between browser and CI diverged. I asked the Storybook team about that, and they found the issue which I fixed here: 93279fa

The issues were not flaky per se, it was that I misconfigured the global decorators in preview.tsx.

For more context, the Storybook folks helped me to find the cause here: https://discord.com/channels/486522875931656193/1301551207835504694/1311423632412774481

I joined their #sb-test-early-access channel in the Storybook Discord server to be able to experiment with this new feature in our repo and get early feedback from them.

kevinb-khan · 2024-11-29T17:43:20Z

Here's an example of how the new Test addon works locally:

In the screen recording it looked like there were three errors, but when you clicked on each test individually it looked like they were all passing. Am I misinterpreting what I was seeing?

@kevinb-khan good question. I added the video to use as a visual reference when we see failures in the browser. You can see the failures being reported in headless mode in CI here: https://github.com/Khan/wonder-blocks/actions/runs/12057096671/job/33620982112

That's super cool that these tests run on CI in Chromatic. This is really powerful stuff. Thanks for setting this up.

beaesguerra

Thanks for setting this up! Looking forward to the upcoming a11y tooling 😄

beaesguerra · 2024-11-29T18:27:05Z

__docs__/wonder-blocks-link/link.stories.tsx

-    // compare different color formats, so hex was converted to RGB.
-    await expect(computedStyle.outline).toBe("rgb(24, 101, 242) solid 1px");
+    // rgb(24, 101, 242) is the same as Color.blue
+    await expect(link).toHaveStyle("outline: rgb(24, 101, 242) solid 1px");


What do we think about tests that check for specific styles? I had started removing jest tests that were asserting styles since it would be visually covered by Chromatic (especially with the "All Variants" stories + pseudostates add on!). Is it helpful to also have these component test checks for styles?

Kinda related - In general, when would it be helpful for us to reach for component tests in Storybook? I'm thinking if there's functionality we want to test for that relies on browser behaviour! Curious to see what other think!

Upgrade Storybook to latest version (next)

83cc551

jandrade self-assigned this Nov 27, 2024

Rever package.json change

d4991cf

jandrade added 9 commits November 27, 2024 09:59

Remove unsused dep, fix lint issues

f63d521

More lint fixes

6d82915

Revert some TS changes

91ed3aa

Upgrade SB eslint plugin

508d892

Add storybook test support in CI

f111d08

Reverting back ESLint SB version, fix SB tests tooling for CI

e0a5132

Pre-install Playwright in CI

c4fa248

Fix some Storybook interaction tests

6ad522e

Export decorators properly in preview.tsx

93279fa

jandrade commented Nov 28, 2024

View reviewed changes

Final fixes

3f3d310

jandrade changed the title ~~Upgrade Storybook to latest version (next)~~ Upgrade Storybook to 8.x (next) Nov 28, 2024

jandrade commented Nov 28, 2024

View reviewed changes

kevinb-khan and others added 2 commits November 28, 2024 15:26

Update TypeScript to 5.7.2 and modify tsconfig-common.json ot use 'bu…

8875ff2

…ndler' for module resolution

Change chromatic GH step to be explicit about the testing part

7060439

jandrade marked this pull request as ready for review November 28, 2024 20:36

khan-actions-bot requested a review from a team November 28, 2024 20:36

jeremywiebe reviewed Nov 29, 2024

View reviewed changes

kevinb-khan approved these changes Nov 29, 2024

View reviewed changes

beaesguerra reviewed Nov 29, 2024

View reviewed changes

jandrade added 3 commits December 11, 2024 12:17

Fix merge conflicts

a136065

Merge remote-tracking branch 'origin' into sb-85

9d18c59

Remove react v16 alias in vite config

0393fdb

jandrade mentioned this pull request Dec 12, 2024

Storybook 8.5: Enable accessibility testing #2393

Draft

Upgrade Storybook to 8.x (next) #2373

Are you sure you want to change the base?

Upgrade Storybook to 8.x (next) #2373

Conversation

jandrade commented Nov 27, 2024 • edited Loading

Summary:

Upgrading to v8.5.x

Integrating the new testing approach in CI

Test plan:

changeset-bot bot commented Nov 27, 2024 • edited Loading

⚠️ No Changeset found

github-actions bot commented Nov 27, 2024 • edited Loading

github-actions bot commented Nov 27, 2024 • edited Loading

A new build was pushed to Chromatic! 🚀

Chromatic results:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khan-actions-bot commented Nov 28, 2024 • edited Loading

Gerald

github-actions bot commented Nov 28, 2024 • edited Loading

npm Snapshot: NOT Published

jandrade commented Nov 28, 2024

jeremywiebe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinb-khan commented Nov 29, 2024

kevinb-khan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jandrade commented Nov 29, 2024

kevinb-khan commented Nov 29, 2024

beaesguerra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jandrade commented Nov 27, 2024 •

edited

Loading

changeset-bot bot commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited

Loading

khan-actions-bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading