Skip to content

CSF: Refactor CsfFile to support tests and tags#32385

Merged
yannbf merged 2 commits into
yann/test-fn-prototypefrom
yann/fix-test-tags-extraction
Sep 3, 2025
Merged

CSF: Refactor CsfFile to support tests and tags#32385
yannbf merged 2 commits into
yann/test-fn-prototypefrom
yann/fix-test-tags-extraction

Conversation

@yannbf

@yannbf yannbf commented Sep 3, 2025

Copy link
Copy Markdown
Member

Closes #

What I did

This PR refactors the CSF file structure to have a flat list of tests, plus a utility to get story tests.
Additionally, it handles story test tags and fixes an issue where the tests were included in autodocs by default.

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 pull request has been released as version 0.0.0-pr-32385-sha-07e99aef. Try it out in a new sandbox by running npx storybook@0.0.0-pr-32385-sha-07e99aef sandbox or in an existing project with npx storybook@0.0.0-pr-32385-sha-07e99aef upgrade.

More information
Published version 0.0.0-pr-32385-sha-07e99aef
Triggered by @yannbf
Repository storybookjs/storybook
Branch yann/fix-test-tags-extraction
Commit 07e99aef
Datetime Wed Sep 3 12:39:56 UTC 2025 (1756903196)
Workflow run 17433721863

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=32385

Greptile Summary

Updated On: 2025-09-03 11:49:45 UTC

This PR refactors the CsfFile class to improve API design and support test functionality in Storybook's Component Story Format (CSF). The changes introduce a new getStoryTests() method to replace direct access to private _storyTests properties, improving encapsulation and providing a cleaner API surface. Additionally, the PR adds special handling for stories tagged with test-fn by automatically applying a !autodocs override tag to prevent test stories from appearing in documentation.

The refactoring touches three key areas: the CSF file test suite, the story preparation logic in preview-api, and the vitest plugin transformer. The getStoryTests() method now serves as the public interface for accessing story test data, hiding implementation details behind a proper getter method. This change supports Storybook's evolving test infrastructure where stories can function as both visual components and executable tests.

The test-fn tag handling ensures that stories intended for testing frameworks like Vitest are properly excluded from Storybook's documentation while remaining available for test execution. This separation allows developers to write test stories alongside regular stories without cluttering the documentation interface.

PR Description Notes:

  • The PR description is incomplete with an empty "What I did" section and unchecked testing/documentation checklists
  • No issue number is provided in the "Closes #" placeholder

Confidence score: 4/5

  • This PR appears safe to merge with some minor concerns about the hardcoded hack implementation
  • Score reflects clean API improvements but includes a temporary workaround that acknowledges technical debt
  • Pay close attention to the prepareStory.ts file where the hardcoded override logic may need future refinement

@yannbf yannbf self-assigned this Sep 3, 2025
@yannbf yannbf added csf ci:normal Run our default set of CI jobs (choose this for most PRs). addon: vitest labels Sep 3, 2025
@nx-cloud

nx-cloud Bot commented Sep 3, 2025

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 639042c

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

☁️ Nx Cloud last updated this comment at 2025-09-03 16:02:58 UTC

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

* !autodocs so the negation does not get passed through, and therefore we need to do it here.
* Therefore, unfortunately we have to duplicate the logic here.
*/
const overrideTags = storyAnnotations?.tags?.includes('test-fn') ? ['!autodocs'] : [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style: Consider making 'test-fn' a named constant instead of a magic string to improve maintainability

Comment thread code/core/src/preview-api/modules/store/csf/prepareStory.ts Outdated

const hasTests = Object.keys(validStories).some(
(exportName) => parsed._storyTests[exportName]?.length > 0
(exportName) => parsed.getStoryTests(exportName).length > 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

style: Same concern about null safety - ensure getStoryTests() consistently returns an array even when no tests exist.

Suggested change
(exportName) => parsed.getStoryTests(exportName).length > 0
(exportName) => parsed.getStoryTests(exportName)?.length > 0

@yannbf yannbf merged commit 3fa6097 into yann/test-fn-prototype Sep 3, 2025
2 of 4 checks passed
@yannbf yannbf deleted the yann/fix-test-tags-extraction branch September 3, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

addon: vitest ci:normal Run our default set of CI jobs (choose this for most PRs). csf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant