Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { isChromatic } from './isChromatic';
sb.mock('../core/template/stories/test/ModuleMocking.utils.ts');
sb.mock('../core/template/stories/test/ModuleSpyMocking.utils.ts', { spy: true });
sb.mock('../core/template/stories/test/ModuleAutoMocking.utils.ts');
sb.mock('../core/template/stories/test/ClearModuleMocksMocking.api.ts', { spy: true });
/* eslint-disable depend/ban-dependencies */
sb.mock(import('lodash-es'));
sb.mock(import('lodash-es/add'));
Expand Down
16 changes: 15 additions & 1 deletion code/core/src/mocking-utils/automock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ const __vitest_current_es_module__ = {
}
const __vitest_mocked_module__ = globalThis[${globalThisAccessor}].mockObject(__vitest_current_es_module__, "${mockType}")
`;

// Register module mock spies in the global registry so that clearAllMocks/resetAllMocks/
// restoreAllMocks from storybook/test can find and clear them. This is needed because the
// module mocker may use a different @vitest/spy instance than the one bundled with storybook/test.
const spyRegistration = `
if (!globalThis.__STORYBOOK_MODULE_MOCK_SPIES__) { globalThis.__STORYBOOK_MODULE_MOCK_SPIES__ = new Set(); }
for (const __vitest_key__ of Object.keys(__vitest_mocked_module__)) {
const __vitest_val__ = __vitest_mocked_module__[__vitest_key__];
if (__vitest_val__ && typeof __vitest_val__ === "function" && __vitest_val__._isMockFunction === true) {
globalThis.__STORYBOOK_MODULE_MOCK_SPIES__.add(__vitest_val__);
}
}
`;

const assigning = allSpecifiers
.map(({ name }, index) => {
return `const __vitest_mocked_${index}__ = __vitest_mocked_module__["${name}"]`;
Expand All @@ -187,6 +201,6 @@ export {
${redeclarations}
}
`;
m.append(moduleObject + assigning + specifiersExports);
m.append(moduleObject + spyRegistration + assigning + specifiersExports);
return m;
}
20 changes: 20 additions & 0 deletions code/core/src/test/spy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,19 @@ export type * from '@vitest/spy';

export { isMockFunction, mocks };

/**
* Global registry for module mock spies created by `sb.mock('...', { spy: true })`.
*
* These spies are created by the module mocker (via `__vitest_mocker__.mockObject()`) and may use a
* different `@vitest/spy` instance than the one bundled with storybook/test. This means they won't
* appear in the `mocks` Set that `clearAllMocks`/`resetAllMocks`/`restoreAllMocks` iterate over.
*
* The automock code generation registers spies here so they can be properly cleared between
* stories.
*/
const moduleMockSpies: Set<MockInstance> = ((globalThis as any).__STORYBOOK_MODULE_MOCK_SPIES__ ??=
new Set<MockInstance>());

type Listener = (mock: MockInstance, args: unknown[]) => void;
const listeners = new Set<Listener>();

Expand Down Expand Up @@ -63,6 +76,7 @@ function listenWhenCalled(mock: MockInstance) {
*/
export function clearAllMocks() {
mocks.forEach((spy) => spy.mockClear());
moduleMockSpies.forEach((spy) => spy.mockClear());
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

These forEach callbacks trip Biome's iterable-callback-return rule.

Biome is already flagging these new lines because the concise arrows return the mock instance expression. Use block bodies so the callbacks return void.

🧹 Proposed fix
-  moduleMockSpies.forEach((spy) => spy.mockClear());
+  moduleMockSpies.forEach((spy) => {
+    spy.mockClear();
+  });
...
-  moduleMockSpies.forEach((spy) => spy.mockReset());
+  moduleMockSpies.forEach((spy) => {
+    spy.mockReset();
+  });
...
-  moduleMockSpies.forEach((spy) => spy.mockClear());
+  moduleMockSpies.forEach((spy) => {
+    spy.mockClear();
+  });

Also applies to: 91-91, 104-104

🧰 Tools
🪛 Biome (2.4.4)

[error] 79-79: This callback passed to forEach() iterable method should not return a value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/test/spy.ts` at line 79, The arrow callbacks with concise
bodies return the mock instance and trigger Biome's iterable-callback-return
rule; update all usages of moduleMockSpies.forEach((spy) => spy.mockClear())
(and the similar forEach calls around the same area) to use block-bodied arrow
callbacks that call spy.mockClear(); with no return (e.g.,
moduleMockSpies.forEach((spy) => { spy.mockClear(); })), ensuring the callback
returns void.

}

/**
Expand All @@ -74,6 +88,7 @@ export function clearAllMocks() {
*/
export function resetAllMocks() {
mocks.forEach((spy) => spy.mockReset());
moduleMockSpies.forEach((spy) => spy.mockReset());
}

/**
Expand All @@ -82,6 +97,11 @@ export function resetAllMocks() {
*/
export function restoreAllMocks() {
mocks.forEach((spy) => spy.mockRestore());
// For module mock spies, we only clear call history (not restore), because:
// - mockRestore() would try to undo the spyOn on the module export object, which is not
// meaningful for automocked modules where the spy reference is captured at module load time
// - The spy needs to remain active for subsequent stories
moduleMockSpies.forEach((spy) => spy.mockClear());
Comment on lines 98 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Vitest/@vitest/spy, how does mockRestore()behave for spies created viaspyOnorvi.mock(..., { spy: true })? Is there a supported way to restore the original implementation while preserving the same spy function reference?

💡 Result:

  • vi.spyOn(obj, 'method') spies: spy.mockRestore() restores the original property on obj (original descriptor / function) and clears spy state. After that, obj.method is no longer the spy (the reference changes), so the old spy object is effectively “detached” from future calls. (vitest.dev)

  • vi.mock(path, { spy: true }) (“spied modules”): Vitest replaces exports with spy wrappers that still call the original implementation by default (so you can assert calls/returns without changing behavior). (vitest.dev)
    If you then override an export (e.g. vi.mocked(mod.fn).mockReturnValue(...)), calling .mockRestore() on that spy follows the same rule: it restores the original implementation, which implies dropping the spy wrapper rather than keeping the same spy function identity. (Vitest’s “restore” concept is “put the original back”, not “keep spying but behave like original”.) (vitest.dev)

  • Supported way to restore original implementation while preserving the same spy function reference?
    No. “Restore” in Vitest means restoring the original method/export (descriptor), which necessarily breaks identity with the spy reference; Vitest explicitly notes that once mocks are restored, you can’t keep using the old spy handle to affect the restored method—you must spy again. (vitest.dev)

Practical workaround (keeps the same spy reference, but does not truly restore):

  • Capture the original implementation first, then make the spy call through to it:
    • const original = obj.method
    • const spy = vi.spyOn(obj, 'method')
    • later: spy.mockImplementation(original) (optionally spy.mockClear() to drop history)
      This preserves the spy function reference and behavior, but it’s not a “restore” (the property remains a spy). (vitest.dev)

Citations:


🏁 Script executed:

cd code/core/src && find . -name "spy.ts" -o -name "spy.js" | head -5

Repository: storybookjs/storybook

Length of output: 78


🏁 Script executed:

cat -n code/core/src/test/spy.ts | head -120

Repository: storybookjs/storybook

Length of output: 5438


🏁 Script executed:

cat -n code/core/src/test/spy.ts | sed -n '85,115p'

Repository: storybookjs/storybook

Length of output: 1579


🏁 Script executed:

cd code && rg '__STORYBOOK_MODULE_MOCK_SPIES__' --type ts --type js -B2 -A2

Repository: storybookjs/storybook

Length of output: 1189


🏁 Script executed:

cd code && rg "moduleMockSpies" --type ts --type js -B2 -A2

Repository: storybookjs/storybook

Length of output: 1118


🏁 Script executed:

cd code && rg "vi\.mock\(" --type ts --type js --max-count=10 | head -20

Repository: storybookjs/storybook

Length of output: 1854


🏁 Script executed:

cd code/core/src/mocking-utils && cat -n automock.ts | head -80

Repository: storybookjs/storybook

Length of output: 3883


🏁 Script executed:

cd code/core/src/mocking-utils && cat -n automock.ts | sed -n '100,180p'

Repository: storybookjs/storybook

Length of output: 4023


🏁 Script executed:

cd code/core/src/mocking-utils && cat -n automock.ts | sed -n '175,200p'

Repository: storybookjs/storybook

Length of output: 1377


🏁 Script executed:

cd code && rg "mockImplementation\|mockResolvedValue" --type ts --type tsx -c | head -10

Repository: storybookjs/storybook

Length of output: 92


🏁 Script executed:

cd code && rg "mockImplementation|mockResolvedValue" --type ts -c | head -10

Repository: storybookjs/storybook

Length of output: 593


🏁 Script executed:

cd code && rg "mockReset" --type ts -A3 -B1 | head -40

Repository: storybookjs/storybook

Length of output: 2448


restoreAllMocks() leaks overridden module-spy implementations between stories.

Calling mockClear() on module mock spies only clears call history but preserves any mockImplementation or mockResolvedValue overrides set during a story. This causes the previous story's mock implementation to persist into the next story, breaking story isolation.

Consider using mockReset() instead for moduleMockSpies in restoreAllMocks(), since mockReset() clears state and resets implementations (whereas mockRestore() would incorrectly attempt to drop the spy wrapper entirely for automocked modules).

🧰 Tools
🪛 Biome (2.4.4)

[error] 99-99: This callback passed to forEach() iterable method should not return a value.

(lint/suspicious/useIterableCallbackReturn)


[error] 104-104: This callback passed to forEach() iterable method should not return a value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/core/src/test/spy.ts` around lines 98 - 104, The restoreAllMocks()
implementation currently only calls mockClear() on moduleMockSpies which
preserves any mockImplementation/mockResolvedValue between stories; change the
moduleMockSpies cleanup to call mockReset() (not mockRestore()) so each module
spy's implementation and call state are fully reset between stories, while
leaving the existing mocks.forEach((spy) => spy.mockRestore()) behavior for
non-module spies intact.

Copy link
Member

Choose a reason for hiding this comment

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

@valentinpalkovic can you verify?

}

/**
Expand Down
17 changes: 17 additions & 0 deletions code/core/template/stories/test/ClearModuleMocksMocking.api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export type Data = {
userId: number;
id: number;
title: string;
body: string;
};

export const fetchData = async (): Promise<Data[]> => {
return Promise.resolve([
{
userId: 1,
id: 1,
title: 'mocked title',
body: 'mocked body',
},
]);
};
51 changes: 51 additions & 0 deletions code/core/template/stories/test/ClearModuleMocksMocking.stories.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Replace your-framework with the framework you are using, e.g. react-vite, nextjs, nextjs-vite, etc.
import { global as globalThis } from '@storybook/global';

import { clearAllMocks, expect, waitFor } from 'storybook/test';

import { fetchData } from './ClearModuleMocksMocking.api';

/**
* The purpose of this story is to verify that the `clearAllMocks` function properly clears mocks
* created with the `spy: true` option in `sb.mock()`. This is necessary because those mocks are
* created with a different instance of `@vitest/spy` than the one bundled with storybook/test. This
* means they won't be cleared by the `clearMocks` option of Vitest, and we need to use
* `clearAllMocks` to clear them manually. See issue:
* https://github.com/storybookjs/storybook/issues/34075
*/
const meta = {
component: globalThis.__TEMPLATE_COMPONENTS__.Button,
args: {
label: 'Fetch Data',
onClick: () => {
fetchData();
},
},
beforeEach: async () => {
clearAllMocks();
},
};

export default meta;

export const First = {
args: {},
play: async ({ canvas }: any) => {
const button = await canvas.getByRole('button');
await button.click();
await waitFor(() => {
expect(fetchData).toHaveBeenCalledTimes(1);
});
},
};

export const Second = {
args: {},
play: async ({ canvas }: any) => {
const button = await canvas.getByRole('button');
await button.click();
await waitFor(() => {
expect(fetchData).toHaveBeenCalledTimes(1);
});
},
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { global as globalThis } from '@storybook/global';

import { expect } from 'storybook/test';
import { v4 } from 'uuid';

import { fn } from './ModuleAutoMocking.utils';

Expand Down
1 change: 1 addition & 0 deletions scripts/tasks/sandbox-parts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ export const extendPreview: Task['run'] = async ({ template, sandboxDir }) => {
"sb.mock('../template-stories/core/test/ModuleMocking.utils.ts');",
"sb.mock('../template-stories/core/test/ModuleSpyMocking.utils.ts', { spy: true });",
"sb.mock('../template-stories/core/test/ModuleAutoMocking.utils.ts');",
"sb.mock('../template-stories/core/test/ClearModuleMocksMocking.api.ts', { spy: true });",
"sb.mock(import('lodash-es'));",
"sb.mock(import('lodash-es/add'));",
"sb.mock(import('lodash-es/sum'));",
Expand Down
Loading