Addon-Test: Support for `@vitest/browser` v2.1.2 #29407

strozw · 2024-10-19T08:56:22Z

What I did

This PR has been corrected to address an error that occurs when importing @vitest/borwser/context when browser mode is not used in vitest v2.1.2 or later.

In @storybook/experiment-addon-test/vitest-plugin, change @vitest/browser/context in setViewport to dynamic import, so that when an error is thrown it is handled as { page: null } when an error is thrown, so that the same processing can be done as before.

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/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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

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

name	before	after	z	%
createSize	0 B	0 B	-	-
generateSize	78.7 MB	78.7 MB	1.58	0%
initSize	147 MB	147 MB	1.5	0%
diffSize	68.3 MB	68.3 MB	1.34	0%
buildSize	6.79 MB	6.79 MB	0.67	0%
buildSbAddonsSize	1.5 MB	1.5 MB	-0.73	0%
buildSbCommonSize	195 kB	195 kB	-	0%
buildSbManagerSize	1.83 MB	1.83 MB	0.75	0%
buildSbPreviewSize	270 kB	270 kB	-0.73	0%
buildStaticSize	0 B	0 B	-	-
buildPrebuildSize	3.8 MB	3.8 MB	0.69	0%
buildPreviewSize	2.99 MB	2.99 MB	-0.73	0%
testBuildSize	0 B	0 B	-	-
testBuildSbAddonsSize	0 B	0 B	-	-
testBuildSbCommonSize	0 B	0 B	-	-
testBuildSbManagerSize	0 B	0 B	-	-
testBuildSbPreviewSize	0 B	0 B	-	-
testBuildStaticSize	0 B	0 B	-	-
testBuildPrebuildSize	0 B	0 B	-	-
testBuildPreviewSize	0 B	0 B	-	-

name	before	after	diff	z	%
createTime	20.6s	11.9s	-8s -666ms	0.05	-72.5%
generateTime	20.9s	37.8s	16.9s	3.39	🔺44.6%
initTime	13.9s	15.2s	1.3s	0.09	8.9%
buildTime	9.2s	8.4s	-758ms	-0.23	-8.9%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	5.7s	5.9s	128ms	-0.49	2.2%
devManagerResponsive	4s	3.9s	-141ms	-0.39	-3.6%
devManagerHeaderVisible	497ms	507ms	10ms	-0.86	2%
devManagerIndexVisible	535ms	542ms	7ms	-0.81	1.3%
devStoryVisibleUncached	625ms	843ms	218ms	-0.68	25.9%
devStoryVisible	530ms	539ms	9ms	-0.86	1.7%
devAutodocsVisible	461ms	495ms	34ms	-0.34	6.9%
devMDXVisible	418ms	400ms	-18ms	-1.47	-4.5%
buildManagerHeaderVisible	486ms	465ms	-21ms	-0.77	-4.5%
buildManagerIndexVisible	500ms	468ms	-32ms	-0.85	-6.8%
buildStoryVisible	524ms	498ms	-26ms	-0.99	-5.2%
buildAutodocsVisible	463ms	465ms	2ms	-0.36	0.4%
buildMDXVisible	453ms	446ms	-7ms	-0.5	-1.6%

Greptile Summary

This PR modifies the Vitest plugin in Storybook's experimental addon-test to support @vitest/browser v2.1.2 and later versions.

Changed import of @vitest/browser/context to dynamic import in code/addons/test/src/vitest-plugin/viewports.ts
Added error handling for cases when browser mode is not used in Vitest
Ensures backward compatibility with previous versions of @vitest/browser
Addresses issue [Bug]: Tests fail in non-browser mode with @storybook/experimental-addon-test/vitest-plugin and @vitest/browser 2.1.2 or later. #29404 where tests failed in non-browser mode
Improves code resilience by gracefully handling unavailable browser context

…rrors thrown by `@vietest/browser/contest`.

JReinhold · 2024-10-19T18:26:09Z

code/package.json

@@ -82,7 +82,7 @@
    "@types/babel__traverse@npm:*": "patch:@types/babel__traverse@npm%3A7.20.6#~/.yarn/patches/@types-babel__traverse-npm-7.20.6-fac4243243.patch",
    "@types/babel__traverse@npm:^7.18.0": "patch:@types/babel__traverse@npm%3A7.20.6#~/.yarn/patches/@types-babel__traverse-npm-7.20.6-fac4243243.patch",
    "@types/node": "^22.0.0",
-    "@vitest/expect@npm:2.0.5": "patch:@vitest/expect@npm%3A2.0.5#~/.yarn/patches/@vitest-expect-npm-2.0.5-8933466cce.patch",
+    "@vitest/expect@npm:2.1.3": "patch:@vitest/expect@npm%3A2.0.5#~/.yarn/patches/@vitest-expect-npm-2.0.5-8933466cce.patch",


Notice that this still points to a patched v2.0.5, that's probably not what we want. Either we should remove the patch (if we can because of the upgrade), or re-apply the patch to v2.1.3

Thanks for pointing that out, I had overlooked that about the patch 🤦‍♂️
As for updating the vitest related packages, as you pointed out separately, there was no need to update them, so we have reverted the commits that update them.

97f93b7

JReinhold · 2024-10-19T18:26:36Z

code/addons/test/package.json

+    "@vitest/browser": "^2.1.3",
    "storybook": "workspace:^",
-    "vitest": "^2.0.0"
+    "vitest": "^2.1.3"


Are these peer deps changes necessary? Can't this change still work with Vitest 2.0.0 and onwards?

The only logic change I see in this PR is moving the import to be a dynamic import, and that's probably still compatible with vitest 2.0.0

As you say, these updates were unnecessary and I have reverted.

97f93b7

This reverts commit 484f930.

…oser to the previous form

…ater

greptile-apps

_{1 file(s) reviewed, 1 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

code/addons/test/src/vitest-plugin/viewports.ts

…ater

nx-cloud · 2024-10-21T08:17:40Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ec16750. 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 target

nx run-many -t build --parallel=3

Sent with 💌 from NxCloud.

strozw added 2 commits October 19, 2024 14:15

chore: update vitest packages to 2.1.3

484f930

fix: Fix vitest-plugin/viewports.ts so that it is not affected by e…

9abcdf6

…rrors thrown by `@vietest/browser/contest`.

JReinhold reviewed Oct 19, 2024

View reviewed changes

strozw and others added 3 commits October 20, 2024 14:48

Revert "chore: update vitest packages to 2.1.3"

97f93b7

This reverts commit 484f930.

refactor: Change dynamic import of @vitest/browser/context to be cl…

4e3bab3

…oser to the previous form

Merge branch 'next' into fix-vitest-plugin-for-vitest-browser-2.1.2-l…

5215f96

…ater

strozw marked this pull request as ready for review October 20, 2024 06:18

greptile-apps bot reviewed Oct 20, 2024

View reviewed changes

code/addons/test/src/vitest-plugin/viewports.ts Outdated Show resolved Hide resolved

strozw requested a review from JReinhold October 20, 2024 06:19

refactor: Revert unncessary changes

0284687

strozw force-pushed the fix-vitest-plugin-for-vitest-browser-2.1.2-later branch from d21a734 to 0284687 Compare October 20, 2024 06:47

valentinpalkovic approved these changes Oct 21, 2024

View reviewed changes

valentinpalkovic self-assigned this Oct 21, 2024

valentinpalkovic added ci:normal bug addon: test labels Oct 21, 2024

valentinpalkovic changed the title ~~Support for @vitest/browser v2.1.2 and later in @storybook/experiment-addon-test/vitest-plugin~~ Addon-Test: Support for @vitest/browser v2.1.2 Oct 21, 2024

Merge branch 'next' into fix-vitest-plugin-for-vitest-browser-2.1.2-l…

ec16750

…ater

valentinpalkovic merged commit 61f7746 into storybookjs:next Oct 21, 2024
48 of 53 checks passed

github-actions bot mentioned this pull request Oct 21, 2024

Release: Prerelease 8.4.0-alpha.8 #29350

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addon-Test: Support for `@vitest/browser` v2.1.2 #29407

Addon-Test: Support for `@vitest/browser` v2.1.2 #29407

strozw commented Oct 19, 2024 •

edited by storybook-pr-benchmarking bot

Loading

JReinhold Oct 19, 2024

strozw Oct 20, 2024

JReinhold Oct 19, 2024

strozw Oct 20, 2024

greptile-apps bot left a comment

nx-cloud bot commented Oct 21, 2024 •

edited

Loading

Addon-Test: Support for @vitest/browser v2.1.2 #29407

Addon-Test: Support for @vitest/browser v2.1.2 #29407

Conversation

strozw commented Oct 19, 2024 • edited by storybook-pr-benchmarking bot Loading

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

JReinhold Oct 19, 2024

Choose a reason for hiding this comment

strozw Oct 20, 2024

Choose a reason for hiding this comment

JReinhold Oct 19, 2024

Choose a reason for hiding this comment

strozw Oct 20, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

nx-cloud bot commented Oct 21, 2024 • edited Loading

☁️ Nx Cloud Report

Addon-Test: Support for `@vitest/browser` v2.1.2 #29407

Addon-Test: Support for `@vitest/browser` v2.1.2 #29407

strozw commented Oct 19, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Oct 21, 2024 •

edited

Loading