Skip to content

Comments

fix: Update TagOptions to allow undefined defaultFilterSelection and add tests for TagsOptions#33895

Open
unional wants to merge 1 commit intostorybookjs:nextfrom
unional:storybook-config-tags
Open

fix: Update TagOptions to allow undefined defaultFilterSelection and add tests for TagsOptions#33895
unional wants to merge 1 commit intostorybookjs:nextfrom
unional:storybook-config-tags

Conversation

@unional
Copy link
Contributor

@unional unional commented Feb 21, 2026

What I did

Add undefined to the defaultFilterSelection to allow simplier usage:

// before
const config: StorybookConfig = {
  tags: {
    ...(isProduction() ? { unit: { defaultFilterSelection: 'exclude' } } : undefined)
  }
}

// after
const config: StorybookConfig = {
  tags: {
    unit: {
      defaultFilterSelection: isProduction() ? 'exclude' : undefined
    }
  }
}

This is generally related to supporting exactOptionalPropertyTypes.
But instead of fixing all issue which will be a lot more substantial,
this PR just focus on this fix.

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!

Can be validated with mutation test by uncommenting the tsconfig line and observe the error in the test (you may need to restart the TS server to see).

Can't enable that directly as there are other failures.

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

Summary by CodeRabbit

  • Tests

    • Added new test coverage for core configuration scenarios.
  • Chores

    • Updated TypeScript configuration and type definitions.

@dosubot
Copy link

dosubot bot commented Feb 21, 2026

Related Documentation

Checked 0 published document(s) in 1 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

📝 Walkthrough

Walkthrough

Modifies the TagOptions type definition to allow undefined values for the defaultFilterSelection property, introduces a test file validating the behavior of defaultFilterSelection in TagsOptions when NODE_ENV differs, and comments out a TypeScript compiler option in the configuration.

Changes

Cohort / File(s) Summary
Type Definition & Test
code/core/src/types/modules/core-common.ts, code/core/src/types/modules/core-common.test.ts
Updated TagOptions.defaultFilterSelection property to allow undefined in addition to 'include' | 'exclude' literals; added test validating defaultFilterSelection behavior based on NODE_ENV context.
Configuration
code/tsconfig.json
Commented out exactOptionalPropertyTypes TypeScript compiler option without enabling it.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
code/core/src/types/modules/core-common.test.ts (1)

7-19: Consider simplifying the test to directly use undefined.

The test is named "accepts undefined" but the conditional logic means undefined is only assigned when NODE_ENV === 'development'. For a type-level test, directly assigning undefined would be clearer and more deterministic:

♻️ Suggested simplification
     it('accepts undefined', () => {
       const config: StorybookConfig = {
         stories: [],
         framework: '@storybook/react-vite',
         tags: {
           unit: {
-            defaultFilterSelection:
-              process.env['NODE_ENV'] !== 'development' ? 'exclude' : undefined,
+            defaultFilterSelection: undefined,
           },
         },
       };
       expect(config).toBeDefined();
     });

If you want to demonstrate the conditional pattern from the PR description works, consider adding a separate test case:

it('accepts conditional assignment pattern', () => {
  const isDev = process.env['NODE_ENV'] === 'development';
  const config: StorybookConfig = {
    stories: [],
    framework: '@storybook/react-vite',
    tags: {
      unit: {
        defaultFilterSelection: isDev ? undefined : 'exclude',
      },
    },
  };
  expect(config).toBeDefined();
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/types/modules/core-common.test.ts` around lines 7 - 19, The
test named "accepts undefined" should deterministically assign undefined rather
than depending on NODE_ENV; update the test that constructs a StorybookConfig to
set tags.unit.defaultFilterSelection: undefined directly (referencing the
StorybookConfig type and the defaultFilterSelection property) so the type-level
acceptance is clear, and optionally add a separate test that demonstrates the
conditional pattern by using a local isDev boolean and assigning
defaultFilterSelection: isDev ? undefined : 'exclude' to validate both cases.
🤖 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/types/modules/core-common.test.ts`:
- Around line 7-19: The test named "accepts undefined" should deterministically
assign undefined rather than depending on NODE_ENV; update the test that
constructs a StorybookConfig to set tags.unit.defaultFilterSelection: undefined
directly (referencing the StorybookConfig type and the defaultFilterSelection
property) so the type-level acceptance is clear, and optionally add a separate
test that demonstrates the conditional pattern by using a local isDev boolean
and assigning defaultFilterSelection: isDev ? undefined : 'exclude' to validate
both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant