Skip to content

Commit a5177c4

Browse files
authored
Merge pull request storybookjs#35158 from storybookjs/jeppe-cursor/docgen-hot-update-e2e-1f53
Open Service: Fix docgen hot update controls refresh
2 parents 3de0692 + 47dedcf commit a5177c4

9 files changed

Lines changed: 204 additions & 29 deletions

File tree

code/.storybook/main.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,9 @@ const config = defineMain({
156156
features: {
157157
developmentModeForBuild: true,
158158
experimentalTestSyntax: true,
159-
// Disabled for now: the docgen service does not yet work in production builds. Keeping it off
160-
// ensures this branch exercises the normal (non-experimental) docgen path without regressions.
161-
experimentalDocgenServer: false,
159+
// Disabled by default for production builds; the internal dev server enables it via
160+
// STORYBOOK_EXPERIMENTAL_DOCGEN_SERVER so hot-update e2e covers the open-service path.
161+
experimentalDocgenServer: process.env.STORYBOOK_EXPERIMENTAL_DOCGEN_SERVER === 'true',
162162
experimentalReactComponentMeta: true,
163163
changeDetection: true,
164164
},

code/core/src/shared/open-service/service-runtime.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,34 @@ describe('service runtime', () => {
8080
unsubscribe();
8181
});
8282

83+
it('emits detached snapshots when nested state changes', async () => {
84+
const service = registerService(mutableRecordLookupServiceDef);
85+
const calls: Array<Record<string, string> | null> = [];
86+
87+
const unsubscribe = service.queries.getRecordFields.subscribe(
88+
{ entryId: 'entry-a' },
89+
(value) => {
90+
calls.push(value);
91+
}
92+
);
93+
94+
await vi.waitFor(() => expect(calls).toEqual([null]));
95+
await service.commands.assignRecordField({
96+
entryId: 'entry-a',
97+
fieldKey: 'marker',
98+
fieldValue: 'first',
99+
});
100+
await service.commands.assignRecordField({
101+
entryId: 'entry-a',
102+
fieldKey: 'marker',
103+
fieldValue: 'second',
104+
});
105+
106+
expect(calls).toEqual([null, { marker: 'first' }, { marker: 'second' }]);
107+
expect(calls[1]).not.toBe(calls[2]);
108+
unsubscribe();
109+
});
110+
83111
it('stops notifying after unsubscribe', async () => {
84112
const service = registerService(mutableRecordLookupServiceDef);
85113
const calls: Array<Record<string, string> | null> = [];

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ function subscribeToQuery<TState>(
11021102
selector(output);
11031103
return detachSnapshot(selector(validated));
11041104
}
1105-
return validateQueryOutput(refs, queryName, queryDef, output);
1105+
return detachSnapshot(validateQueryOutput(refs, queryName, queryDef, output));
11061106
});
11071107

11081108
let hasEmitted = false;

code/core/src/shared/open-service/services/docgen/server.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
22

33
import { Tag } from '../../../../shared/constants/tags.ts';
44
import type { DocsIndexEntry, IndexEntry, StoryIndex } from '../../../../types/modules/indexer.ts';
5-
import { buildStaticFiles, clearRegistry } from '../../server.ts';
5+
import { buildStaticFiles, clearRegistry, getService } from '../../server.ts';
6+
import type { ModuleGraphService } from '../module-graph/definition.ts';
67
import { registerTestModuleGraphService } from '../module-graph/module-graph.test-helpers.ts';
78
import { registerDocgenService } from './server.ts';
89
import type { DocgenPayload, DocgenProvider } from './types.ts';
@@ -195,6 +196,39 @@ describe('docgen open service', () => {
195196
});
196197
});
197198

199+
describe('module graph hot refresh', () => {
200+
it('refreshes already-extracted components without loading every bumped component', async () => {
201+
const buttonEntry = makeStoryEntry('button--primary', 'Button');
202+
const cardEntry = makeStoryEntry('card--primary', 'Card');
203+
const provider = vi.fn<DocgenProvider>(async ({ entry }) =>
204+
makeDocgenPayload({
205+
id: entry.id.split('--')[0],
206+
name: entry.title,
207+
path: entry.importPath,
208+
})
209+
);
210+
const service = registerDocgenService({
211+
getIndex: makeGetIndex([buttonEntry, cardEntry]),
212+
provider,
213+
});
214+
215+
await service.queries.getDocgen.loaded({ id: 'button' });
216+
217+
const moduleGraph = getService<ModuleGraphService>('core/module-graph');
218+
await moduleGraph.commands._applyGraphUpdate({
219+
storiesByFile: {},
220+
bumpedStoryFiles: ['./button.stories.tsx', './card.stories.tsx'],
221+
});
222+
223+
await vi.waitFor(() =>
224+
expect(provider.mock.calls.map(([input]) => input.entry.importPath)).toEqual([
225+
'./button.stories.tsx',
226+
'./button.stories.tsx',
227+
])
228+
);
229+
});
230+
});
231+
198232
describe('static build', () => {
199233
it('does not request docgen for component ids that only exist on unattached docs entries', async () => {
200234
const storyEntry = makeStoryEntry('button--primary', 'Button');

code/core/src/shared/open-service/services/docgen/server.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { getService, registerService } from '../../server.ts';
88
import type { ModuleGraphService } from '../module-graph/definition.ts';
99
import { toStoryIndexPath } from '../module-graph/types.ts';
1010
import { docgenServiceDef } from './definition.ts';
11-
import type { DocgenPayload, DocgenProvider } from './types.ts';
11+
import type { DocgenProvider } from './types.ts';
1212

1313
export type RegisterDocgenServiceOptions = {
1414
workingDir?: string;
@@ -41,6 +41,7 @@ export type RegisterDocgenServiceOptions = {
4141
*/
4242
export function registerDocgenService(options: RegisterDocgenServiceOptions) {
4343
const workingDir = options.workingDir ?? process.cwd();
44+
const extractedComponentIds = new Set<string>();
4445

4546
const runtime = registerService(docgenServiceDef, {
4647
queries: {
@@ -77,6 +78,7 @@ export function registerDocgenService(options: RegisterDocgenServiceOptions) {
7778
ctx.self.setState((state) => {
7879
state.components[input.id] = payload;
7980
});
81+
extractedComponentIds.add(input.id);
8082
return payload;
8183
},
8284
},
@@ -137,7 +139,7 @@ export function registerDocgenService(options: RegisterDocgenServiceOptions) {
137139
// Only refresh components that already have extracted docgen in service state, so we never
138140
// eagerly extract docgen nobody has requested yet.
139141
const componentIdsToRefresh = Array.from(bumpedComponentIds).filter((id) => {
140-
return runtime.queries.getDocgen({ id }) !== undefined;
142+
return extractedComponentIds.has(id);
141143
});
142144

143145
if (componentIdsToRefresh.length === 0) {

code/core/src/shared/open-service/use-service-query.test.tsx

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,27 +134,34 @@ describe('useServiceQuery', () => {
134134
expect(subscribeSpy.mock.calls.length).toBe(subscribeCallsAfterMount);
135135
});
136136

137-
it('maintains referential stability when result is deeply equal', async () => {
138-
const service = registerService(mutableRecordLookupServiceDef);
139-
140-
await service.commands.assignRecordField({ entryId: 'a', fieldKey: 'k', fieldValue: 'v' });
141-
137+
it('uses emitted snapshots without deep-equal output filtering', async () => {
138+
const subscribers: Array<(value: { k: string }) => void> = [];
139+
const getRecordFields = Object.assign(
140+
vi.fn(() => ({ k: 'v' })),
141+
{
142+
subscribe: vi.fn(
143+
(_input: { entryId: string }, callback: (value: { k: string }) => void) => {
144+
subscribers.push(callback);
145+
return () => {};
146+
}
147+
),
148+
}
149+
);
150+
const service = { queries: { getRecordFields } };
142151
let renderCount = 0;
143152
const { result } = renderHook(() => {
144153
renderCount++;
145-
return useServiceQuery(service, 'getRecordFields', { entryId: 'a' });
154+
return useServiceQuery(service, 'getRecordFields', { entryId: 'a' }) as { k: string };
146155
});
147156

148157
const firstRef = result.current;
149158
const countAfterMount = renderCount;
150159

151-
// Assign the same value again — deeply equal, so no re-render.
152-
await service.commands.assignRecordField({ entryId: 'a', fieldKey: 'k', fieldValue: 'v' });
160+
await waitFor(() => expect(subscribers).toHaveLength(1));
161+
subscribers[0]({ k: 'v' });
153162

154-
// Wait a tick to let any spurious re-renders fire.
155-
await new Promise<void>((resolve) => setTimeout(resolve, 20));
156-
157-
expect(result.current).toBe(firstRef);
158-
expect(renderCount).toBe(countAfterMount);
163+
await waitFor(() => expect(renderCount).toBe(countAfterMount + 1));
164+
expect(result.current).toEqual(firstRef);
165+
expect(result.current).not.toBe(firstRef);
159166
});
160167
});

code/core/src/shared/open-service/use-service-query.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
* Backed by `useSyncExternalStore`, so it integrates correctly with React 18+ concurrent
55
* features and works in both manager and preview contexts without any adapter.
66
*
7-
* Re-renders only when the specific query result changes by value. Signal-level dedup
8-
* inside the service runtime ensures that a load which rewrites a deeply-equal payload does
9-
* not re-fire the subscription; `isEqual` in the subscription callback provides an additional
10-
* referential-stability layer at the React boundary so the component sees a stable object
11-
* reference across updates that return the same logical value.
7+
* Re-renders only when the subscribed service query emits a new snapshot. Signal-level dedup inside
8+
* the service runtime ensures that a load which rewrites a deeply-equal payload does not re-fire the
9+
* subscription; this hook intentionally trusts those emissions rather than deep-comparing outputs a
10+
* second time.
1211
*
1312
* Object inputs are compared with deep equality when deciding whether to re-subscribe, so inline
1413
* literals at the call site are safe.
@@ -79,9 +78,6 @@ export function useServiceQuery<TKey extends string, TInput, TOutput>(
7978
const subscribe = React.useCallback(
8079
(listener: () => void): (() => void) =>
8180
queryFn.subscribe(subscriptionKey.input, (value) => {
82-
if (isEqual(value, snapshotRef.current)) {
83-
return;
84-
}
8581
snapshotRef.current = value;
8682
listener();
8783
}),
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { readFile, writeFile } from 'node:fs/promises';
2+
import { join } from 'node:path';
3+
4+
import { expect, test } from '@playwright/test';
5+
import process from 'process';
6+
7+
import { PREVIEW_STORY_TIMEOUT, waitForPreviewReady } from './helpers.ts';
8+
9+
const storybookUrl = process.env.STORYBOOK_URL || 'http://localhost:6006';
10+
const runsAgainstDevServer = !['build', 'static'].includes(process.env.STORYBOOK_TYPE || 'dev');
11+
12+
const codeDir = process.cwd();
13+
const buttonSourcePath = join(codeDir, 'core/src/components/components/Button/Button.tsx');
14+
const storyPath = '/story/button-component--base';
15+
const hotUpdatePropName = 'e2eDocgenHotUpdateProp';
16+
const hotUpdatePropSource = `
17+
/**
18+
* E2E-only docgen hot update marker.
19+
*/
20+
${hotUpdatePropName}?: 'before' | 'after';
21+
`;
22+
23+
// Start the internal dev server with STORYBOOK_EXPERIMENTAL_DOCGEN_SERVER=true before running this
24+
// spec. CI sets that env var in the internal Storybook e2e job.
25+
let originalButtonSource: string | undefined;
26+
27+
async function restoreFile(path: string, contents: string) {
28+
if ((await readFile(path, 'utf8')) !== contents) {
29+
await writeFile(path, contents, 'utf8');
30+
}
31+
}
32+
33+
async function addHotUpdateProp() {
34+
const current = await readFile(buttonSourcePath, 'utf8');
35+
if (current.includes(hotUpdatePropName)) {
36+
return;
37+
}
38+
39+
const marker = ' shortcut?: API_KeyCollection;\n';
40+
expect(current, `Could not find ButtonProps insertion marker in ${buttonSourcePath}`).toContain(
41+
marker
42+
);
43+
44+
await writeFile(buttonSourcePath, current.replace(marker, `${marker}${hotUpdatePropSource}`));
45+
}
46+
47+
test.describe('docgen open service hot updates', () => {
48+
test.describe.configure({ mode: 'serial' });
49+
test.setTimeout(90_000);
50+
51+
test.beforeAll(async () => {
52+
test.skip(!runsAgainstDevServer, 'Docgen hot updates require the dev server file watcher.');
53+
54+
originalButtonSource = await readFile(buttonSourcePath, 'utf8');
55+
});
56+
57+
test.afterAll(async () => {
58+
if (originalButtonSource) {
59+
await restoreFile(buttonSourcePath, originalButtonSource);
60+
}
61+
});
62+
63+
test('updates manager Controls when a component prop type changes without navigation', async ({
64+
page,
65+
}) => {
66+
await restoreFile(buttonSourcePath, originalButtonSource!);
67+
68+
await page.goto(`${storybookUrl}/?path=${storyPath}`);
69+
await expect
70+
.poll(
71+
() =>
72+
page.evaluate(() =>
73+
Boolean(
74+
(
75+
globalThis as {
76+
FEATURES?: { experimentalDocgenServer?: boolean };
77+
}
78+
).FEATURES?.experimentalDocgenServer
79+
)
80+
),
81+
{ timeout: PREVIEW_STORY_TIMEOUT }
82+
)
83+
.toBe(true);
84+
await waitForPreviewReady(page);
85+
await expect(
86+
page.frameLocator('#storybook-preview-iframe').getByRole('button', { name: 'Button' })
87+
).toBeVisible({ timeout: PREVIEW_STORY_TIMEOUT });
88+
89+
await page.getByRole('tab', { name: 'Controls' }).click();
90+
const controlsPanel = page.getByRole('tabpanel', { name: 'Controls' });
91+
await expect(
92+
controlsPanel.getByRole('cell', { name: 'ariaLabel', exact: true }).first()
93+
).toBeVisible({
94+
timeout: PREVIEW_STORY_TIMEOUT,
95+
});
96+
await expect(controlsPanel.getByText(hotUpdatePropName)).toHaveCount(0);
97+
98+
try {
99+
await addHotUpdateProp();
100+
101+
await expect(
102+
controlsPanel.getByRole('cell', { name: hotUpdatePropName, exact: true }).first()
103+
).toBeVisible({ timeout: PREVIEW_STORY_TIMEOUT });
104+
} finally {
105+
await restoreFile(buttonSourcePath, originalButtonSource!);
106+
}
107+
});
108+
});

scripts/ci/common-jobs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ export const internalStorybookE2e = defineJob(
176176
name: 'Run internal Storybook',
177177
working_directory: 'code',
178178
background: true,
179-
command: 'yarn storybook:ui',
179+
command: 'STORYBOOK_EXPERIMENTAL_DOCGEN_SERVER=true yarn storybook:ui',
180180
},
181181
},
182182
server.wait(['6006']),

0 commit comments

Comments
 (0)