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

Source: Support async parameters.docs.source.transform #30426

Open
wants to merge 11 commits into
base: next
Choose a base branch
from

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Jan 30, 2025

Closes #24236
Closes #23703

What I did

  • Added support to pass an async function to parameters.docs.source.transform
  • Deprecated parameters.docs.source.format

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 80.4 MB 80.4 MB 319 B 2.34 0%
initSize 80.4 MB 80.4 MB 319 B -1.31 0%
diffSize 97 B 97 B 0 B -1.36 0%
buildSize 7.25 MB 7.25 MB 2.37 kB 1.77 0%
buildSbAddonsSize 1.87 MB 1.87 MB 1.42 kB 1.16 0.1%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.88 MB 1.88 MB 0 B 1.11 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.94 MB 3.95 MB 1.42 kB 1.14 0%
buildPreviewSize 3.3 MB 3.3 MB 953 B 2.34 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 22s 24.6s 2.6s 0.56 10.6%
generateTime 19s 18.6s -417ms -0.58 -2.2%
initTime 4.7s 4.5s -227ms -1.31 🔰-5%
buildTime 8.3s 8.5s 272ms -0.59 3.2%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6s 6.8s 750ms 2.13 🔺11%
devManagerResponsive 4.4s 4.9s 518ms 2.01 🔺10.5%
devManagerHeaderVisible 966ms 1s 38ms 1.62 3.8%
devManagerIndexVisible 1s 1s 58ms 1.79 🔺5.5%
devStoryVisibleUncached 4.4s 4.4s 11ms 1.19 0.2%
devStoryVisible 1s 1.1s 114ms 1.96 🔺10.2%
devAutodocsVisible 924ms 871ms -53ms 1.03 -6.1%
devMDXVisible 879ms 996ms 117ms 2.64 🔺11.7%
buildManagerHeaderVisible 791ms 1.1s 323ms 0.29 29%
buildManagerIndexVisible 955ms 1.2s 290ms 0.35 23.3%
buildStoryVisible 768ms 986ms 218ms 0.25 22.1%
buildAutodocsVisible 727ms 889ms 162ms -0.12 18.2%
buildMDXVisible 710ms 858ms 148ms 1.41 🔺17.2%

Greptile Summary

Added support for asynchronous source code transformation in Storybook's Source Doc Block, allowing dynamic code processing with async functions while deprecating the format parameter.

  • Added async support to parameters.docs.source.transform in code/lib/blocks/src/blocks/Source.tsx using React hooks
  • Added new AsyncTransform example in code/addons/docs/template/stories/docspage/source.stories.ts demonstrating async usage
  • Deprecated parameters.docs.source.format parameter in favor of custom transformers in docs/api/doc-blocks/doc-block-source.mdx
  • Updated documentation in code/addons/docs/docs/recipes.md with async transform examples and migration guidance

Copy link

nx-cloud bot commented Jan 30, 2025

View your CI Pipeline Execution ↗ for commit 9b88069.

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

☁️ Nx Cloud last updated this comment at 2025-02-07 14:45:28 UTC

@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Jan 30, 2025

Package Benchmarks

Commit: 9b88069, ran on 7 February 2025 at 14:52:22 UTC

No significant changes detected, all good. 👏

@valentinpalkovic valentinpalkovic changed the title Docs: Support async docs.source.transform Source: Support async parameters.docs.source.transform Jan 31, 2025
@valentinpalkovic valentinpalkovic marked this pull request as ready for review January 31, 2025 10:22
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.

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

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

A few comments.
I also think it deserves a migration note, describing how you can implement the current formatter as an async transfomer.

Probably also needs @shilman's blessing on deprecating docs.source.format in favor of asking users to implement their own if they want to, probably with prettier.

The rationale is that the current format feature is half-baked because it only supports a hard-coded limited set of languages, and it requires a hefty prettier bundled in.

const sourceParameters = (storyContext.parameters.docs?.source || {}) as SourceParameters;
const parameters = storyContext.parameters ?? {};
const { __isArgsStory: isArgsStory } = parameters;
const [transformedCode, setTransformedCode] = useState('Loading...');
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Maybe 'Transforming...' instead of 'Loading...'?
  2. the default could also just be code if we detect that there is no transformer, and 'Transforming...' if there is?
  3. The default could also just always be code, to be replaced by the transformed code afterwards.

Copy link
Contributor Author

@valentinpalkovic valentinpalkovic Jan 31, 2025

Choose a reason for hiding this comment

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

the default could also just be code if we detect that there is no transformer, and 'Transforming...' if there is?

It is a bit of a spaghetti code, but at the bottom of the hook, transformedCode is not used if code is defined. I don't prefer to set the default to code because then you might have a flickering between two codes, and you might not understand why it was changing. Transforming... indicates that the transformation is still in progress and you should wait to interpret the code. transform can not just be used to format code, but also to display something completely different (I think we use a very custom transformer for Angular for example).


Type: `boolean | 'dedent' | BuiltInParserName`

Default: `parameters.docs.source.format` or `true`

Deprecated: Will be removed in Storybook 9. Please use `docs.source.transform` instead and use your own formatter
Copy link
Contributor

Choose a reason for hiding this comment

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

I think an example of how to do this with prettier here would come a long way.

Copy link
Member

Choose a reason for hiding this comment

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

Agree

@valentinpalkovic
Copy link
Contributor Author

Good catches! I'll take care of it.

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.

4 participants