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

Addon A11y: Make Vitest Axe optional #30442

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Feb 3, 2025

Relates #30423

What I did

  • made vitest in @storybook/addon-a11y an optional peer-dependency

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-30442-sha-6e0c6f13. Try it out in a new sandbox by running npx storybook@0.0.0-pr-30442-sha-6e0c6f13 sandbox or in an existing project with npx storybook@0.0.0-pr-30442-sha-6e0c6f13 upgrade.

More information
Published version 0.0.0-pr-30442-sha-6e0c6f13
Triggered by @valentinpalkovic
Repository storybookjs/storybook
Branch valentin/make-vitest-axe-optional
Commit 6e0c6f13
Datetime Mon Feb 3 10:00:20 UTC 2025 (1738576820)
Workflow run 13110891933

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=30442

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78 MB 78 MB 6.33 kB 1.93 0%
initSize 131 MB 131 MB 6.33 kB 1.57 0%
diffSize 53 MB 53 MB 0 B 1 0%
buildSize 7.17 MB 7.17 MB 37 B 0.75 0%
buildSbAddonsSize 1.85 MB 1.85 MB 0 B 0.65 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 0 B -1 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.91 MB 3.91 MB 0 B 0.63 0%
buildPreviewSize 3.26 MB 3.26 MB 37 B 0.73 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 19.9s 11.9s -8s -33ms -1.45 🔰-67.2%
generateTime 19.7s 23.7s 3.9s 2.47 🔺16.7%
initTime 17.4s 13.9s -3s -569ms 0.4 -25.6%
buildTime 8.5s 10.7s 2.1s 1.67 🔺20.4%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5.9s 5.1s -792ms -0.25 -15.3%
devManagerResponsive 4.2s 3.9s -291ms -0.09 -7.3%
devManagerHeaderVisible 801ms 804ms 3ms 0.34 0.4%
devManagerIndexVisible 812ms 839ms 27ms 0.2 3.2%
devStoryVisibleUncached 4.1s 3.4s -697ms 0.05 -20.4%
devStoryVisible 830ms 834ms 4ms 0.13 0.5%
devAutodocsVisible 774ms 738ms -36ms 0.17 -4.9%
devMDXVisible 739ms 744ms 5ms 0.48 0.7%
buildManagerHeaderVisible 776ms 716ms -60ms -0.25 -8.4%
buildManagerIndexVisible 908ms 903ms -5ms 0.03 -0.6%
buildStoryVisible 758ms 695ms -63ms -0.25 -9.1%
buildAutodocsVisible 669ms 697ms 28ms -0.15 4%
buildMDXVisible 683ms 783ms 100ms 1.33 🔺12.8%

Greptile Summary

Makes the Vitest Axe dependency optional in the @storybook/addon-a11y package to prevent errors for users not using Vitest.

  • Moved vitest-axe from dependencies to devDependencies in code/addons/a11y/package.json
  • Modified code/addons/a11y/src/preview.tsx to dynamically import Vitest Axe matchers only when needed
  • Added flag vitestMatchersExtended to ensure matchers are only extended once
  • Maintains core accessibility testing functionality using axe-core
  • Fixes issue [Bug]: Updating from 8.4.7 to 8.5.0 on a webpack setup does not work #30423 where non-Vitest users encountered errors when upgrading to Storybook 8.5.0

@valentinpalkovic valentinpalkovic force-pushed the valentin/make-vitest-axe-optional branch from 496c142 to 6e0c6f1 Compare February 3, 2025 09:01
Copy link

nx-cloud bot commented Feb 3, 2025

View your CI Pipeline Execution ↗ for commit 8569d2c.

Command Status Duration Result
nx affected -t check -c production --parallel=7 ✅ Succeeded <1s View ↗
nx run-many -t build -c production --parallel=3 ✅ Succeeded 3s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-04 10:15:07 UTC

Copy link

nx-cloud bot commented Feb 3, 2025

View your CI Pipeline Execution ↗ for commit 6e0c6f1.

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

☁️ Nx Cloud last updated this comment at 2025-02-03 09:05:51 UTC

@valentinpalkovic valentinpalkovic added bug patch:yes Bugfix & documentation PR that need to be picked to main branch ci:normal and removed ci:normal labels Feb 3, 2025
@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Feb 3, 2025

Package Benchmarks

Commit: 8569d2c, ran on 4 February 2025 at 10:22:08 UTC

The following packages have significant changes to their size or dependencies:

@storybook/addon-a11y

Before After Difference
Dependency count 59 56 🎉 -3 🎉
Self size 228 KB 332 KB 🚨 +104 KB 🚨
Dependency size 13.47 MB 12.71 MB 🎉 -755 KB 🎉
Bundle Size Analyzer Link Link

@valentinpalkovic valentinpalkovic marked this pull request as ready for review February 4, 2025 08:31
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.

2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 60 to 61
// @ts-expect-error - todo - fix type extension of expect from @storybook/test
expect(result).toHaveNoViolations();
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding a type assertion or updating @storybook/test types to properly handle the toHaveNoViolations matcher

@valentinpalkovic valentinpalkovic force-pushed the valentin/make-vitest-axe-optional branch from ee9ebb5 to 8569d2c Compare February 4, 2025 08:37
@valentinpalkovic valentinpalkovic added ci:daily Run the CI jobs that normally run in the daily job. and removed ci:normal labels Feb 4, 2025
@valentinpalkovic valentinpalkovic merged commit b2d8a3c into next Feb 4, 2025
110 of 116 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/make-vitest-axe-optional branch February 4, 2025 10:46
shilman pushed a commit that referenced this pull request Feb 5, 2025
…ptional

Addon A11y: Make Vitest Axe optional

(cherry picked from commit b2d8a3c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:daily Run the CI jobs that normally run in the daily job. patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants