Skip to content

Core: Compose core annotations before preview beforeAll - #35323

Merged
ndelangen merged 3 commits into
nextfrom
fix/preview-core-annotations
Jul 1, 2026
Merged

ndelangen merged 3 commits into
nextfrom
fix/preview-core-annotations

Conversation

@JReinhold

@JReinhold JReinhold commented Jun 29, 2026 •

Copy link
Copy Markdown
Contributor

Closes #

What I did

Plain (CSF1–3) previews don't fold Storybook's core annotations into getProjectAnnotations(), so PreviewWeb ran beforeAll against annotations that lacked them. Core beforeAll hooks (registering built-in open services like core/docgen and core/story-docs in the preview) never ran — e.g. autodocs <Controls /> in MDX failed with "No registered service with id core/docgen exists in this environment". HMR of preview.js also dropped core annotations entirely (backgrounds/viewport globals lost until full reload).

This PR composes core annotations at the PreviewWeb layer (before beforeAll runs) and extracts the shared "prepend core unless already composed, then mark" logic into composeProjectAnnotationsWithCore, used by the preview, StoryStore, and portable stories so they can't drift.

Reviewer guide

File Why it matters
composeProjectAnnotationsWithCore.ts Start here — single source of truth for core-annotation composition
Preview.tsx The actual bug fix — compose before runBeforeAllHook
StoryStore.ts, portable-stories.ts Mechanical dedup onto the shared helper (behavior should be equivalent)
PreviewWeb.test.ts HMR assertions loosened to objectContaining because core globals are now correctly preserved

Risk areas: CSF4 definePreview double-composition (guarded by hasCoreAnnotations marker), portable-stories import * as preview namespace unwrapping (now one composeConfigs pass instead of per-module), falsy annotation entries (filter(Boolean) in helper).

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

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Generate a sandbox with a plain preview object (not definePreview), e.g. yarn task sandbox --template react-vite/default-ts --start-from auto
  2. Enable experimentalDocgenServer in .storybook/main.ts (or via feature flag if your template supports it)
  3. Add or open an autodocs/MDX page that renders <Controls /> for a component story
  4. Expected: Controls render with prop tables; preview console has no "No registered service with id core/docgen" error
  5. Edit .storybook/preview.ts globals (e.g. change a background) and save to trigger HMR
  6. Expected: Core globals (backgrounds, viewport, etc.) remain available after HMR — not reset to user-only globals
  7. Run yarn test PreviewWeb and yarn test portable-stories — both should pass

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

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • 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>

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Improved preview startup so built-in services and setup hooks are registered consistently.
    • Fixed project annotation handling to avoid missing or duplicated core settings in preview and story loading flows.
    • Relaxed global matching in tests to better reflect real-world global objects while keeping key values verified.

Plain (CSF1-3) previews don't fold the core annotations into
`getProjectAnnotations()`, so `PreviewWeb` ran `beforeAll` against
annotations without them — meaning core `beforeAll` hooks (e.g. the ones
registering the `core/docgen` and `core/story-docs` open services in the
preview) never ran, and an HMR of preview.js dropped core annotations
entirely (losing backgrounds/viewport/etc. globals until a full reload).

Compose the core annotations at the `PreviewWeb` layer, mirroring what the
store and portable stories already do. The shared decision (prepend core
unless already composed, then mark the result) is extracted into
`composeProjectAnnotationsWithCore` so the preview, the store, and portable
stories stay in sync instead of each re-implementing it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JReinhold JReinhold added bug core ci:daily Run the CI jobs that normally run in the daily job. qa:skip Pull Requests that do not need any QA. (e.g. documentation) labels Jun 29, 2026
@JReinhold JReinhold self-assigned this Jun 29, 2026
JReinhold and others added 2 commits June 29, 2026 16:38
TypeScript cannot infer the renderer from ModuleExports[], so the
helper defaulted to Renderer and broke compile in Preview.tsx.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JReinhold
JReinhold marked this pull request as ready for review June 29, 2026 20:31
@JReinhold
JReinhold requested review from ndelangen and yannbf June 29, 2026 20:31
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bfa45c79-f690-4f0d-b19c-d5a452b082aa

📥 Commits

Reviewing files that changed from the base of the PR and between 98de385 and ff2a9a8.

📒 Files selected for processing (6)
  • code/core/src/preview-api/modules/preview-web/Preview.tsx
  • code/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts
  • code/core/src/preview-api/modules/store/StoryStore.ts
  • code/core/src/preview-api/modules/store/csf/composeProjectAnnotationsWithCore.ts
  • code/core/src/preview-api/modules/store/csf/index.ts
  • code/core/src/preview-api/modules/store/csf/portable-stories.ts

📝 Walkthrough

Walkthrough

Introduces a new composeProjectAnnotationsWithCore helper that centralizes the logic for conditionally prepending Storybook core annotations. StoryStore, portable-stories/setProjectAnnotations, and Preview.getProjectAnnotationsOrRenderError are all updated to route through this helper, removing duplicated branching based on hasCoreAnnotations/getCoreAnnotations. Test assertions are relaxed to objectContaining to accommodate extra globals keys.

Core Annotation Composition Refactor

Layer / File(s) Summary
composeProjectAnnotationsWithCore helper
code/core/src/preview-api/modules/store/csf/composeProjectAnnotationsWithCore.ts, code/core/src/preview-api/modules/store/csf/index.ts
New exported function filters falsy entries, detects whether core annotations are already present, conditionally prepends them, composes via composeConfigs, and marks the result with markAsComposedWithCoreAnnotations. Re-exported from the csf index.
StoryStore and portable-stories migration
code/core/src/preview-api/modules/store/StoryStore.ts, code/core/src/preview-api/modules/store/csf/portable-stories.ts
Removes getCoreAnnotations/hasCoreAnnotations imports and manual branching; composeProjectAnnotations and setProjectAnnotations now delegate entirely to composeProjectAnnotationsWithCore.
Preview initialization and test updates
code/core/src/preview-api/modules/preview-web/Preview.tsx, code/core/src/preview-api/modules/preview-web/PreviewWeb.test.ts
getProjectAnnotationsOrRenderError wraps fetched annotations with composeProjectAnnotationsWithCore. Test assertions for globals/userGlobals are relaxed to expect.objectContaining.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

@ndelangen
ndelangen merged commit 9efd8af into next Jul 1, 2026
332 of 333 checks passed
@ndelangen
ndelangen deleted the fix/preview-core-annotations branch July 1, 2026 13:51
@github-actions github-actions Bot mentioned this pull request Jul 1, 2026
2 tasks done
This was referenced Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:daily Run the CI jobs that normally run in the daily job. core qa:skip Pull Requests that do not need any QA. (e.g. documentation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants