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

Vitest plugin: Fix renamed export stories #29250

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

shilman
Copy link
Member

@shilman shilman commented Oct 1, 2024

Closes #29244

What I did

  1. Save the local name of export variables in the CsfFile parsed output
  2. Use that local name to fix the generated code in the Vitest plugin

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

In a sandbox, create a story that uses a renamed export and verify that the Vitest test succeeds. (I did not test!)

🦋 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 78.5 MB 714 kB 9.74 0.9%
initSize 150 MB 153 MB 2.7 MB -0.27 1.8%
diffSize 72.5 MB 74.5 MB 1.98 MB -0.44 2.7%
buildSize 6.77 MB 6.96 MB 184 kB 0.73 2.6%
buildSbAddonsSize 1.5 MB 1.57 MB 61.8 kB 0.73 3.9%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.83 MB 1.91 MB 80.8 kB 0.73 4.2%
buildSbPreviewSize 270 kB 311 kB 41.3 kB 0.73 13.3%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.8 MB 3.98 MB 184 kB 0.73 4.6%
buildPreviewSize 2.97 MB 2.97 MB 14 B -1.15 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 5.9s 7.3s 1.4s -1.01 19.6%
generateTime 20.5s 24.4s 3.9s 1.33 🔺16%
initTime 14.2s 19.9s 5.7s 2.61 🔺28.9%
buildTime 9.7s 8.7s -938ms -1.09 -10.7%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 7s 7.2s 232ms 0.07 3.2%
devManagerResponsive 4.3s 4.8s 545ms 0.4 11.2%
devManagerHeaderVisible 564ms 668ms 104ms 0.25 15.6%
devManagerIndexVisible 595ms 722ms 127ms 0.39 17.6%
devStoryVisibleUncached 1.2s 1.2s 8ms 0.19 0.6%
devStoryVisible 594ms 724ms 130ms 0.39 18%
devAutodocsVisible 498ms 646ms 148ms 1.06 22.9%
devMDXVisible 452ms 559ms 107ms 0.02 19.1%
buildManagerHeaderVisible 526ms 627ms 101ms 0.36 16.1%
buildManagerIndexVisible 559ms 632ms 73ms 0.24 11.6%
buildStoryVisible 560ms 666ms 106ms 0.25 15.9%
buildAutodocsVisible 515ms 587ms 72ms 0.23 12.3%
buildMDXVisible 504ms 542ms 38ms 0.08 7%

Greptile Summary

This PR adds support for renamed exports in the Storybook Vitest plugin, addressing issue #29244.

  • Added localName property to StaticStory interface in CsfFile.ts to store original variable names
  • Modified transformer.ts to use localName when generating test statements for renamed exports
  • Updated CsfFile.test.ts to include tests for the new localName property
  • Added a new test case in transformer.test.ts to verify handling of renamed exported stories

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

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

Copy link

nx-cloud bot commented Oct 1, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d23542f. 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 2 targets

Sent with 💌 from NxCloud.

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Thanks! Changes look good, but I found a bug in CsfFile (which could be done in a separate PR) where the story names are not handled in reexported consts like so:

const B = () => {}  
B.storyName = 'Custom name B'

const C = {
  name: 'Custom name C'
}
// story name is still B and D, should be "Custom name ..." for each story
export  { B, C as D }

@yannbf yannbf added ci:normal patch:yes Bugfix & documentation PR that need to be picked to main branch and removed ci:normal labels Oct 2, 2024
@yannbf yannbf merged commit ed00bc5 into next Oct 2, 2024
62 of 63 checks passed
@yannbf yannbf deleted the shilman/29244-vitest-renamed-exports branch October 2, 2024 15:43
storybook-bot pushed a commit that referenced this pull request Oct 2, 2024
…ed-exports

Vitest plugin: Fix renamed export stories

(cherry picked from commit ed00bc5)
storybook-bot pushed a commit that referenced this pull request Oct 3, 2024
…ed-exports

Vitest plugin: Fix renamed export stories

(cherry picked from commit ed00bc5)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: test bug ci:normal patch:done Patch/release PRs already cherry-picked to main/release branch 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.

Support renamed exports in vitest plugin e.g. export { A as B }
2 participants