Skip to content

Open-Service: Type getService for core services per runtime - #35242

Merged
JReinhold merged 6 commits into
nextfrom
split/typed-core-getservice
Jun 25, 2026
Merged

JReinhold merged 6 commits into
nextfrom
split/typed-core-getservice

Conversation

@JReinhold

@JReinhold JReinhold commented Jun 20, 2026 •

Copy link
Copy Markdown
Contributor

Closes #

What I did

Makes module-level getService('core/...') type-aware at compile time for core services, per runtime (manager, preview, server), without requiring an explicit generic. Addon services keep the existing getService<MyService>('my-addon/service') path.

  • Added core-service-types.ts with per-runtime definition lists as the single source of truth; *CoreServices types and TypedGetService overloads derive from those lists.
  • Extended defineService / ServiceDefinition with a TId generic so service ids stay as string literals (e.g. 'core/docgen') for map key derivation.
  • Split server extraction registration into extraction-service.server.ts plus per-service docgen/server.ts and story-docs/server.ts registrars (one registrar file per service per runtime).
  • Added a membership unit test that compares def lists to the registrar-file convention, with actionable failure messages.
  • Hardened extraction-service wiring: command names are typed as keyof TCommands, and invariants assert query/command names exist on the definition before registration.
  • Removed redundant explicit generics at call sites; documented the convention in open-service/README.md.

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!

No additional manual QA beyond the automated checks below is required for this PR: the change is compile-time typing and internal registration wiring with no user-visible UI surface. A maintainer can still spot-check the extraction services if desired.

  1. From the repo root, run the focused unit and type tests:

    yarn test --run core-service-types.test docgen/server.test story-docs/server.test
    yarn test --run open-service/manager.test-d open-service/preview.test-d open-service/server.test-d

    Expect all tests to pass with no type errors.

  2. Confirm TypeScript is clean for core:

    yarn nx check core

    Expect ✅ No type errors.

  3. (Optional regression check) If experimentalDocgenServer is enabled in the internal Storybook UI, open a docs story and confirm Controls / Source blocks still load docgen and story-docs data without console errors:

    cd code && yarn storybook:ui

Areas most likely to regress: server-side docgen/story-docs extraction and hot-refresh re-extraction after story file changes (touched by the extraction-service split).

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

  • Refactor
    • Improved getService typing across manager, preview, and server using core service id inference, with unknown ids falling back to a generic runtime type.
    • Split docgen and story-docs registration into dedicated APIs, using a shared extraction-service registration for per-component payloads.
    • Enhanced service definition typing to preserve service id as a literal type.
  • Documentation
    • Added a “Core service typing” guide describing id-to-type mapping and fallback behavior across runtimes.
  • Tests
    • Added TypeScript compile-time checks for getService typing (including fallback/override behavior).
    • Added tests validating declared core service inventories match service folders on disk.

JReinhold and others added 3 commits June 18, 2026 23:02
Adds per-runtime core-service maps so getService('core/...') is typed
without an explicit generic, while keeping the generic fallback for addon
services. Splits the server extraction-service registration into per-service
registrar files and preserves the literal service id through defineService so
the maps derive their keys from each definition.

Co-authored-by: Cursor <cursoragent@cursor.com>
… into split/typed-core-getservice

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	code/core/src/shared/open-service/services/docgen/server.ts
Constrains the extraction registrar's command names to keys of the
service's commands (matching queryName) and adds invariants that the
query/command names exist on the definition, so a wiring typo fails at the
boundary instead of silently registering nothing. Also clarifies that the
core-service-types membership test guards the registrar-file convention, not
the actual call site.

Co-authored-by: Cursor <cursoragent@cursor.com>
@JReinhold JReinhold added build Internal-facing build tooling & test updates ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. (e.g. documentation) maintenance User-facing maintenance tasks and removed build Internal-facing build tooling & test updates labels Jun 20, 2026
@JReinhold JReinhold self-assigned this Jun 20, 2026
@JReinhold
JReinhold requested a review from ndelangen June 20, 2026 19:55
@storybook-app-bot

storybook-app-bot Bot commented Jun 20, 2026 •

Copy link
Copy Markdown
Contributor

Package Benchmarks

Commit: 0e08aeb, ran on 25 June 2026 at 13:19:39 UTC

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

storybook

Before After Difference
Dependency count 72 72 0
Self size 21.14 MB 21.68 MB 🚨 +531 KB 🚨
Dependency size 36.44 MB 36.44 MB 0 B
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 204 204 0
Self size 821 KB 821 KB 🎉 -84 B 🎉
Dependency size 90.18 MB 90.71 MB 🚨 +531 KB 🚨
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 197 197 0
Self size 32 KB 32 KB 0 B
Dependency size 88.66 MB 89.19 MB 🚨 +531 KB 🚨
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 73 73 0
Self size 1.09 MB 1.09 MB 0 B
Dependency size 57.58 MB 58.11 MB 🚨 +531 KB 🚨
Bundle Size Analyzer node node

@JReinhold
JReinhold marked this pull request as ready for review June 20, 2026 20:21
@coderabbitai

coderabbitai Bot commented Jun 20, 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: ca08049f-b361-4989-96d2-666aa150bcb4

📥 Commits

Reviewing files that changed from the base of the PR and between 5d39a2a and 0e08aeb.

📒 Files selected for processing (1)
  • code/core/src/core-server/presets/common-preset.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • code/core/src/core-server/presets/common-preset.ts

📝 Walkthrough

Walkthrough

Adds typed core-service getService exports, splits docgen and story-docs registration into separate APIs through a shared extraction-service helper, updates callers, and adds type, membership, and service tests plus documentation for the core-service typing convention.

Changes

Typed core getService and split extraction service registration

Layer / File(s) Summary
Service typing contracts
code/core/src/shared/open-service/types.ts, code/core/src/shared/open-service/service-definition.ts, code/core/src/shared/open-service/core-service-types.ts, code/core/src/shared/open-service/core-service-types.test.ts, code/core/src/shared/open-service/manager.test-d.ts, code/core/src/shared/open-service/preview.test-d.ts, code/core/src/shared/open-service/server.test-d.ts, code/core/src/shared/open-service/README.md
ServiceDefinition and defineService carry literal service ids, core-service-types.ts defines per-runtime core service lists and typed getService overloads, the membership test checks those lists against runtime registrar files, the typed getService compile-time tests cover known and unknown ids, and the README adds the core-service typing section.
Typed getService exports and callers
code/core/src/shared/open-service/manager.ts, code/core/src/shared/open-service/preview.ts, code/core/src/shared/open-service/server.ts, code/core/src/controls/manager.tsx, code/core/src/core-server/change-detection/change-detection-service.ts, code/core/src/core-server/utils/manifests/manifests.ts, code/addons/docs/src/blocks/blocks/use-service-docgen.ts, code/addons/docs/src/blocks/blocks/use-service-story-docs.ts, code/core/src/shared/open-service/services/story-docs/story-docs-source-before-each.ts
The manager, preview, and server open-service entrypoints export typed getService constants, and call sites in controls, change detection, manifests, addon hooks, and story-docs source code drop explicit getService<T> annotations.
Shared extraction service and service APIs
code/core/src/shared/open-service/services/extraction-service.server.ts, code/core/src/shared/open-service/services/docgen/server.ts, code/core/src/shared/open-service/services/story-docs/server.ts, code/core/src/shared/open-service/services/docgen/server.test.ts, code/core/src/shared/open-service/services/story-docs/server.test.ts
registerExtractionService centralizes per-component extraction and refresh behavior, registerDocgenService and registerStoryDocsService delegate to it with separate option shapes, and the service tests cover the updated registration and refresh paths.
Preset and manifest wiring
code/core/src/core-server/presets/common-preset.ts, code/core/src/core-server/utils/manifests/manifests.test.ts
The experimental docgen preset and manifest-generation tests switch from the combined docgen services helper to separate docgen and story-docs registrations using the new provider option names.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

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

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
code/core/src/shared/open-service/services/extraction-service.server.ts (1)

72-74: 💤 Low value

Consider logging refresh failures for observability.

Errors from individual component refreshes are silently swallowed. While this prevents one failure from breaking others, it may make debugging harder when extractions fail unexpectedly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/core/src/shared/open-service/services/extraction-service.server.ts`
around lines 72 - 74, The refreshComponent call within the idsToRefresh.map() is
silently catching and ignoring errors with .catch(() => undefined), which
prevents visibility into refresh failures. Modify the catch handler for
refreshComponent to log the error details before returning undefined, so that
individual refresh failures are recorded for debugging purposes while still
allowing other refresh operations to continue without interruption.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@code/core/src/shared/open-service/services/story-docs/story-docs-source-before-each.ts`:
- Around line 23-25: The variable `service` is declared without a type
annotation, which loses the compile-time safety benefits of the typed
`getService('core/story-docs')` function. Replace the bare `let service;`
declaration with a properly typed declaration that captures the return type from
`getService('core/story-docs')`, so that subsequent operations like
`service.queries.getStoryDocs` remain statically type-checked. Use the
appropriate type or let TypeScript infer it from the initial assignment within
the try block.

---

Nitpick comments:
In `@code/core/src/shared/open-service/services/extraction-service.server.ts`:
- Around line 72-74: The refreshComponent call within the idsToRefresh.map() is
silently catching and ignoring errors with .catch(() => undefined), which
prevents visibility into refresh failures. Modify the catch handler for
refreshComponent to log the error details before returning undefined, so that
individual refresh failures are recorded for debugging purposes while still
allowing other refresh operations to continue without interruption.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e9908a5b-c7b4-461c-ac1f-c0197b037ed9

📥 Commits

Reviewing files that changed from the base of the PR and between 0cc5e6c and c7c69be.

📒 Files selected for processing (24)
  • code/addons/docs/src/blocks/blocks/use-service-docgen.ts
  • code/addons/docs/src/blocks/blocks/use-service-story-docs.ts
  • code/core/src/controls/manager.tsx
  • code/core/src/core-server/change-detection/change-detection-service.ts
  • code/core/src/core-server/presets/common-preset.ts
  • code/core/src/core-server/utils/manifests/manifests.test.ts
  • code/core/src/core-server/utils/manifests/manifests.ts
  • code/core/src/shared/open-service/README.md
  • code/core/src/shared/open-service/core-service-types.test.ts
  • code/core/src/shared/open-service/core-service-types.ts
  • code/core/src/shared/open-service/manager.test-d.ts
  • code/core/src/shared/open-service/manager.ts
  • code/core/src/shared/open-service/preview.test-d.ts
  • code/core/src/shared/open-service/preview.ts
  • code/core/src/shared/open-service/server.test-d.ts
  • code/core/src/shared/open-service/server.ts
  • code/core/src/shared/open-service/service-definition.ts
  • code/core/src/shared/open-service/services/docgen/server.test.ts
  • code/core/src/shared/open-service/services/docgen/server.ts
  • code/core/src/shared/open-service/services/extraction-service.server.ts
  • code/core/src/shared/open-service/services/story-docs/server.test.ts
  • code/core/src/shared/open-service/services/story-docs/server.ts
  • code/core/src/shared/open-service/services/story-docs/story-docs-source-before-each.ts
  • code/core/src/shared/open-service/types.ts

@JReinhold JReinhold mentioned this pull request Jun 21, 2026
51 of 61 tasks
JReinhold and others added 2 commits June 23, 2026 13:52
Use an IIFE so the service handle keeps its inferred preview type instead
of widening through an untyped let declaration.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve conflicts from the query rename (drop redundant `get` prefix) while
keeping per-service registrars and typed getService. Update query/command
wiring to docgen, storyDocs, and latestStoryChanges.

Co-authored-by: Cursor <cursoragent@cursor.com>
Base automatically changed from split/open-service-query-states to next June 25, 2026 13:03
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

ci:normal Run our default set of CI jobs (choose this for most PRs). maintenance User-facing maintenance tasks 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