React: Add jsxOnly option to experimentalCodeExamples#33573
Closed
rawvv wants to merge 2 commits into
Closed
Conversation
Contributor
📝 WalkthroughWalkthroughWidened StorybookConfigRaw.features.experimentalCodeExamples from Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Storybook Config
participant Enricher as enrichCsf
participant Generator as getCodeSnippet
participant Output as Component Manifest
Config->>Enricher: experimentalCodeExamples (boolean | { jsxOnly })
Enricher->>Generator: getCodeSnippet(csf, storyName, componentName, { jsxOnly })
Generator->>Generator: if jsxOnly -> extractJsx / return ExpressionStatement
Generator->>Output: emit snippet (JSX expression or wrapper function)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/renderers/react/src/componentManifest/generateCodeSnippet.ts (1)
183-277: Guard jsxOnly against dropping local statements.
extractJsxFromFunction(Line 191–213) and the jsxOnly return path (Line 257–276) can emit bare JSX even when the function body contains local declarations. This drops required statements, producing snippets that reference undefined locals and are not copy‑pasteable. Consider only emitting JSX when the body is a single return statement; otherwise keep the wrapper or surface an error.Proposed guard to avoid lossy extraction
- const extractJsxFromFunction = ( - fn: t.ArrowFunctionExpression | t.FunctionExpression | t.FunctionDeclaration - ): t.JSXElement | t.JSXFragment | null => { + const extractJsxFromFunction = ( + fn: t.ArrowFunctionExpression | t.FunctionExpression | t.FunctionDeclaration + ): t.JSXElement | t.JSXFragment | null => { + const getReturnJsx = (stmt: t.Statement) => + t.isReturnStatement(stmt) && + stmt.argument && + (t.isJSXElement(stmt.argument) || t.isJSXFragment(stmt.argument)) + ? stmt.argument + : null; + if (t.isArrowFunctionExpression(fn)) { if (t.isJSXElement(fn.body) || t.isJSXFragment(fn.body)) { return fn.body; } - if (t.isBlockStatement(fn.body)) { - for (const stmt of fn.body.body) { - if (t.isReturnStatement(stmt) && stmt.argument) { - if (t.isJSXElement(stmt.argument) || t.isJSXFragment(stmt.argument)) { - return stmt.argument; - } - } - } + if (t.isBlockStatement(fn.body) && fn.body.body.length === 1) { + const ret = getReturnJsx(fn.body.body[0]); + if (ret) return ret; } } if ( (t.isFunctionDeclaration(fn) || t.isFunctionExpression(fn)) && t.isBlockStatement(fn.body) ) { - for (const stmt of fn.body.body) { - if (t.isReturnStatement(stmt) && stmt.argument) { - if (t.isJSXElement(stmt.argument) || t.isJSXFragment(stmt.argument)) { - return stmt.argument; - } - } - } + if (fn.body.body.length === 1) { + const ret = getReturnJsx(fn.body.body[0]); + if (ret) return ret; + } } return null; }; @@ - if (changed) { - if (jsxOnly && transformedJsx) { - return t.expressionStatement(transformedJsx); - } + if (changed) { + const singleReturn = + newBody.length === 1 && + t.isReturnStatement(newBody[0]) && + (t.isJSXElement(newBody[0].argument) || t.isJSXFragment(newBody[0].argument)); + if (jsxOnly && transformedJsx && singleReturn) { + return t.expressionStatement(transformedJsx); + } return t.isFunctionDeclaration(fn) ? t.functionDeclaration(fn.id, [], t.blockStatement(newBody), fn.generator, fn.async) : t.variableDeclaration('const', [ t.variableDeclarator( t.identifier(storyName), t.arrowFunctionExpression([], t.blockStatement(newBody), fn.async) ), ]); }
🤖 Fix all issues with AI agents
In `@code/core/src/types/modules/core-common.ts`:
- Around line 505-529: The JSDoc examples for the experimentalCodeExamples
option use invalid TypeScript object syntax (they use semicolons instead of
commas); update the two example object literals in the comment (the ones showing
features: { experimentalCodeExamples: true; } and features: {
experimentalCodeExamples: { jsxOnly: true; } }) to use commas between properties
(e.g., features: { experimentalCodeExamples: true, } and features: {
experimentalCodeExamples: { jsxOnly: true, } }) so the examples are valid and
copy-pastable for the experimentalCodeExamples option.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 181 KB | 🚨 +181 KB 🚨 |
| Dependency size | 0 B | 2.98 MB | 🚨 +2.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-docs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 18 | 🚨 +18 🚨 |
| Self size | 0 B | 1.64 MB | 🚨 +1.64 MB 🚨 |
| Dependency size | 0 B | 9.25 MB | 🚨 +9.25 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-links
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 14 KB | 🚨 +14 KB 🚨 |
| Dependency size | 0 B | 5 KB | 🚨 +5 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-onboarding
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 331 KB | 🚨 +331 KB 🚨 |
| Dependency size | 0 B | 673 B | 🚨 +673 B 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook-addon-pseudo-states
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 21 KB | 🚨 +21 KB 🚨 |
| Dependency size | 0 B | 692 B | 🚨 +692 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-themes
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/addon-vitest
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 377 KB | 🚨 +377 KB 🚨 |
| Dependency size | 0 B | 338 KB | 🚨 +338 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 17 | 🚨 +17 🚨 |
| Self size | 0 B | 125 KB | 🚨 +125 KB 🚨 |
| Dependency size | 0 B | 2.00 MB | 🚨 +2.00 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 75 KB | 🚨 +75 KB 🚨 |
| Dependency size | 0 B | 32.26 MB | 🚨 +32.26 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 49 | 🚨 +49 🚨 |
| Self size | 0 B | 20.19 MB | 🚨 +20.19 MB 🚨 |
| Dependency size | 0 B | 16.52 MB | 🚨 +16.52 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 192 | 🚨 +192 🚨 |
| Self size | 0 B | 118 KB | 🚨 +118 KB 🚨 |
| Dependency size | 0 B | 30.48 MB | 🚨 +30.48 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 196 | 🚨 +196 🚨 |
| Self size | 0 B | 15 KB | 🚨 +15 KB 🚨 |
| Dependency size | 0 B | 28.98 MB | 🚨 +28.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 22 KB | 🚨 +22 KB 🚨 |
| Dependency size | 0 B | 2.16 MB | 🚨 +2.16 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 538 | 🚨 +538 🚨 |
| Self size | 0 B | 646 KB | 🚨 +646 KB 🚨 |
| Dependency size | 0 B | 59.24 MB | 🚨 +59.24 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 127 | 🚨 +127 🚨 |
| Self size | 0 B | 1.12 MB | 🚨 +1.12 MB 🚨 |
| Dependency size | 0 B | 21.82 MB | 🚨 +21.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 13 KB | 🚨 +13 KB 🚨 |
| Dependency size | 0 B | 2.15 MB | 🚨 +2.15 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 159 | 🚨 +159 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 23.12 MB | 🚨 +23.12 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 117 | 🚨 +117 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 19.61 MB | 🚨 +19.61 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 278 | 🚨 +278 🚨 |
| Self size | 0 B | 24 KB | 🚨 +24 KB 🚨 |
| Dependency size | 0 B | 44.14 MB | 🚨 +44.14 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 204 | 🚨 +204 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 33.51 MB | 🚨 +33.51 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 24 | 🚨 +24 🚨 |
| Self size | 0 B | 56 KB | 🚨 +56 KB 🚨 |
| Dependency size | 0 B | 26.82 MB | 🚨 +26.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 25 | 🚨 +25 🚨 |
| Self size | 0 B | 56 KB | 🚨 +56 KB 🚨 |
| Dependency size | 0 B | 26.88 MB | 🚨 +26.88 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 114 | 🚨 +114 🚨 |
| Self size | 0 B | 35 KB | 🚨 +35 KB 🚨 |
| Dependency size | 0 B | 43.96 MB | 🚨 +43.96 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 21 | 🚨 +21 🚨 |
| Self size | 0 B | 19 KB | 🚨 +19 KB 🚨 |
| Dependency size | 0 B | 2.19 MB | 🚨 +2.19 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 183 | 🚨 +183 🚨 |
| Self size | 0 B | 775 KB | 🚨 +775 KB 🚨 |
| Dependency size | 0 B | 67.35 MB | 🚨 +67.35 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 176 | 🚨 +176 🚨 |
| Self size | 0 B | 30 KB | 🚨 +30 KB 🚨 |
| Dependency size | 0 B | 65.92 MB | 🚨 +65.92 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/core-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 1 | 🚨 +1 🚨 |
| Self size | 0 B | 11 KB | 🚨 +11 KB 🚨 |
| Dependency size | 0 B | 28 KB | 🚨 +28 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 50 | 🚨 +50 🚨 |
| Self size | 0 B | 999 KB | 🚨 +999 KB 🚨 |
| Dependency size | 0 B | 36.70 MB | 🚨 +36.70 MB 🚨 |
| Bundle Size Analyzer | node | node |
@storybook/csf-plugin
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 9 | 🚨 +9 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.26 MB | 🚨 +1.26 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
eslint-plugin-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 20 | 🚨 +20 🚨 |
| Self size | 0 B | 131 KB | 🚨 +131 KB 🚨 |
| Dependency size | 0 B | 2.82 MB | 🚨 +2.82 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 791 B | 🚨 +791 B 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-create-react-app
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 68 | 🚨 +68 🚨 |
| Self size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Dependency size | 0 B | 5.98 MB | 🚨 +5.98 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 170 | 🚨 +170 🚨 |
| Self size | 0 B | 18 KB | 🚨 +18 KB 🚨 |
| Dependency size | 0 B | 31.29 MB | 🚨 +31.29 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-server-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 10 | 🚨 +10 🚨 |
| Self size | 0 B | 7 KB | 🚨 +7 KB 🚨 |
| Dependency size | 0 B | 1.20 MB | 🚨 +1.20 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 29 KB | 🚨 +29 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 16 KB | 🚨 +16 KB 🚨 |
| Dependency size | 0 B | 32 KB | 🚨 +32 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 57 | 🚨 +57 🚨 |
| Self size | 0 B | 719 KB | 🚨 +719 KB 🚨 |
| Dependency size | 0 B | 12.95 MB | 🚨 +12.95 MB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 8 KB | 🚨 +8 KB 🚨 |
| Dependency size | 0 B | 716 KB | 🚨 +716 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 2 | 🚨 +2 🚨 |
| Self size | 0 B | 45 KB | 🚨 +45 KB 🚨 |
| Dependency size | 0 B | 230 KB | 🚨 +230 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 55 KB | 🚨 +55 KB 🚨 |
| Dependency size | 0 B | 211 KB | 🚨 +211 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 3 | 🚨 +3 🚨 |
| Self size | 0 B | 41 KB | 🚨 +41 KB 🚨 |
| Dependency size | 0 B | 47 KB | 🚨 +47 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
Author
|
Hi @kasperpeulen @valentinpalkovic, The CI unit-test failures appear to be unrelated to my changes. I verified this by running the same failing test ( Error: [baseline-browser-mapping] The data in this module is over two months old. This seems to be a dependency issue with Should I do anything to address this, or will it be handled separately? Thanks! |
valentinpalkovic
moved this from Empathy Queue (prioritized)
to In Progress
in Core Team Projects
Contributor
|
Thanks for the contribution and the thoguhts you put into both this and the issue! I personally don't think it's good idea for this to be configurable. As far as I can see, your reasoning in the original issue is:
Let me respond to these individually:
I've seen a lot of power-users over the years adding their own wrapper using the I understand this can be a transition to get used to, historically Storybook has shown JSX-only for many years, but I honestly think it's an improvement here. One thing I could get behind, is stealing the "Expanded code" toggle that MUI has. I think that's pretty clever, allowing the reader to toggle between the modes depending on their needs. It would probably require that we both generate "expanded" and "collapsed" code, but collapsed should be derivable from the expanded, so I think that's doable. |
Member
|
@JReinhold Great response. I also don't like the idea of making this configurable and I like the proposal of stealing MUI's |
Contributor
|
@ho8ae Thank you for your contribution and kickstarting a valuable discussion with your work. I am closing this PR because we are envisioning a different solution for the problem. Feel all free to move the discussion into the existing issue or into a new GitHub Issue/Discussion or just continue here and create a new PR with the accepted solution. |
github-project-automation
Bot
moved this from In Progress
to Done
in Core Team Projects
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #33473
What I did
Added a
jsxOnlyoption toexperimentalCodeExamplesto output clean JSX without wrapper functions.Background
Issue #33473 reports that compound components lose dot notation in "Show Code".
type: 'dynamic'):<Card.Header>→<Header>❌<Card.Header>is preserved ✅ but wrapped in functionThis PR adds flexibility to
experimentalCodeExamplesby allowing JSX-only output.Usage
Output comparison
experimentalCodeExamples: trueconst Story = () => <Card.Header />experimentalCodeExamples: { jsxOnly: true }<Card.Header />Why this helps with #33473
Users experiencing the compound component issue can now:
This provides a better DX than the current workaround of manually setting displayName.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
- Default: Shows const Default = () => <Card.Header>...
- jsxOnly: true: Shows <Card.Header>... only
Documentation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.