Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
import { CHANGE_DETECTION_STATUS_TYPE_ID } from 'storybook/internal/types';

import { getService } from '../../shared/open-service/server.ts';
import type { moduleGraphServiceDef } from '../../shared/open-service/services/module-graph/definition.ts';
import type { ModuleGraphService } from '../../shared/open-service/services/module-graph/definition.ts';
import { getStoryIdsByAbsolutePath } from '../../shared/open-service/services/module-graph/story-files.ts';
import type {
ErrorLike,
Expand Down Expand Up @@ -151,7 +151,7 @@ export class ChangeDetectionService {
}

private getModuleGraph() {
return getService<typeof moduleGraphServiceDef>('core/module-graph');
return getService<ModuleGraphService>('core/module-graph');
}

/** True while the service is live and change-detection status publishing is enabled. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { normalize } from 'pathe';
import { vi } from 'vitest';

import { getService } from '../../shared/open-service/server.ts';
import type { moduleGraphServiceDef } from '../../shared/open-service/services/module-graph/definition.ts';
import type { ModuleGraphService } from '../../shared/open-service/services/module-graph/definition.ts';
import { ModuleGraphEngine } from '../../shared/open-service/services/module-graph/engine/module-graph-engine.ts';
import type { ModuleGraphStatus } from '../../shared/open-service/services/module-graph/types.ts';
import {
Expand Down Expand Up @@ -80,7 +80,7 @@ export function installModuleGraphQueryMock(engine: ModuleGraphEngine) {
}
),
},
} as unknown as ReturnType<typeof getService<typeof moduleGraphServiceDef>>);
} as unknown as ModuleGraphService);

return {
applySnapshot: () => {
Expand Down
4 changes: 2 additions & 2 deletions code/core/src/core-server/utils/manifests/manifests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { Polka } from 'polka';
import invariant from 'tiny-invariant';

import { getService } from '../../../shared/open-service/server.ts';
import type { docgenServiceDef } from '../../../shared/open-service/services/docgen/definition.ts';
import type { DocgenService } from '../../../shared/open-service/services/docgen/definition.ts';
import { Tag } from '../../../shared/constants/tags.ts';
import type { ComponentManifest, ComponentsManifest } from '../../../types/modules/core-common.ts';
import type { DocgenPayload } from '../../../shared/open-service/services/docgen/types.ts';
Expand Down Expand Up @@ -111,7 +111,7 @@ async function renderComponentsHtmlFromService(
manifestComponentIds: string[],
docsManifest?: DocsManifest
) {
const docgenService = getService<typeof docgenServiceDef>('core/docgen');
const docgenService = getService<DocgenService>('core/docgen');
const startTime = performance.now();
const allPayloads = await docgenService.queries.getDocgenForAllComponents.loaded();
const durationMs = Math.round(performance.now() - startTime);
Expand Down
16 changes: 9 additions & 7 deletions code/core/src/shared/open-service/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ Handlers resolve other registered services through `ctx.getService(serviceId)`.
parameter, the return type is `RuntimeService` — query and command results are erased to
`unknown`.

Pass the source service definition as a generic to recover the full typed runtime surface:
Pass the source service instance type as a generic to recover the full typed runtime surface:

```ts
import type { mutableRecordLookupServiceDef } from './mutable-record-lookup.ts';
import type { MutableRecordLookupService } from './mutable-record-lookup.ts';

handler: (input, ctx) => {
const lookup = ctx.getService<typeof mutableRecordLookupServiceDef>(
const lookup = ctx.getService<MutableRecordLookupService>(
'internal-fixture/mutable-record-lookup'
);

Expand Down Expand Up @@ -258,10 +258,12 @@ That split is intentional:
writing for the current server process; the registry itself lives in
[service-registry.ts](./service-registry.ts), shared with the browser entrypoints

`registerService(definition)` throws `OpenServiceDuplicateRegistrationError` if a service with the
same id is already registered. The default `services` preset hook in
[common-preset.ts](../../../core-server/presets/common-preset.ts) also throws if the preset is applied
more than once in the same process, which catches duplicate registration paths early.
`registerService(definition)` is idempotent by id: registering an id that already exists returns the
existing runtime instead of throwing. This keeps core services safe to register from a `beforeAll`
annotation, which CSF4 composes twice (once in `definePreview`, once in `StoryStore`) and which also
re-runs on HMR. The default `services` preset hook in
[common-preset.ts](../../../core-server/presets/common-preset.ts) still throws if the preset is applied
more than once in the same process, which catches misconfigured preset wiring early.

The internal Storybook config registers an example debug service through a dedicated preset file
([`code/.storybook/services-preset.ts`](../../../../.storybook/services-preset.ts)), gated on
Expand Down
5 changes: 4 additions & 1 deletion code/core/src/shared/open-service/fixtures.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as v from 'valibot';

import { defineService } from './service-definition.ts';
import type { ServiceInstanceOf } from './types.ts';

/** Shared schema used by fixtures that address one logical record by id. */
export const entryIdInputSchema = v.object({ entryId: v.string() });
Expand Down Expand Up @@ -53,6 +54,8 @@ export const mutableRecordLookupServiceDef = defineService({
},
});

export type MutableRecordLookupService = ServiceInstanceOf<typeof mutableRecordLookupServiceDef>;

export type PreloadedValueState = Record<string, string | undefined>;

/** Service fixture that loads state from a command before returning it. */
Expand Down Expand Up @@ -250,7 +253,7 @@ export function createDerivedBooleanFromChildQueryServiceDef() {
input: entryIdInputSchema,
output: booleanOutputSchema,
handler: (input, ctx) => {
const source = ctx.getService<typeof mutableRecordLookupServiceDef>(
const source = ctx.getService<MutableRecordLookupService>(
mutableRecordLookupServiceDef.id
);
const record = source.queries.getRecordFields({
Expand Down
6 changes: 3 additions & 3 deletions code/core/src/shared/open-service/server.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as v from 'valibot';
import { describe, expectTypeOf, it } from 'vitest';

import { defineService } from './index.ts';
import { mutableRecordLookupServiceDef } from './fixtures.ts';
import { type MutableRecordLookupService, mutableRecordLookupServiceDef } from './fixtures.ts';
import { registerService } from './server.ts';
import type { RuntimeService } from './types.ts';

Expand Down Expand Up @@ -130,7 +130,7 @@ describe('open-service registration types', () => {
});
});

it('types cross-service lookups when getService receives a definition generic', () => {
it('types cross-service lookups when getService receives an instance generic', () => {
registerService(mutableRecordLookupServiceDef);
registerService(
defineService({
Expand All @@ -141,7 +141,7 @@ describe('open-service registration types', () => {
input: entryIdInputSchema,
output: v.nullable(v.string()),
handler: (_input, ctx) => {
const lookup = ctx.getService<typeof mutableRecordLookupServiceDef>(
const lookup = ctx.getService<MutableRecordLookupService>(
'internal-fixture/mutable-record-lookup'
);

Expand Down
3 changes: 2 additions & 1 deletion code/core/src/shared/open-service/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import {
awaitedPreloadValueServiceDef,
createSharedStaticFileServiceDef,
type MutableRecordLookupService,
mutableRecordLookupServiceDef,
} from './fixtures.ts';

Expand Down Expand Up @@ -114,7 +115,7 @@ describe('server static builds', () => {
input: v.undefined(),
output: v.undefined(),
handler: async (_input, ctx) => {
const source = ctx.getService<typeof mutableRecordLookupServiceDef>(
const source = ctx.getService<MutableRecordLookupService>(
'internal-fixture/mutable-record-lookup'
);
const record = source.queries.getRecordFields({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@
* Peers are simulated with the test channel's `emitExternal`, the same approach the sync tests use.
*/
import * as v from 'valibot';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest';

import { mutableRecordLookupServiceDef } from './fixtures.ts';
import {
awaitedPreloadValueServiceDef,
entryIdInputSchema,
mutableRecordLookupServiceDef,
preloadedValueOutputSchema,
voidOutputSchema,
} from './fixtures.ts';
import { defineService } from './service-definition.ts';
import {
SERVICE_COMMAND_ACK,
Expand Down Expand Up @@ -54,6 +60,29 @@ const throwingCommandServiceDef = defineService({
},
});

/** Query `load` invokes a command that has no handler in this runtime (peer-only). */
const loadInvokesRemoteCommandServiceDef = defineService({
id: 'internal-fixture/load-invokes-remote-command',
description: 'Query load calls a command declared without a local handler.',
initialState: {} as Record<string, string | undefined>,
queries: {
getPreloadedValue: {
description: 'Populates state via a remote-only command inside load.',
input: entryIdInputSchema,
output: preloadedValueOutputSchema,
handler: (input, ctx) => ctx.self.state[input.entryId] ?? null,
load: (input, ctx) => ctx.self.commands.preloadValue(input).then(() => undefined),
},
},
commands: {
preloadValue: {
description: 'Populates one entry — implemented only on a peer in this test runtime.',
input: entryIdInputSchema,
output: voidOutputSchema,
},
},
});

function emittedCalls(channel: ReturnType<typeof createTestChannel>, event: string) {
return channel.emit.mock.calls.filter(([name]) => name === event);
}
Expand Down Expand Up @@ -279,3 +308,54 @@ describe('remote command responder (has local handler)', () => {
expect(emittedCalls(channel, SERVICE_COMMAND_RESULT)).toHaveLength(0);
});
});

describe('load bodies and command routing', () => {
it('calls the local command handler from a load body without emitting command-invoke', async () => {
const channel = createTestChannel();
installTestChannel(channel);
const handlerSpy = vi.spyOn(awaitedPreloadValueServiceDef.commands.preloadValue, 'handler');
onTestFinished(() => {
handlerSpy.mockRestore();
});

const service = registerService(awaitedPreloadValueServiceDef);

await service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' });

expect(handlerSpy).toHaveBeenCalledTimes(1);
expect(handlerSpy.mock.calls[0]?.[0]).toEqual({ entryId: 'entry-a' });
expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(0);
expect(service.queries.getPreloadedValue({ entryId: 'entry-a' })).toBe('preloaded');
});

it('routes a load-body command through command-invoke when no local handler exists', async () => {
const channel = createTestChannel();
installTestChannel(channel);

const service = registerService(loadInvokesRemoteCommandServiceDef);
const promise = service.queries.getPreloadedValue.loaded({ entryId: 'entry-a' });

await vi.waitFor(() => expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)).toHaveLength(1));

expect(emittedCalls(channel, SERVICE_COMMAND_INVOKE)[0]?.[1]).toMatchObject({
serviceId: loadInvokesRemoteCommandServiceDef.id,
commandName: 'preloadValue',
input: { entryId: 'entry-a' },
callId: expect.any(String),
clientId: expect.any(String),
});

const { callId } = emittedCalls(
channel,
SERVICE_COMMAND_INVOKE
)[0]?.[1] as CommandInvokePayload;
channel.emitExternal(SERVICE_COMMAND_RESULT, {
serviceId: loadInvokesRemoteCommandServiceDef.id,
callId,
result: undefined,
clientId: 'peer',
});

await expect(promise).resolves.toBeNull();
});
});
21 changes: 7 additions & 14 deletions code/core/src/shared/open-service/service-registration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
assignEntryFieldInputSchema,
createDerivedBooleanFromChildQueryServiceDef,
entryIdInputSchema,
type MutableRecordLookupService,
hiddenServiceDef,
internalStaticBuildServiceDef,
mixedVisibilityServiceDef,
Expand Down Expand Up @@ -69,20 +70,12 @@ describe('service registration', () => {
expect(descriptor.commands.assignRecordField.output).toBe(voidOutputSchema);
});

it('throws when registering the same service id twice', () => {
registerService(mutableRecordLookupServiceDef);
it('is idempotent by id: re-registering returns the existing instance instead of throwing', () => {
const first = registerService(mutableRecordLookupServiceDef);
const second = registerService(mutableRecordLookupServiceDef);

try {
registerService(mutableRecordLookupServiceDef);
expect.unreachable('Expected duplicate registration to throw');
} catch (error) {
expect(error).toMatchObject({
fromStorybook: true,
code: 6,
message:
'A service with id "internal-fixture/mutable-record-lookup" is already registered.',
});
}
expect(second).toBe(first);
expect(getRegisteredServices()).toHaveLength(1);
});

it('throws a Storybook error when resolving a missing registered service id', () => {
Expand Down Expand Up @@ -143,7 +136,7 @@ describe('service registration', () => {
},
assignFromLookup: {
handler: async (input, ctx) => {
const lookup = ctx.getService<typeof mutableRecordLookupServiceDef>(
const lookup = ctx.getService<MutableRecordLookupService>(
'internal-fixture/mutable-record-lookup'
);

Expand Down
26 changes: 13 additions & 13 deletions code/core/src/shared/open-service/service-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import {
OpenServiceDuplicateRegistrationError,
OpenServiceMissingChannelError,
OpenServiceMissingServiceError,
} from '../../server-errors.ts';
Expand All @@ -34,7 +33,6 @@ import type {
ServiceDescriptor,
ServiceId,
ServiceInstance,
ServiceInstanceOf,
ServiceRegistrationOptions,
ServiceRegistryApi,
ServiceSummary,
Expand Down Expand Up @@ -197,7 +195,7 @@ export interface ServiceRegisterOptions {
* callers use, wraps commands to broadcast their post-mutation state, and joins the cross-peer sync
* protocol as a hub or leaf (`relay`). Each runtime must install the addons channel at its entry
* boundary before calling this (builders, manager boot, server `services` preset, or Node import
* bootstrap). Duplicate ids are rejected up front so lookups remain deterministic.
* bootstrap). Registration is idempotent by id: a repeated registration returns the existing runtime.
*/
export function registerService<
TState,
Expand All @@ -210,8 +208,15 @@ export function registerService<
): ServiceInstance<TState, TQueries, TCommands> & ServiceRegistryApi {
const registry = getRegistry();

if (registry.has(definition.id)) {
throw new OpenServiceDuplicateRegistrationError({ serviceId: definition.id });
// Registration is idempotent by id. Re-registering an already-registered service returns the
// existing runtime instead of throwing. This deliberately swallows duplicate-id collisions, which is
// the right trade-off: core services register from a `beforeAll` annotation that CSF4 composes twice
// (once in `definePreview`, once in `StoryStore`), and `beforeAll` also re-runs on HMR. A second
// registration is a no-op rather than a crash.
const existingEntry = registry.get(definition.id);
if (existingEntry) {
return existingEntry.instance as unknown as ServiceInstance<TState, TQueries, TCommands> &
ServiceRegistryApi;
}

const ownClientId = generateClientId();
Expand Down Expand Up @@ -265,6 +270,7 @@ export function registerService<
commands: runtime.commands as Record<string, (input: unknown) => Promise<unknown>>,
implementedCommandNames,
commandNames: Object.keys(resolvedDefinition.commands),
runtime,
});

const instance = {
Expand Down Expand Up @@ -317,20 +323,14 @@ export async function describeService(serviceId: ServiceId): Promise<ServiceDesc
* Query and command contexts delegate cross-service calls through this lookup so one service can reuse
* another's runtime contract. Synchronous because callers need it inside sync query handlers.
*/
export function getService(serviceId: ServiceId): RuntimeService;
export function getService<TDefinition extends AnyServiceDefinition>(
serviceId: ServiceId
): ServiceInstanceOf<TDefinition>;
export function getService<TDefinition extends AnyServiceDefinition>(
serviceId: ServiceId
): RuntimeService | ServiceInstanceOf<TDefinition> {
export function getService<TInstance = RuntimeService>(serviceId: ServiceId): TInstance {
const entry = getRegistry().get(serviceId);

if (!entry) {
throw new OpenServiceMissingServiceError({ serviceId });
}

return entry.instance as unknown as ServiceInstanceOf<TDefinition>;
return entry.instance as unknown as TInstance;
}

/**
Expand Down
Loading