Skip to content

HMR: Fix race conditions causing stale play functions to fire on re-rendered stories #33930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
valentinpalkovic merged 3 commits into from
Feb 27, 2026
Merged

HMR: Fix race conditions causing stale play functions to fire on re-rendered stories #33930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cba4bde
Initial plan
Copilot Feb 25, 2026
c047ebe
Fix HMR race conditions: fix STORY_INDEX_INVALIDATED double-fire and …
Copilot Feb 25, 2026
a49d9e1
Fix linting
valentinpalkovic Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ yarn nx compile <package-name> -c production # Compile specific package
yarn lint # Run all linting checks (~4 min)
```

Fix linting on all touched files by running the following command before commiting:

```bash
yarn --cwd code lint:js:cmd <file> --fix
```

### Type Checking

```bash
Expand Down
24 changes: 8 additions & 16 deletions code/builders/builder-vite/src/codegen-modern-iframe-script.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ describe('generateModernIframeScriptCodeFromPreviews', () => {
window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;

if (import.meta.hot) {
import.meta.hot.on('vite:afterUpdate', () => {
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
});

import.meta.hot.accept('virtual:/@storybook/builder-vite/storybook-stories.js', (newModule) => {
// Cancel any running play function before patching in the new importFn
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
// importFn has changed so we need to patch the new one in
window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
});
Expand Down Expand Up @@ -57,11 +55,9 @@ describe('generateModernIframeScriptCodeFromPreviews', () => {
window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;

if (import.meta.hot) {
import.meta.hot.on('vite:afterUpdate', () => {
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
});

import.meta.hot.accept('virtual:/@storybook/builder-vite/storybook-stories.js', (newModule) => {
// Cancel any running play function before patching in the new importFn
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
// importFn has changed so we need to patch the new one in
window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
});
Expand Down Expand Up @@ -89,11 +85,9 @@ describe('generateModernIframeScriptCodeFromPreviews', () => {
window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;

if (import.meta.hot) {
import.meta.hot.on('vite:afterUpdate', () => {
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
});

import.meta.hot.accept('virtual:/@storybook/builder-vite/storybook-stories.js', (newModule) => {
// Cancel any running play function before patching in the new importFn
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
// importFn has changed so we need to patch the new one in
window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
});
Expand Down Expand Up @@ -121,11 +115,9 @@ describe('generateModernIframeScriptCodeFromPreviews', () => {
window.__STORYBOOK_STORY_STORE__ = window.__STORYBOOK_STORY_STORE__ || window.__STORYBOOK_PREVIEW__.storyStore;

if (import.meta.hot) {
import.meta.hot.on('vite:afterUpdate', () => {
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
});

import.meta.hot.accept('virtual:/@storybook/builder-vite/storybook-stories.js', (newModule) => {
// Cancel any running play function before patching in the new importFn
window.__STORYBOOK_PREVIEW__.channel.emit('storyHotUpdated');
// importFn has changed so we need to patch the new one in
window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
});
Expand Down
6 changes: 2 additions & 4 deletions code/builders/builder-vite/src/codegen-modern-iframe-script.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,9 @@ export async function generateModernIframeScriptCodeFromPreviews(options: {

return dedent`
if (import.meta.hot) {
import.meta.hot.on('vite:afterUpdate', () => {
window.__STORYBOOK_PREVIEW__.channel.emit('${STORY_HOT_UPDATED}');
});

import.meta.hot.accept('${SB_VIRTUAL_FILES.VIRTUAL_STORIES_FILE}', (newModule) => {
// Cancel any running play function before patching in the new importFn
window.__STORYBOOK_PREVIEW__.channel.emit('${STORY_HOT_UPDATED}');

@ghengeveld ghengeveld Feb 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks broken, since it's not a template string but a regular string.

// importFn has changed so we need to patch the new one in
window.__STORYBOOK_PREVIEW__.onStoriesChanged({ importFn: newModule.importFn });
});
Expand Down
5 changes: 5 additions & 0 deletions code/builders/builder-vite/src/codegen-project-annotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getFrameworkName, loadPreviewOrConfigFile } from 'storybook/internal/common';
import { STORY_HOT_UPDATED } from 'storybook/internal/core-events';
import { isCsfFactoryPreview, readConfig } from 'storybook/internal/csf-tools';
import type { Options, PreviewAnnotation } from 'storybook/internal/types';

Expand Down Expand Up @@ -68,6 +69,8 @@ export function generateProjectAnnotationsCodeFromPreviews(options: {

if (import.meta.hot) {
import.meta.hot.accept([${JSON.stringify(previewFileURL)}], (previewAnnotationModules) => {
// Cancel any running play function before patching in the new getProjectAnnotations
window?.__STORYBOOK_PREVIEW__?.channel?.emit('${STORY_HOT_UPDATED}');
// getProjectAnnotations has changed so we need to patch the new one in
window?.__STORYBOOK_PREVIEW__?.onGetProjectAnnotationsChanged({
getProjectAnnotations: () => getProjectAnnotations(previewAnnotationModules),
Expand Down Expand Up @@ -96,6 +99,8 @@ export function generateProjectAnnotationsCodeFromPreviews(options: {

if (import.meta.hot) {
import.meta.hot.accept(${JSON.stringify(previewAnnotationURLs)}, (previewAnnotationModules) => {
// Cancel any running play function before patching in the new getProjectAnnotations
window?.__STORYBOOK_PREVIEW__?.channel?.emit('${STORY_HOT_UPDATED}');
// getProjectAnnotations has changed so we need to patch the new one in
window?.__STORYBOOK_PREVIEW__?.onGetProjectAnnotationsChanged({
getProjectAnnotations: () => getProjectAnnotations(previewAnnotationModules),
Expand Down
10 changes: 4 additions & 6 deletions code/builders/builder-webpack5/templates/virtualModuleModernEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,16 @@ window.__STORYBOOK_STORY_STORE__ = preview.storyStore;
window.__STORYBOOK_ADDONS_CHANNEL__ = channel;

if (import.meta.webpackHot) {
import.meta.webpackHot.addStatusHandler((status) => {
if (status === 'idle') {
preview.channel.emit(STORY_HOT_UPDATED);
}
});

import.meta.webpackHot.accept('{{storiesFilename}}', () => {
// Cancel any running play function before patching in the new importFn
preview.channel.emit(STORY_HOT_UPDATED);
// importFn has changed so we need to patch the new one in
preview.onStoriesChanged({ importFn });
});

import.meta.webpackHot.accept(['{{previewAnnotations}}'], () => {
// Cancel any running play function before patching in the new getProjectAnnotations
preview.channel.emit(STORY_HOT_UPDATED);
// getProjectAnnotations has changed so we need to patch the new one in
preview.onGetProjectAnnotationsChanged({ getProjectAnnotations });
});
Expand Down
38 changes: 35 additions & 3 deletions code/core/src/core-server/utils/index-json.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,20 +644,52 @@ describe('registerIndexJsonRoute', () => {
onChange(`${workingDir}/src/nested/Button.stories.ts`);
onChange(`${workingDir}/src/nested/Button.stories.ts`);

// Wait for first batch to be processed and emit (leading edge)
// With trailing-only debounce, the first emit only fires after the debounce period
await vi.waitFor(() => {
expect(mockServerChannel.emit).toHaveBeenCalledTimes(1);
});
expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED);

// Fire another change event after the first batch is processed
// This will trigger the trailing edge of the debounce
// This will trigger the trailing edge of the debounce again
onChange(`${workingDir}/src/nested/Button.stories.ts`);

// Wait for trailing debounce to trigger second emit
// Wait for the trailing debounce to trigger a second emit
await vi.waitFor(() => {
expect(mockServerChannel.emit).toHaveBeenCalledTimes(2);
});
});

it('only emits once per file change (no double-fire from leading+trailing edges)', async () => {
vi.mocked(debounce).mockImplementation(
(await vi.importActual<typeof import('es-toolkit/function')>('es-toolkit/function'))
.debounce
);

const mockServerChannel = { emit: vi.fn() } as any as ServerChannel;
registerIndexJsonRoute({
app,
channel: mockServerChannel,
workingDir,
normalizedStories,
storyIndexGeneratorPromise: getStoryIndexGeneratorPromise(),
});

const watcher = Watchpack.mock.instances[0];
const onChange = watcher.on.mock.calls[0][1];

// Fire a single change event
onChange(`${workingDir}/src/nested/Button.stories.ts`);

// Wait for the trailing debounce to fire
await vi.waitFor(() => {
expect(mockServerChannel.emit).toHaveBeenCalledTimes(1);
});

// Ensure it was only called once (no double-fire from both leading and trailing edges)
await new Promise((resolve) => setTimeout(resolve, 200));
expect(mockServerChannel.emit).toHaveBeenCalledTimes(1);
Comment on lines +663 to +691

@coderabbitai coderabbitai Bot Feb 27, 2026

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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Check the exact code at lines 663-691
echo "=== Code at lines 663-691 ==="
sed -n '663,691p' code/core/src/core-server/utils/index-json.test.ts

echo -e "\n=== DEBOUNCE constant definition ==="
rg -n "const DEBOUNCE" code/core/src/core-server/utils/index-json.test.ts

Repository: storybookjs/storybook

Length of output: 1263

🏁 Script executed:

#!/bin/bash
# 2. Check for other mockImplementation calls in the test file
echo "=== All mockImplementation calls ==="
rg -n "mockImplementation\(" code/core/src/core-server/utils/index-json.test.ts

Repository: storybookjs/storybook

Length of output: 267

🏁 Script executed:

#!/bin/bash
# 3. Check the structure of beforeEach blocks in the file
echo "=== beforeEach blocks structure ==="
rg -n "beforeEach" code/core/src/core-server/utils/index-json.test.ts -A 5

Repository: storybookjs/storybook

Length of output: 722

🏁 Script executed:

#!/bin/bash
# Search for DEBOUNCE more broadly (could be imported)
echo "=== Search for DEBOUNCE usage ==="
rg -n "DEBOUNCE" code/core/src/core-server/utils/index-json.test.ts

echo -e "\n=== Also check the main implementation file for debounce timing ==="
rg -n "DEBOUNCE|debounce.*\d+" code/core/src/core-server/utils/index-json.ts | head -20

Repository: storybookjs/storybook

Length of output: 362

🏁 Script executed:

#!/bin/bash
# Check context at line 605 for mockImplementation
echo "=== Context around line 605 ==="
sed -n '600,620p' code/core/src/core-server/utils/index-json.test.ts

Repository: storybookjs/storybook

Length of output: 790

Move mock setup to beforeEach and derive debounce wait time from DEBOUNCE constant.

Lines 664–667 set mock behavior inside the test body, violating Vitest guidelines to implement mock behaviors in beforeEach blocks. Additionally, the hardcoded 200ms sleep at line 690 is arbitrary; it should derive from the DEBOUNCE constant (100ms) to keep the test self-documenting and maintainable.

Suggested adjustment 
-    it('only emits once per file change (no double-fire from leading+trailing edges)', async () => {
-      vi.mocked(debounce).mockImplementation(
-        (await vi.importActual<typeof import('es-toolkit/function')>('es-toolkit/function'))
-          .debounce
-      );
+    describe('single-emission behavior', () => {
+      beforeEach(async () => {
+        vi.mocked(debounce).mockImplementation(
+          (await vi.importActual<typeof import('es-toolkit/function')>('es-toolkit/function'))
+            .debounce
+        );
+      });
+
+      it('only emits once per file change (no double-fire from leading+trailing edges)', async () => {
         // ...
-      await new Promise((resolve) => setTimeout(resolve, 200));
+      await new Promise((resolve) => setTimeout(resolve, DEBOUNCE * 2));
       expect(mockServerChannel.emit).toHaveBeenCalledTimes(1);
+      });
+    });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/core-server/utils/index-json.test.ts` around lines 663 - 691,
Move the debounce mock setup out of the test body into the test suite's
beforeEach so mocking rules run in setup (replace the inline
vi.mocked(debounce).mockImplementation(...) currently in the test "only emits
once per file change..." with the same mock in beforeEach); also replace the
hardcoded 200ms sleep after triggering onChange(`${workingDir}/...`) with a wait
derived from the DEBOUNCE constant (e.g., use DEBOUNCE * 2 or similar) so the
assertion on mockServerChannel.emit (used by registerIndexJsonRoute and the
Watchpack onChange handler) waits an appropriate, self-documented duration.

expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED);
});
});
});
2 changes: 1 addition & 1 deletion code/core/src/core-server/utils/index-json.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function registerIndexJsonRoute({
normalizedStories: NormalizedStoriesSpecifier[];
}) {
const maybeInvalidate = debounce(() => channel.emit(STORY_INDEX_INVALIDATED), DEBOUNCE, {
edges: ['leading', 'trailing'],
edges: ['trailing'],
});
watchStorySpecifiers(normalizedStories, { workingDir }, async (path, removed) => {
(await storyIndexGeneratorPromise).invalidate(path, removed);
Expand Down
Loading