Maintenance: extract shared helpers from new-story-templates#34461
Maintenance: extract shared helpers from new-story-templates#34461mixelburg wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughA new shared helpers module extracts common template data fields into Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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.
🧹 Nitpick comments (1)
code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts (1)
16-19: Intentional divergence fromserializeArgshelper due to different output format.The CSF factory template correctly uses its own args serialization because the output format differs from the CSF3 templates:
- CSF factory needs:
meta.story({ args: {...} })ormeta.story({})- CSF3 needs:
{ args: {...}, }as a property inside an object literalThis is a valid reason not to use
serializeArgs. However, for completeness and to fully realize the PR objective of centralizing args serialization, consider adding a helper for this format as well (e.g.,serializeArgsAsObject). This is optional since the current implementation is correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts` around lines 16 - 19, The current CSF factory template builds argsString inline (variable argsString) instead of using the shared serializeArgs helper because it needs a different shape; to centralize serialization add a new helper (e.g., serializeArgsAsObject) that returns either "{ args: <serializedArgs> }" or "{}" and replace the inline argsString construction with a call to serializeArgsAsObject(data.args), updating the CSF factory template to import and use serializeArgsAsObject while leaving the existing serializeArgs for CSF3 unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/core/src/core-server/utils/new-story-templates/csf-factory-template.ts`:
- Around line 16-19: The current CSF factory template builds argsString inline
(variable argsString) instead of using the shared serializeArgs helper because
it needs a different shape; to centralize serialization add a new helper (e.g.,
serializeArgsAsObject) that returns either "{ args: <serializedArgs> }" or "{}"
and replace the inline argsString construction with a call to
serializeArgsAsObject(data.args), updating the CSF factory template to import
and use serializeArgsAsObject while leaving the existing serializeArgs for CSF3
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97ba4a6d-6330-4c4c-91a9-893c2b040d61
📒 Files selected for processing (4)
code/core/src/core-server/utils/new-story-templates/csf-factory-template.tscode/core/src/core-server/utils/new-story-templates/helpers.tscode/core/src/core-server/utils/new-story-templates/javascript.tscode/core/src/core-server/utils/new-story-templates/typescript.ts
|
Closing due to #34453 (comment) |
Closes #34456
What I did
The three new-story-file template generators (
javascript.ts,typescript.ts,csf-factory-template.ts) contained near-identical component-import resolution and args serialization logic (~18 lines duplicated across all three). Any change to import generation logic had to be applied in three places.Extracted shared helpers into a new
helpers.tsfile:BaseTemplateDatainterface — common fields shared by all three templatesresolveComponentImport(data)— resolvesimportName+importStatement(with trailing semicolon) from template dataserializeArgs(args)— serializes story args toargs: {...},or''when emptyEach template interface now extends
BaseTemplateDataand delegates the shared logic to these helpers.Also fixed a pre-existing inconsistency: the TS template omitted the trailing semicolon from the import statement string and added it in the outer template literal, while the JS and CSF templates included it directly.
resolveComponentImportnow always includes the semicolon so all three templates are consistent.All 7 existing unit tests pass unchanged.
Summary by CodeRabbit