Skip to content

Commit 62011d4

Browse files
authored
Merge pull request storybookjs#35154 from storybookjs/jeppe-cursor/cd8d79b3
Open Service: Load static query snapshots in browser builds
2 parents 48669a1 + 3b8ceaa commit 62011d4

38 files changed

Lines changed: 1692 additions & 401 deletions

code/.storybook/main.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const currentDirPath = dirname(currentFilePath);
1414

1515
const componentsPath = join(currentDirPath, '../core/src/components/index.ts');
1616
const managerApiPath = join(currentDirPath, '../core/src/manager-api/index.mock.ts');
17+
const previewApiPath = join(currentDirPath, '../core/src/preview-api/index.ts');
1718
const themingCreatePath = join(currentDirPath, '../core/src/theming/create.ts');
1819
const themingPath = join(currentDirPath, '../core/src/theming/index.ts');
1920
const imageContextPath = join(currentDirPath, '../frameworks/nextjs/src/image-context.ts');
@@ -173,12 +174,14 @@ const config = defineMain({
173174
? {
174175
'storybook/internal/components': componentsPath,
175176
'storybook/manager-api': managerApiPath,
177+
'storybook/preview-api': previewApiPath,
176178
'storybook/theming/create': themingCreatePath,
177179
'storybook/theming': themingPath,
178180
'sb-original/image-context': imageContextPath,
179181
}
180182
: {
181183
'storybook/manager-api': managerApiPath,
184+
'storybook/preview-api': previewApiPath,
182185
},
183186
},
184187
plugins: [react()],

code/.storybook/preview.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,20 @@ sb.mock(import('uuid'));
5454
import '../core/src/shared/open-service/sync-test/preview.ts';
5555

5656
const { document } = global;
57-
globalThis.CONFIG_TYPE = 'DEVELOPMENT';
57+
58+
// The internal Storybook's manager stories (sidebar context menu, onboarding guide, …) demonstrate
59+
// DEVELOPMENT-only UI, so they must render as if in development even though this Storybook is itself
60+
// built and served statically (`yarn storybook:ui`, Chromatic, Vitest portable stories). In Vitest the
61+
// preview builder never injects CONFIG_TYPE (it is undefined); in Chromatic it is injected as PRODUCTION.
62+
// The one exception is the open-service static-load E2E, which serves a plain production build to
63+
// exercise the real static-loading path — there CONFIG_TYPE is PRODUCTION and Chromatic is not driving
64+
// the browser, so we leave the injected value untouched.
65+
// TODO(open-service): #29743 added an unconditional `CONFIG_TYPE = 'DEVELOPMENT'` here with no rationale.
66+
// Ask Norbert why the internal Storybook needs to force development mode rather than the stories opting
67+
// in per-story; if it can be removed, this gate should go with it.
68+
if (isChromatic() || globalThis.CONFIG_TYPE !== 'PRODUCTION') {
69+
globalThis.CONFIG_TYPE = 'DEVELOPMENT';
70+
}
5871

5972
const ThemeBlock = styled.div<{ side: 'left' | 'right'; layout: string }>(
6073
{

code/core/src/manager-errors.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export enum Category {
1919
MANAGER_ROUTER = 'MANAGER_ROUTER',
2020
MANAGER_THEMING = 'MANAGER_THEMING',
2121
MANAGER_UNIVERSAL_STORE = 'MANAGER_UNIVERSAL-STORE',
22+
MANAGER_OPEN_SERVICE = 'MANAGER_OPEN-SERVICE',
2223
}
2324

2425
export class ProviderDoesNotExtendBaseProviderError extends StorybookError {
@@ -78,3 +79,46 @@ export class UniversalStoreFollowerTimeoutError extends StorybookError {
7879
});
7980
}
8081
}
82+
83+
export class OpenServiceStaticSnapshotLoadError extends StorybookError {
84+
constructor(
85+
public data: {
86+
serviceId: string;
87+
queryName: string;
88+
input: unknown;
89+
logicalPath: string;
90+
url: string;
91+
cause: unknown;
92+
status?: number;
93+
statusText?: string;
94+
}
95+
) {
96+
super({
97+
name: 'OpenServiceStaticSnapshotLoadError',
98+
category: Category.MANAGER_OPEN_SERVICE,
99+
code: 1,
100+
message: `Failed to load open-service static snapshot "${data.logicalPath}" from "${data.url}" for "${data.serviceId}.${data.queryName}".`,
101+
cause: data.cause,
102+
});
103+
}
104+
}
105+
106+
export class OpenServiceStaticSnapshotInvalidError extends StorybookError {
107+
constructor(
108+
public data: {
109+
serviceId: string;
110+
queryName: string;
111+
input: unknown;
112+
logicalPath: string;
113+
url: string;
114+
received: unknown;
115+
}
116+
) {
117+
super({
118+
name: 'OpenServiceStaticSnapshotInvalidError',
119+
category: Category.MANAGER_OPEN_SERVICE,
120+
code: 2,
121+
message: `Open-service static snapshot "${data.logicalPath}" from "${data.url}" for "${data.serviceId}.${data.queryName}" must be a plain object.`,
122+
});
123+
}
124+
}

code/core/src/manager/globals/exports.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,8 @@ export default {
659659
],
660660
'storybook/internal/manager-errors': [
661661
'Category',
662+
'OpenServiceStaticSnapshotInvalidError',
663+
'OpenServiceStaticSnapshotLoadError',
662664
'ProviderDoesNotExtendBaseProviderError',
663665
'StatusTypeIdMismatchError',
664666
'UncaughtManagerError',

code/core/src/server-errors.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,17 @@ export class OpenServiceRemoteCommandDisconnectedError extends StorybookError {
276276
}
277277
}
278278

279+
export class OpenServiceRemoteCommandUnhandledError extends StorybookError {
280+
constructor(public data: { serviceId: ServiceId; commandName: string }) {
281+
super({
282+
name: 'OpenServiceRemoteCommandUnhandledError',
283+
category: Category.CORE_COMMON,
284+
code: 15,
285+
message: `No runtime acknowledged remote command "${data.serviceId}.${data.commandName}"; its handler is not implemented in any connected runtime.`,
286+
});
287+
}
288+
}
289+
279290
export class WebpackMissingStatsError extends StorybookError {
280291
constructor() {
281292
super({

code/core/src/shared/open-service/README.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Internal tests and implementation code may import from the individual modules di
6969
- [service-error-serialization.ts](./service-error-serialization.ts): transport-safe (de)serialization of thrown errors and their `cause` chains, used by remote command replies
7070
- [channel-slot.ts](../../channels/channel-slot.ts): `getChannel` / `setChannel` — the shared channel install surface
7171
- [service-transport.ts](./service-transport.ts): shared channel transport — wraps commands to broadcast, wires the sync-start initialization + patch listeners (hub or leaf), and runs the remote-command-execution protocol
72-
- [service-sync.ts](./service-sync.ts): last-write-wins ordering, the `deepReconcile` structural merge, and the per-service snapshot reconciler
72+
- [service-sync.ts](./service-sync.ts): last-write-wins ordering, `applyStatePatch` structural state application, and the per-service snapshot reconciler
7373
- [use-service-query.ts](./use-service-query.ts): `useServiceQuery` React hook backed by `useSyncExternalStore`
7474
- [use-service-command.ts](./use-service-command.ts): `useServiceCommand` React hook returning a stable command reference
7575
- [fixtures.ts](./fixtures.ts): scenario fixtures used by the test suite
@@ -338,10 +338,10 @@ an arbitrary style choice:
338338
- **Reactivity.** State is wrapped in a `deepSignal` proxy for fine-grained per-field tracking, and
339339
`deepSignal` throws (`"this object can't be observed"`) on scalars, `null`, and `undefined` — there
340340
are no fields to track on a scalar.
341-
- **Sync.** Cross-peer reconciliation (`deepReconcile` in [service-sync.ts](./service-sync.ts)) merges
341+
- **Sync.** Cross-peer reconciliation (`applyStatePatch` in [service-sync.ts](./service-sync.ts)) merges
342342
state by walking object keys; it has no concept of replacing a whole scalar.
343343

344-
Arrays are a special case: `deepSignal` *can* observe them, but `deepReconcile` replaces arrays
344+
Arrays are a special case: `deepSignal` *can* observe them, but `applyStatePatch` replaces arrays
345345
wholesale rather than merging by key, so a **top-level** array state would silently fail to sync
346346
between peers. They are therefore rejected too. Wrap collections in a field instead:
347347

@@ -422,7 +422,7 @@ If multiple tasks resolve to the same path, their states are deep-merged.
422422

423423
`writeOpenServiceStaticFiles(outputDir)` then writes those logical paths underneath `<outputDir>/services`, converting slash-separated logical keys into native filesystem paths for the current operating system.
424424

425-
These snapshots are currently only a build artifact for the server-side static build flow. This slice does not implement a separate runtime mode that consumes prebuilt snapshot stores instead of running `load` normally.
425+
In a static Storybook build (`CONFIG_TYPE === 'PRODUCTION'`), manager and preview runtimes pass a browser static loader into `registerService`. For every query that declares `staticPath`, the runtime swaps the authored `load` hook for a fetch of `./services/<serviceId>/<staticPath>` relative to the served HTML root (the same convention as `./index.json`), then applies the JSON snapshot via `applyStatePatch(..., { preserveMissingKeys: true })` so snapshots for one query input do not delete state populated by other inputs. Development mode keeps running `load` normally against the dev server.
426426

427427
Static path rules:
428428

@@ -489,9 +489,9 @@ Every channel event carries the emitter's `clientId` (generated per `registerSer
489489

490490
Incoming state (from sync-start-reply or patches) is applied via `serviceRuntime.commandSelf.setState(...)` directly — not through the wrapped commands — so no broadcast is triggered for received state.
491491

492-
### `deepReconcile`
492+
### `applyStatePatch`
493493

494-
Rather than replacing the entire state object on each patch (which would invalidate all signal subscriptions), `deepReconcile` (in [service-sync.ts](./service-sync.ts)) recursively merges plain-object values in place: arrays and primitives are replaced directly, keys absent from the snapshot are deleted so deletions propagate, and `__proto__`/`constructor`/`prototype` are skipped to block prototype pollution. This keeps fine-grained subscriptions on unaffected nested fields from firing spuriously.
494+
Rather than replacing the entire state object on each patch (which would invalidate all signal subscriptions), `applyStatePatch` (in [service-sync.ts](./service-sync.ts)) recursively merges plain-object values in place: arrays and primitives are replaced directly, `__proto__`/`constructor`/`prototype` are skipped to block prototype pollution, and `preserveMissingKeys` controls whether missing keys are deleted. Cross-peer sync passes `false` so deletions propagate from full snapshots; static JSON loading passes `true` because each static file is a partial snapshot. This keeps fine-grained subscriptions on unaffected nested fields from firing spuriously.
495495

496496
### State sync sequence
497497

@@ -635,9 +635,11 @@ The manager is connected to both the dev server and the preview, so it can invok
635635
in either; but a preview cannot directly invoke a server-only command, and vice versa — route such
636636
calls through the manager, or implement the command on a directly-connected peer.
637637

638-
There is **no timeout**. Invoking a command that no reachable peer implements leaves the promise
639-
pending forever, until the service is unregistered — `disconnect` rejects every outstanding call with
640-
`OpenServiceRemoteCommandDisconnectedError`.
638+
If no peer emits `services:command-ack` within a short window, the requester
639+
rejects with `OpenServiceRemoteCommandUnhandledError` — the common case in a static build when a
640+
query's `load` calls a server-only command. Once a peer acknowledges, the requester waits for
641+
`services:command-result` or `services:command-error` as before. Unregistering the service still
642+
rejects outstanding calls with `OpenServiceRemoteCommandDisconnectedError`.
641643

642644
## React Hooks
643645

code/core/src/shared/open-service/manager.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121

2222
import { getService, registerService as registerServiceCore } from './service-registry.ts';
23+
import { createBrowserStaticLoader } from './static-fetch.ts';
2324
import type {
2425
Commands,
2526
Queries,
@@ -48,5 +49,8 @@ export function registerService<
4849
definition: ServiceDefinition<TState, TQueries, TCommands>,
4950
registration?: ServiceRegistrationOptions<TState, TQueries, TCommands>
5051
): ServiceInstance<TState, TQueries, TCommands> & ServiceRegistryApi {
51-
return registerServiceCore(definition, registration, { relay: true });
52+
return registerServiceCore(definition, registration, {
53+
relay: true,
54+
staticLoader: createBrowserStaticLoader(),
55+
});
5256
}

code/core/src/shared/open-service/preview.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
*/
2323

2424
import { getService, registerService as registerServiceCore } from './service-registry.ts';
25+
import { createBrowserStaticLoader } from './static-fetch.ts';
2526
import type {
2627
Commands,
2728
Queries,
@@ -47,5 +48,7 @@ export function registerService<
4748
definition: ServiceDefinition<TState, TQueries, TCommands>,
4849
registration?: ServiceRegistrationOptions<TState, TQueries, TCommands>
4950
): ServiceInstance<TState, TQueries, TCommands> & ServiceRegistryApi {
50-
return registerServiceCore(definition, registration);
51+
return registerServiceCore(definition, registration, {
52+
staticLoader: createBrowserStaticLoader(),
53+
});
5154
}

code/core/src/shared/open-service/service-channel.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ export type CommandInvokePayload = v.InferOutput<typeof commandInvokeSchema>;
8989
/**
9090
* Emitted by an implementing peer the moment it accepts a `services:command-invoke`.
9191
*
92-
* Purely informational: it tells observers (and the requester) that at least one peer has picked
93-
* the call up. The requester still resolves/rejects only on the result/error reply.
92+
* Requesters use this to detect that at least one peer will run the command; if no ack arrives
93+
* within a short window, the request rejects as unhandled. The requester still resolves/rejects on
94+
* the result/error reply once a peer has acknowledged.
9495
*/
9596
export const commandAckSchema = v.object({
9697
serviceId: v.string(),

code/core/src/shared/open-service/service-command-transport.test.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
} from './service-channel.ts';
2828
import { deserializeError } from './service-error-serialization.ts';
2929
import { clearRegistry, registerService, unregisterService } from './service-registry.ts';
30+
import { REMOTE_COMMAND_ACK_TIMEOUT_MS } from './service-transport.ts';
3031
import { createTestChannel, installTestChannel } from '../../channels/test-channel.ts';
3132

3233
const remoteOnlyServiceDef = defineService({
@@ -88,6 +89,7 @@ function emittedCalls(channel: ReturnType<typeof createTestChannel>, event: stri
8889
}
8990

9091
afterEach(() => {
92+
vi.useRealTimers();
9193
clearRegistry();
9294
installTestChannel(null);
9395
});
@@ -98,7 +100,6 @@ describe('remote command requester (no local handler)', () => {
98100
installTestChannel(channel);
99101

100102
const service = registerService(remoteOnlyServiceDef);
101-
// Never settled in this test; swallow the unregister rejection clearRegistry triggers in afterEach.
102103
service.commands.doThing({ value: 'hi' }).catch(() => {});
103104

104105
const invokes = emittedCalls(channel, SERVICE_COMMAND_INVOKE);
@@ -110,6 +111,8 @@ describe('remote command requester (no local handler)', () => {
110111
callId: expect.any(String),
111112
clientId: expect.any(String),
112113
});
114+
115+
unregisterService(remoteOnlyServiceDef.id);
113116
});
114117

115118
it('resolves with the result of the matching command-result reply', async () => {
@@ -214,6 +217,50 @@ describe('remote command requester (no local handler)', () => {
214217
await expect(promise).resolves.toBe('correct');
215218
});
216219

220+
it('rejects when no peer acknowledges the invoke within the timeout', async () => {
221+
vi.useFakeTimers();
222+
223+
const channel = createTestChannel();
224+
installTestChannel(channel);
225+
226+
const service = registerService(remoteOnlyServiceDef);
227+
const promise = service.commands.doThing({ value: 'hi' });
228+
const assertion = expect(promise).rejects.toThrow(
229+
'No runtime acknowledged remote command "internal-fixture/remote-only-command.doThing"; its handler is not implemented in any connected runtime.'
230+
);
231+
232+
await vi.advanceTimersByTimeAsync(REMOTE_COMMAND_ACK_TIMEOUT_MS);
233+
await assertion;
234+
});
235+
236+
it('does not reject after a peer acknowledges the invoke', async () => {
237+
vi.useFakeTimers();
238+
239+
const channel = createTestChannel();
240+
installTestChannel(channel);
241+
242+
const service = registerService(remoteOnlyServiceDef);
243+
const promise = service.commands.doThing({ value: 'hi' });
244+
245+
const { callId } = emittedCalls(channel, SERVICE_COMMAND_INVOKE)[0][1] as CommandInvokePayload;
246+
channel.emitExternal(SERVICE_COMMAND_ACK, {
247+
serviceId: remoteOnlyServiceDef.id,
248+
callId,
249+
clientId: 'peer',
250+
});
251+
252+
await vi.advanceTimersByTimeAsync(REMOTE_COMMAND_ACK_TIMEOUT_MS);
253+
254+
channel.emitExternal(SERVICE_COMMAND_RESULT, {
255+
serviceId: remoteOnlyServiceDef.id,
256+
callId,
257+
result: 'done',
258+
clientId: 'peer',
259+
});
260+
261+
await expect(promise).resolves.toBe('done');
262+
});
263+
217264
it('rejects in-flight remote calls when the service is unregistered', async () => {
218265
const channel = createTestChannel();
219266
installTestChannel(channel);

0 commit comments

Comments
 (0)