CSF-Factories: Skip non-factory exports instead of throwing error by kasperpeulen · Pull Request #33550 · storybookjs/storybook

kasperpeulen · 2026-01-15T13:34:18Z

What I did

Previously, CSF factory files would throw a MixedFactoryError if any named export was not a factory story. This caused issues when files had helper exports or utility functions alongside stories.

Now, non-factory exports in factory files are simply skipped during indexing. This means:

Helper exports no longer need excludeStories to avoid errors
excludeStories/includeStories still work for factory stories
Only actual factory stories (meta.story() / meta.extend()) are indexed

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

Run yarn task --task sandbox --start-from auto --template react-vite/default-ts

Add non-factory exports to a story file:

// In Button.stories.ts, add at the end:
export const mockData = { foo: 'bar' };
export const helperFunction = () => console.log('helper');

Run yarn storybook
Before fix: Would fail with CSF: expected factory story error
After fix: Storybook starts successfully, only factory stories are indexed

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-33550-sha-bd642744. Try it out in a new sandbox by running npx storybook@0.0.0-pr-33550-sha-bd642744 sandbox or in an existing project with npx storybook@0.0.0-pr-33550-sha-bd642744 upgrade.

More information


Published version	`0.0.0-pr-33550-sha-bd642744`
Triggered by	@kasperpeulen
Repository	storybookjs/storybook
Branch	`kasper/fix-csf-factories-exclude-stories`
Commit	`bd642744`
Datetime	Thu Jan 15 13:53:17 UTC 2026 (`1768485197`)
Workflow run	21033721817

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 publish.yml --field pr=33550

coderabbitai · 2026-01-15T13:37:16Z

📝 Walkthrough

Walkthrough

CSF parsing now skips non-factory exports when a file contains factory stories instead of throwing MixedFactoryError. Tests updated to assert non-factory exports are ignored and that excludeStories continues to exclude specified factory exports.

Changes

Cohort / File(s)	Change Summary
CsfFile implementation `code/core/src/csf-tools/CsfFile.ts`	Reworked ExportNamedDeclaration handling: defer story registration until factory/non-factory validation, detect factory stories by unwrapping TS expressions, skip non-factory exports early in factory files (no exception), preserve annotation handling and __namedExportsOrder logic.
CsfFile tests `code/core/src/csf-tools/CsfFile.test.ts`	Replaced MixedFactoryError expectation with assertions that non-factory exports are ignored; added test ensuring `excludeStories` excludes specified factory exports.

Sequence Diagram(s)

(omitted — changes are internal to CSF parsing and do not introduce a multi-component sequential flow that requires diagramming)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

React: Fix several CSF factory bugs #33354 — Related changes to processing factory stories and skipping non-factory exports in CSF parsing.
Core: Fix built-in tags filtering #32526 — Edits touching the same csf-tools parsing logic and tests for export handling.
CLI: Skip vitest transform for CSF Factories in a11y-addon-test automigration #31941 — Related to CSF factory detection logic used by automigration/isCsfFactoryPreview.

✨ Finishing touches

📝 Generate docstrings

🧹 Recent nitpick comments

code/core/src/csf-tools/CsfFile.ts (1)
586-595: Consider defensive check for empty arguments.

If meta.story() is called without arguments, storyNode.arguments[0] would be undefined. The code handles this gracefully (the t.isObjectExpression check at line 633 returns false for undefined), but an explicit guard could make the intent clearer.
Suggested defensive check
 if (
   t.isCallExpression(storyNode) &&
   t.isMemberExpression(storyNode.callee) &&
   t.isIdentifier(storyNode.callee.property) &&
   (storyNode.callee.property.name === 'story' ||
     storyNode.callee.property.name === 'extend')
 ) {
   storyIsFactory = true;
-  storyNode = storyNode.arguments[0];
+  storyNode = storyNode.arguments[0] ?? t.objectExpression([]);
 }
code/core/src/csf-tools/CsfFile.test.ts (1)

3005-3032: Consider moving success-case tests out of 'errors' block.

The "skips non-factory exports in factory files" test validates successful skip behavior rather than error handling. For consistency with the surrounding test structure, these tests might fit better in the describe('normal', ...) block at line 2545 or a new dedicated block.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02dfa84 and eb657fd.

📒 Files selected for processing (2)

code/core/src/csf-tools/CsfFile.test.ts
code/core/src/csf-tools/CsfFile.ts

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

code/core/src/csf-tools/CsfFile.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 the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

code/core/src/csf-tools/CsfFile.test.ts

**/*.{js,jsx,ts,tsx,json,md,html,css,scss}