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

React-vite: Upgrade react-docgen-typescript plugin #29286

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

Sidnioulz
Copy link
Contributor

@Sidnioulz Sidnioulz commented Oct 6, 2024

What I did

I updated @joshwooding/[email protected] in the react-vite framework.

The current version uses a deprecated version of glob, which throws warnings when Storybook is used with PNPM and looks kinda goofy.

There still is one instance of the deprecated glob in the ecosystem, through auto used in the addon-kit, but those aren't as public-facing so they're less detrimental reputation-wise.

Checklist for Contributors

Testing

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

Caution

No automated tests were made. I do not know if CI would catch react docgen issues.

Manual testing

I built the package and ran storybook:ui, did not spot any errors, but there is relatively little docgen input data in that stack as almost no prop tables have descriptions.

I cannot guarantee for sure an absence of regressions, though I believe the upstream dep update was non-breaking despite the < v0 minor change.

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 diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.7 MB 78.7 MB 0 B 1.53 0%
initSize 151 MB 151 MB -111 kB -0.53 -0.1%
diffSize 72.6 MB 72.4 MB -111 kB -1.08 -0.2%
buildSize 6.77 MB 6.77 MB 0 B -0.65 0%
buildSbAddonsSize 1.5 MB 1.5 MB 0 B -0.65 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.83 MB 1.83 MB 0 B -0.63 0%
buildSbPreviewSize 270 kB 270 kB 0 B -0.65 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.8 MB 3.8 MB 0 B -0.65 0%
buildPreviewSize 2.97 MB 2.97 MB 0 B -0.93 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 20s 15.1s -4s -862ms 0.2 -32.1%
generateTime 21.1s 23.6s 2.4s 0.97 10.5%
initTime 15.4s 16s 524ms 0.56 3.3%
buildTime 10s 9.3s -649ms -0.37 -6.9%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.8s 7.7s 871ms 0.59 11.2%
devManagerResponsive 4.3s 5s 635ms 0.64 12.6%
devManagerHeaderVisible 656ms 627ms -29ms 0.1 -4.6%
devManagerIndexVisible 690ms 676ms -14ms 0.22 -2.1%
devStoryVisibleUncached 1.2s 1.1s -67ms -0.11 -5.8%
devStoryVisible 684ms 674ms -10ms 0.19 -1.5%
devAutodocsVisible 547ms 512ms -35ms -0.09 -6.8%
devMDXVisible 613ms 501ms -112ms -0.3 -22.4%
buildManagerHeaderVisible 593ms 533ms -60ms -0.22 -11.3%
buildManagerIndexVisible 595ms 572ms -23ms -0.1 -4%
buildStoryVisible 670ms 570ms -100ms -0.29 -17.5%
buildAutodocsVisible 498ms 520ms 22ms -0.16 4.2%
buildMDXVisible 556ms 620ms 64ms 0.73 10.3%

Greptile Summary

This pull request updates the @joshwooding/vite-plugin-react-docgen-typescript dependency in the react-vite framework to address warnings related to a deprecated 'glob' version when using Storybook with PNPM.

  • Updated @joshwooding/vite-plugin-react-docgen-typescript from 0.3.0 to ^0.4.1 in code/frameworks/react-vite/package.json
  • Change aims to resolve warnings when Storybook is used with PNPM
  • No automated tests added; manual testing limited to building and running 'storybook:ui'
  • Potential impact on React docgen functionality, though believed to be non-breaking
  • Author notes one remaining instance of deprecated 'glob' in the ecosystem through 'auto' in addon-kit

@Sidnioulz
Copy link
Contributor Author

@ndelangen judging by the Git history on this package, you might be the best suited reviewer.

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.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

nx-cloud bot commented Oct 6, 2024

☁️ Nx Cloud Report

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

Sent with 💌 from NxCloud.

@shilman shilman changed the title chore: Update react-docgen-typescript plugin to avoid deprecated dep React: Update react-docgen-typescript plugin Oct 7, 2024
@shilman shilman changed the title React: Update react-docgen-typescript plugin React-vite: Upgrade react-docgen-typescript plugin Oct 7, 2024
@shilman shilman assigned shilman and kasperpeulen and unassigned shilman Oct 7, 2024
@shilman
Copy link
Member

shilman commented Oct 7, 2024

@kasperpeulen Can you please test and verify that this does not bring the HMR performance issue back? We specifically downgraded this package in #29184, but @joshwooding thinks that this has been fixed in the latest release.

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.

3 participants