Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Sep 24, 2025

Closes #

What I did

While QAing I noticed that the csf factories codemod did not account for files using satisfies operator, so this PR fixes that

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>

Summary by CodeRabbit

  • Refactor

    • Consolidated internal logic for locating preview declarations to reduce duplication. No functional changes expected.
  • Tests

    • Added test coverage ensuring previews are wrapped with definePreview for const-defined previews with type annotations, mixed annotations, and default const exports.
    • Renamed an existing test for clarity and added inline snapshot assertions to confirm consistent output.
  • Chores

    • No user-facing changes; behavior remains the same while improving reliability through expanded tests.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

  • Added internal helper findDeclarationNodeIndex in configToCsfFactory to locate variable declarations (including those with TS type annotations) that initialize to object expressions.
  • Replaced duplicated inline findIndex logic in two branches handling default exports with calls to the new helper.
  • Updated tests in config-to-csf-factory.test.ts: renamed one test and added two new tests to verify definePreview wrapping across const-defined previews with type annotations, mixed annotations, and default const exports.
  • No exported/public API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer Code
  participant Codemod as configToCsfFactory
  participant AST as AST Parser/Traversal
  participant Helper as findDeclarationNodeIndex
  participant Output as Transformed Code

  Dev->>Codemod: Provide source file
  Codemod->>AST: Parse to AST
  Codemod->>Helper: Find declaration index (by name)
  note over Helper: Unwrap TS type annotations<br/>Ensure init is ObjectExpression
  Helper-->>Codemod: Declaration index (or -1)
  alt Declaration found
    Codemod->>AST: Adjust/remove original declaration
  else Not found
    Codemod->>AST: Keep existing structure
  end
  Codemod->>Output: Wrap preview with definePreview (as needed)
  Output-->>Dev: Emit transformed code
Loading

Suggested labels

typescript

Suggested reviewers

  • valentinpalkovic
  • ndelangen

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely states the primary change: enhancing config-to-csf-factory to handle TypeScript "type wrappers" (e.g., the satisfies operator), and it matches the PR objective and the code/tests that add handling for type-annotated declarations. It is specific, focused, and free of noise or vague wording, so a reviewer can understand the main intent at a glance.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch yann/csf-factories-codemod-more-ts-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b9ff97 and 6860988.

📒 Files selected for processing (2)
  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts (2 hunks)
  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
code/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx

Files:

  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together

Files:

  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors

Applied to files:

  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks

Applied to files:

  • code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Danger JS
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (6)
code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.ts (3)

35-52: Excellent refactoring to handle TypeScript type wrappers.

The new findDeclarationNodeIndex helper properly handles the TypeScript satisfies operator introduced in TypeScript 4.9, which allows type checking without losing type inference. The logic correctly unwraps both TSAsExpression and TSSatisfiesExpression to access the underlying ObjectExpression, addressing the PR objective of supporting type wrappers.


82-82: Clean code improvement through DRY principle.

The replacement of inline findIndex logic with the helper function eliminates code duplication and improves maintainability. The helper correctly preserves the original behavior while making the codebase more readable.


125-125: Consistent application of the helper function.

The second occurrence of the duplicated findIndex logic is appropriately replaced with the same helper function, ensuring consistent behavior across both "export default identifier" branches.

code/lib/cli-storybook/src/codemod/helpers/config-to-csf-factory.test.ts (3)

245-245: Test description clarifies scope.

The renamed test title "should wrap definePreview for mixed annotations and default export" better describes what the test verifies compared to the generic "should work" title.


263-290: Comprehensive test coverage for satisfies operator support.

This test validates that the codemod correctly handles const-defined previews with TypeScript satisfies type annotations, ensuring the new findDeclarationNodeIndex helper works as intended. The test covers the key scenario mentioned in the PR objectives where satisfies type wrappers were not previously supported.


292-318: Thorough test for mixed export scenarios.

This test validates the combination of named exports (export const decorators = []) with const-defined default exports using satisfies annotations, ensuring comprehensive coverage of the enhanced type wrapper support.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Sep 24, 2025

View your CI Pipeline Execution ↗ for commit bc4185b

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

☁️ Nx Cloud last updated this comment at 2025-10-01 10:13:30 UTC

@storybook-app-bot
Copy link

storybook-app-bot bot commented Sep 24, 2025

Package Benchmarks

Commit: bc4185b, ran on 1 October 2025 at 09:59:42 UTC

The following packages have significant changes to their size or dependencies:

@storybook/nextjs

Before After Difference
Dependency count 532 532 0
Self size 950 KB 950 KB 0 B
Dependency size 58.52 MB 58.47 MB 🎉 -46 KB 🎉
Bundle Size Analyzer Link Link

@github-actions github-actions bot added the Stale label Oct 13, 2025
@valentinpalkovic valentinpalkovic merged commit e6663f2 into next Oct 13, 2025
54 of 56 checks passed
@valentinpalkovic valentinpalkovic deleted the yann/csf-factories-codemod-more-ts-support branch October 13, 2025 13:51
@github-actions github-actions bot mentioned this pull request Oct 14, 2025
13 tasks
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