Open Service: Call remote commands in load functions#35106
Conversation
Load bodies now call the channel-routed command map so queries can invoke commands implemented only on a peer (e.g. server-side extraction). Also makes service registration idempotent and simplifies getService generics. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
… types getService<typeof docgenServiceDef> must resolve to ServiceInstanceOf so runtime queries expose .loaded(). Keep a separate overload for explicit instance types such as DocgenService. Co-authored-by: Cursor <cursoragent@cursor.com>
…instance types" This reverts commit f37fa24.
…call With getService<TInstance>, pass runtime instance types rather than typeof the service definition. Adds DocgenService alias and updates manifests.ts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds channel-installed command maps for load bodies (attachChannelCommands), routes remote commands via the channel while preserving local gating, consolidates getService to an instance-type generic, makes registerService idempotent by service id, and adds tests covering load-body remote-command routing. ChangesChannel-routed load-body command execution with service-instance typing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
Use onTestFinished for handler spy restore in load-body test, and reuse a single remoteCommandNames Set cleared on attachChannelCommands. Co-authored-by: Cursor <cursoragent@cursor.com>
…ce type After merging next, the getService<typeof def> callsites for core/module-graph resolved to the definition type (no subscribe/loaded) under this PR's single getService<TInstance> generic. Add a ModuleGraphService instance-type alias and use it. Also fix remoteCommandNames Set.add spread misuse. Co-authored-by: Cursor <cursoragent@cursor.com>
Closes #
What I did
Open-service infrastructure so query load bodies invoke the channel-routed command map instead of the raw local map. This lets a peer-only command (e.g. server
extractDocgencalled from managergetDocgenload) run remotely instead of throwing locally. This already worked when the manager-side called the command directly, but it didn't work if the command was called from within a query's load-function.Also makes
registerServiceidempotent by id (needed for corebeforeAllregistration) and simplifiesgetServicegenerics. Meaning that before we would throw when trying to register a service with an ID that already existed - now it just no-ops, allowing the sameregisterService-call to happen multiple times. This can silently hide bugs, but it's the practical thing to do right now.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated 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 user-visible behavior change until a service load invokes a peer-only command. Unit tests in
service-command-transport.test.tsandservice-registration.test.tscover the new routing and idempotent registration.Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake 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/coreteam 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
New Features
Bug Fixes
Refactor
Tests
Documentation