Addon-docs: Add transformCode option to apply transform to code prop#34519
Addon-docs: Add transformCode option to apply transform to code prop#34519adam-sajko wants to merge 1 commit intostorybookjs:nextfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 154-160: The hook useTransformCode must be called unconditionally
to keep hook order stable: always invoke useTransformCode (passing props.code
and the transformer value derived from props.transform ??
sourceParameters.transform or undefined) and then conditionally use its returned
transformed code only when transformer and props.code are present; update the
code variable assignment (currently using transformer ? useTransformCode(...) :
props.code) so useTransformCode is called every render and its result is
selected conditionally, leaving transformCode, transformer, props.code,
props.transform, sourceParameters.transform, storyContext, argsForSource, and
code as the relevant symbols to locate and update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8076e744-a2b1-4060-8339-ea6ac38ba203
📒 Files selected for processing (2)
code/addons/docs/src/blocks/blocks/Source.tsxdocs/api/doc-blocks/doc-block-source.mdx
d79b66b to
14aa612
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/docs/src/blocks/blocks/Source.tsx (1)
139-165:⚠️ Potential issue | 🟠 Major
parameters.docs.source.codestill skipstransformCode.This only fixes the
props.codebranch.useCode()still returnssourceParameters.codeunchanged on Line 93-Line 95, soparameters.docs.source.transformCode = truewill not transform story/globalparameters.docs.source.code. That leaves one of the advertised entry points for the feature non-functional.🔧 Suggested direction
const useCode = ({ snippet, storyContext, typeFromProps, transformFromProps, + transformCodeFromProps, }: { snippet: string; storyContext: ReturnType<DocsContextProps['getStoryContext']>; typeFromProps: SourceType; transformFromProps?: SourceProps['transform']; + transformCodeFromProps?: SourceProps['transformCode']; }): string => { const parameters = storyContext.parameters ?? {}; const { __isArgsStory: isArgsStory } = parameters; const sourceParameters = (parameters.docs?.source || {}) as SourceParameters; @@ const code = useSnippet ? snippet : sourceParameters.originalSource || ''; const transformer = transformFromProps ?? sourceParameters.transform; + const providedCode = sourceParameters.code; + const shouldTransformProvidedCode = + transformCodeFromProps ?? sourceParameters.transformCode ?? false; + const providedCodeTransformer = shouldTransformProvidedCode + ? (transformFromProps ?? sourceParameters.transform) + : undefined; const transformedCode = transformer ? useTransformCode(code, transformer, storyContext) : code; + const transformedProvidedCode = useTransformCode( + providedCode ?? '', + providedCodeTransformer ?? ((c) => c), + storyContext + ); - if (sourceParameters.code !== undefined) { - return sourceParameters.code; + if (providedCode !== undefined) { + return providedCodeTransformer ? transformedProvidedCode : providedCode; }const transformedCode = useCode({ snippet: source ? source.code : '', storyContext: { ...storyContext, args: argsForSource }, typeFromProps: props.type as SourceType, transformFromProps: props.transform, + transformCodeFromProps: props.transformCode, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Source.tsx` around lines 139 - 165, The parameters.docs.source.code path isn't being passed through the transformCode logic; update the code that reads sourceParameters.code (e.g., inside useCode or wherever sourceParameters.code is returned) to mirror the props.code branch: detect shouldTransformCode (from props.transformCode ?? sourceParameters.transformCode), pick the transformer (props.transform ?? sourceParameters.transform), run useTransformCode on sourceParameters.code (or apply the transformer) and return the transformed result (use transformedCodeProp-like value) instead of returning sourceParameters.code unchanged; ensure you reference sourceParameters, shouldTransformCode, codeTransformer, useTransformCode, and transformedCodeProp so the global/story-level parameters.docs.source.code honors transformCode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 139-165: The parameters.docs.source.code path isn't being passed
through the transformCode logic; update the code that reads
sourceParameters.code (e.g., inside useCode or wherever sourceParameters.code is
returned) to mirror the props.code branch: detect shouldTransformCode (from
props.transformCode ?? sourceParameters.transformCode), pick the transformer
(props.transform ?? sourceParameters.transform), run useTransformCode on
sourceParameters.code (or apply the transformer) and return the transformed
result (use transformedCodeProp-like value) instead of returning
sourceParameters.code unchanged; ensure you reference sourceParameters,
shouldTransformCode, codeTransformer, useTransformCode, and transformedCodeProp
so the global/story-level parameters.docs.source.code honors transformCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cb00aab-82b4-4cf2-95b1-0169b41d9b57
📒 Files selected for processing (2)
code/addons/docs/src/blocks/blocks/Source.tsxdocs/api/doc-blocks/doc-block-source.mdx
730ed19 to
73e927f
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/addons/docs/src/blocks/blocks/Source.tsx (1)
135-163: Add tests for the newtransformCodebranch matrix.Lines 136-163 add multiple precedence paths (
props.codevs parameter code,transformCodeprop vs parameter, and transform origin). Please add focused unit coverage for these combinations to prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Source.tsx` around lines 135 - 163, Add unit tests for the Source component covering the transformCode precedence matrix: create focused tests that exercise (1) directCode provided via props vs code via sourceParameters, (2) transformCode true/false/undefined from props vs sourceParameters, and (3) transform function coming from props vs sourceParameters. For each combination assert whether useTransformCode (transformedDirectCode) is used or the raw code is returned, and also verify format selection logic when props.code is set vs when it falls back to format/ source?.format; reference the variables/source operators in the component (sourceParameters, directCode, shouldTransformCode, codeTransformer, transformedDirectCode, useTransformCode, props.code, props.transformCode) to guide test setup and expected outcomes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 138-145: The code may call user-provided transformers even when
directCode is empty; change the logic so the transformer is only selected when
shouldTransformCode is true AND directCode is non-empty. Concretely update the
computation of codeTransformer (used by useTransformCode for
transformedDirectCode) to require directCode (e.g., codeTransformer =
shouldTransformCode && directCode ? (props.transform ??
sourceParameters.transform) : undefined), and make the same guard for the other
transformer usage (the highlighted/other transformed code block) so
useTransformCode receives an identity/undefined transformer when there's no
direct code to avoid running user transforms unnecessarily.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 135-163: Add unit tests for the Source component covering the
transformCode precedence matrix: create focused tests that exercise (1)
directCode provided via props vs code via sourceParameters, (2) transformCode
true/false/undefined from props vs sourceParameters, and (3) transform function
coming from props vs sourceParameters. For each combination assert whether
useTransformCode (transformedDirectCode) is used or the raw code is returned,
and also verify format selection logic when props.code is set vs when it falls
back to format/ source?.format; reference the variables/source operators in the
component (sourceParameters, directCode, shouldTransformCode, codeTransformer,
transformedDirectCode, useTransformCode, props.code, props.transformCode) to
guide test setup and expected outcomes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d008f6d-1aaf-4c33-b4de-75fea958c75a
📒 Files selected for processing (2)
code/addons/docs/src/blocks/blocks/Source.tsxdocs/api/doc-blocks/doc-block-source.mdx
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
83abec2 to
188f19a
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/addons/docs/src/blocks/blocks/Source.tsx (1)
167-174:⚠️ Potential issue | 🟡 MinorMinor:
formatfallback for thesourceParameters.codebranch skipssourceParameters.format.When
directCodecomes fromsourceParameters.code(notprops.code), Line 170 falls back asformat ?? source?.format ?? trueand never consultssourceParameters.format, even thoughlanguageanddarkabove do consultsourceParameters.*. InsideSourceWithCodethesourceContextisEMPTY_SOURCE_CONTEXT, sosource?.formatwill always beundefinedon this path and the final value collapses totruewheneverprops.formatis unset — effectively ignoring any user-providedparameters.docs.source.formaton the parameter-code path.If that's the intended semantics, fine to ignore; otherwise consider:
🔧 Suggested tweak
- format: props.code ? format : (format ?? source?.format ?? true), + format: props.code + ? format + : (format ?? sourceParameters.format ?? source?.format ?? true),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Source.tsx` around lines 167 - 174, The format fallback for the hasDirectCode branch ignores sourceParameters.format: update the return value in the hasDirectCode block so format resolves in order props.format, format (local var), sourceParameters.format, source?.format, true; locate the hasDirectCode branch in Source.tsx (where code: codeTransformer ? transformedDirectCode : directCode is set) and include sourceParameters.format in the fallback chain (similar to how language and dark consult sourceParameters), ensuring this also works when SourceWithCode uses EMPTY_SOURCE_CONTEXT.
🧹 Nitpick comments (2)
code/addons/docs/src/blocks/blocks/Source.tsx (2)
205-220: Optional:SourceImplduplicates story resolution already performed inuseSourceProps.
SourceImplcallsdocsContext.resolveOf/storyById()insideuseMemoonly to pick a subcomponent, and thenuseSourceProps(inside either branch) resolves the same story again. Cost is bounded byuseMemoso this is fine functionally, but the two copies of the "find the story, readparameters.docs.source.code" logic can drift. Consider extracting a small helper (e.g.resolveDirectCode(props, docsContext)) and using it both to route inSourceImpland insideuseSourceProps, so there is a single source of truth for "is there direct code?".Non-blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Source.tsx` around lines 205 - 220, SourceImpl duplicates the logic that finds a story and checks parameters.docs.source.code (it calls docsContext.resolveOf/docsContext.storyById inside a useMemo) while useSourceProps repeats that resolution; extract that shared logic into a small helper (e.g., resolveDirectCode(props, docsContext)) that returns a boolean (or the resolved code/source info) and then use that helper both in SourceImpl to decide between SourceWithCode and SourceWithStorySnippet and inside useSourceProps to determine direct code, keeping unique symbols: SourceImpl, useSourceProps, docsContext.resolveOf, docsContext.storyById, SourceWithCode, and SourceWithStorySnippet unchanged.
25-26: No tests cover the newtransformCodebranches.The PR description notes no automated tests were added, but this change introduces several non-trivial branches that are easy to regress:
props.transformCodevssourceParameters.transformCodeprecedence and default (false).transformis ignored whencodeis provided andtransformCodeis unset (the behavior the docs now guarantees).transformis applied whentransformCodeistrue, for bothprops.codeandsourceParameters.code.- Routing in
SourceImplwhen onlysourceParameters.codeis set (previously handled by the story-snippet path).Happy to draft unit tests for
useSourcePropscovering these four cases (including thetransformCode=true+sourceParameters.codecombination) if you'd like — just say the word.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/addons/docs/src/blocks/blocks/Source.tsx` around lines 25 - 26, Add unit tests for useSourceProps and SourceImpl covering the new transformCode branches: assert precedence (props.transformCode overrides sourceParameters.transformCode, default false), verify that when transformCode is unset transform is ignored if props.code is provided, verify transform is applied when transformCode is true for both props.code and sourceParameters.code, and include the routing case where only sourceParameters.code is set (matching previous story-snippet path). Target tests at the functions/components named useSourceProps and SourceImpl and exercise combinations of props.transformCode, sourceParameters.transformCode, props.code, sourceParameters.code, and a mock transform to validate expected outputs and routing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 167-174: The format fallback for the hasDirectCode branch ignores
sourceParameters.format: update the return value in the hasDirectCode block so
format resolves in order props.format, format (local var),
sourceParameters.format, source?.format, true; locate the hasDirectCode branch
in Source.tsx (where code: codeTransformer ? transformedDirectCode : directCode
is set) and include sourceParameters.format in the fallback chain (similar to
how language and dark consult sourceParameters), ensuring this also works when
SourceWithCode uses EMPTY_SOURCE_CONTEXT.
---
Nitpick comments:
In `@code/addons/docs/src/blocks/blocks/Source.tsx`:
- Around line 205-220: SourceImpl duplicates the logic that finds a story and
checks parameters.docs.source.code (it calls
docsContext.resolveOf/docsContext.storyById inside a useMemo) while
useSourceProps repeats that resolution; extract that shared logic into a small
helper (e.g., resolveDirectCode(props, docsContext)) that returns a boolean (or
the resolved code/source info) and then use that helper both in SourceImpl to
decide between SourceWithCode and SourceWithStorySnippet and inside
useSourceProps to determine direct code, keeping unique symbols: SourceImpl,
useSourceProps, docsContext.resolveOf, docsContext.storyById, SourceWithCode,
and SourceWithStorySnippet unchanged.
- Around line 25-26: Add unit tests for useSourceProps and SourceImpl covering
the new transformCode branches: assert precedence (props.transformCode overrides
sourceParameters.transformCode, default false), verify that when transformCode
is unset transform is ignored if props.code is provided, verify transform is
applied when transformCode is true for both props.code and
sourceParameters.code, and include the routing case where only
sourceParameters.code is set (matching previous story-snippet path). Target
tests at the functions/components named useSourceProps and SourceImpl and
exercise combinations of props.transformCode, sourceParameters.transformCode,
props.code, sourceParameters.code, and a mock transform to validate expected
outputs and routing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d000cb5-00a1-4e6a-ae3e-e89b01780003
📒 Files selected for processing (2)
code/addons/docs/src/blocks/blocks/Source.tsxdocs/api/doc-blocks/doc-block-source.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/api/doc-blocks/doc-block-source.mdx
0d0dea4 to
81f3666
Compare
|
@coderabbitai review |
|
Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit. |
81f3666 to
3548a89
Compare
|
Ready for review. Cleaned up the Source block internals while I was in there - pulled out a |
`transform` is currently ignored when `code` is provided (via prop or parameter). This makes sense as a default, but sometimes you want `transform` applied to `code`. This PR adds a `transformCode` boolean (defaults to `false`) to opt in. Works as a prop, story parameter, or globally in `preview.ts`. Also fixes `format` fallback to respect `sourceParameters.format` when `directCode` comes from `sourceParameters.code`, and cleans up `SourceImpl` to follow the same pattern as `useSourceProps`.
3548a89 to
e635a3d
Compare
transformis currently ignored whencodeis provided (via prop or parameter). This makes sense as a default, you don't want a global Prettier transform reformatting your hand-written snippets.But sometimes you do want
transformapplied tocode. This PR adds atransformCodeboolean (defaults tofalse) to opt in.Works as a prop, story parameter, or globally in
preview.ts:What I did
transformCode?: booleantoSourceParameterstrue,transformruns on thecodeprop before renderingtransformCodesection, clarifiedtransform/codeinteractionChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template react-vite/default-ts<Source code="..." transform={fn} />— should render raw code (unchanged behavior)<Source code="..." transform={fn} transformCode />— should apply transformparameters.docs.source.transformCode: trueinpreview.ts— should apply globallyMy Sandbox
Documentation
MIGRATION.MD
Summary by CodeRabbit
New Features
Documentation