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 Test: Enable strict mode #30131

Merged
merged 2 commits into from
Dec 24, 2024

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Dec 24, 2024

Closes #

What I did

Enable Typescript's strict mode for @storybook/experimental-addon-test package.

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 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 diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.8 MB 77.8 MB 0 B 1.62 0%
initSize 143 MB 143 MB 0 B 1.46 0%
diffSize 65.6 MB 65.6 MB 0 B 1.45 0%
buildSize 7.19 MB 7.19 MB 0 B -1.07 0%
buildSbAddonsSize 1.85 MB 1.85 MB 0 B -0.88 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.87 MB 1.87 MB 0 B 0.42 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.92 MB 3.92 MB 0 B -0.9 0%
buildPreviewSize 3.28 MB 3.28 MB 0 B -1.11 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 11.4s 24.5s 13.1s 1.65 🔺53.5%
generateTime 21.3s 20.8s -566ms 0.04 -2.7%
initTime 15.3s 15.5s 131ms 0.44 0.8%
buildTime 8.8s 8.9s 136ms -0.47 1.5%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 4.8s 5.7s 916ms 0.42 15.9%
devManagerResponsive 3.6s 4.2s 533ms 0.32 12.7%
devManagerHeaderVisible 562ms 690ms 128ms 0.28 18.6%
devManagerIndexVisible 574ms 713ms 139ms 0.23 19.5%
devStoryVisibleUncached 2s 2.1s 91ms 0.44 4.3%
devStoryVisible 593ms 712ms 119ms 0.19 16.7%
devAutodocsVisible 457ms 595ms 138ms 0.18 23.2%
devMDXVisible 485ms 585ms 100ms 0.03 17.1%
buildManagerHeaderVisible 637ms 627ms -10ms -0.11 -1.6%
buildManagerIndexVisible 730ms 725ms -5ms -0.11 -0.7%
buildStoryVisible 626ms 608ms -18ms -0.04 -3%
buildAutodocsVisible 453ms 557ms 104ms 0.43 18.7%
buildMDXVisible 459ms 530ms 71ms 0.38 13.4%

Greptile Summary

Here's my analysis of the PR enabling TypeScript's strict mode for the @storybook/experimental-addon-test package:

Enables TypeScript's strict mode for the test addon package, improving type safety through proper null checks, type assertions, and better handling of optional values across multiple components.

  • Added proper null checks and optional chaining in TestProviderRender.tsx and vitest-manager.ts for safer property access
  • Fixed type definitions in components like Panel.tsx and MethodCall.tsx to handle optional/nullable values
  • Added non-null assertions in StatusBadge.tsx and boot-test-runner.ts where values are guaranteed to exist
  • Several @ts-expect-error comments added that need proper follow-up fixes
  • Set strict: true in tsconfig.json to enforce stricter type checking project-wide

The changes improve type safety but some concerning @ts-expect-error suppressions should be properly addressed rather than bypassed.

@valentinpalkovic valentinpalkovic self-assigned this Dec 24, 2024
@valentinpalkovic valentinpalkovic added ci:normal build Internal-facing build tooling & test updates labels Dec 24, 2024
Copy link

nx-cloud bot commented Dec 24, 2024

View your CI Pipeline Execution ↗ for commit 3247e70.

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

☁️ Nx Cloud last updated this comment at 2024-12-24 09:20:55 UTC

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.

18 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile

code/addons/test/src/components/Panel.tsx Show resolved Hide resolved
code/addons/test/src/node/reporter.ts Show resolved Hide resolved
code/addons/test/src/node/vitest-manager.ts Show resolved Hide resolved
code/addons/test/src/vitest-plugin/index.ts Show resolved Hide resolved
code/addons/test/src/vitest-plugin/index.ts Show resolved Hide resolved
@valentinpalkovic valentinpalkovic merged commit 537acf2 into next Dec 24, 2024
60 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/enable-strict-mode-for-test branch December 24, 2024 09:40
@github-actions github-actions bot mentioned this pull request Dec 24, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Internal-facing build tooling & test updates ci:normal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants