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

Preview: Add globals to extract() #30415

Merged
merged 5 commits into from
Jan 29, 2025
Merged

Preview: Add globals to extract() #30415

merged 5 commits into from
Jan 29, 2025

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jan 29, 2025

What I did

I added globals to the extract() method.

This is important for integrators to get some details about the story correct, such as the currently selected background and viewport.

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 is how I manually verified this change:

  1. open the main storybook: nr storybook:ui
  2. eject the iframe
  3. open devtools and open the console
  4. type this: await __STORYBOOK_PREVIEW__.storyStoreValue.cacheAllCSFFiles()
  5. wait for the above to complete and then type: await __STORYBOOK_PREVIEW__.storyStoreValue.extract()
  6. Now verify that every story object contains a property called globals.
  7. Now verify that the story with ID=addons-backgrounds-globals--set has globals.background.value="red"

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.9 MB 77.9 MB 0 B 1.37 0%
initSize 131 MB 131 MB 600 B 1.01 0%
diffSize 53 MB 53 MB 600 B -0.29 0%
buildSize 7.17 MB 7.17 MB 364 B -1.18 0%
buildSbAddonsSize 1.85 MB 1.85 MB 273 B 2.44 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 0 B -1.22 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.91 MB 3.91 MB 273 B -1.11 0%
buildPreviewSize 3.26 MB 3.26 MB 91 B -1.21 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 25.7s 19.1s -6s -621ms 0.5 -34.6%
generateTime 17.4s 19.7s 2.2s -0.63 11.6%
initTime 11.5s 13.3s 1.7s -0.6 13.4%
buildTime 8.5s 9.4s 866ms -0.1 9.2%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5s 5.8s 754ms 1.61 🔺13%
devManagerResponsive 3.7s 4.3s 651ms 2.67 🔺14.9%
devManagerHeaderVisible 771ms 784ms 13ms 2.09 1.7%
devManagerIndexVisible 852ms 847ms -5ms 1.92 -0.6%
devStoryVisibleUncached 2.6s 4.1s 1.4s 6.91 🔺34.8%
devStoryVisible 853ms 880ms 27ms 2.56 3.1%
devAutodocsVisible 688ms 783ms 95ms 2.37 🔺12.1%
devMDXVisible 705ms 876ms 171ms 4.28 🔺19.5%
buildManagerHeaderVisible 713ms 776ms 63ms 1.74 🔺8.1%
buildManagerIndexVisible 848ms 861ms 13ms 1.62 1.5%
buildStoryVisible 691ms 707ms 16ms 1.24 2.3%
buildAutodocsVisible 776ms 640ms -136ms 1.27 🔰-21.2%
buildMDXVisible 624ms 679ms 55ms 2.31 🔺8.1%

Greptile Summary

Added globals support to the StoryStore's extract() method, enabling integrators to access story-specific settings like background and viewport configurations.

  • Modified code/core/src/preview-api/modules/store/StoryStore.ts to include globals from userGlobals.initialGlobals, userGlobals.globals, and story.storyGlobals in the extracted story data
  • Follows existing pattern from getStoryContext() for consistent globals merging order
  • Added manual testing steps to verify globals property in extracted stories, specifically checking background value in 'addons-backgrounds-globals--set' story

Copy link

nx-cloud bot commented Jan 29, 2025

View your CI Pipeline Execution ↗ for commit 0f065e7.

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

☁️ Nx Cloud last updated this comment at 2025-01-29 15:52:01 UTC

@ndelangen ndelangen changed the title add globals to extract Preview: Add globals to extract() Jan 29, 2025
@ndelangen ndelangen marked this pull request as ready for review January 29, 2025 14:36
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 | Greptile

@ndelangen ndelangen added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jan 29, 2025
@ndelangen ndelangen merged commit c4ff1b4 into next Jan 29, 2025
59 checks passed
@ndelangen ndelangen deleted the norbert/extract-globals branch January 29, 2025 16:07
args: story.initialArgs,
globals: {
...this.userGlobals.initialGlobals,
...this.userGlobals.globals,
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to include "temporary" global changes in extract?

For instance if I go to http://localhost:6006 should I get a different result for .extract() than if I go to http://localhost:6006?globals:X=y ?

shilman pushed a commit that referenced this pull request Feb 1, 2025
Preview: Add `globals` to `extract()`
(cherry picked from commit c4ff1b4)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal feature request 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 preview-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants