Merge pull request #26740 from storybookjs/kasper/reactive-spies

Test: Make spies reactive so that they can be logged by addon-actions

code/addons/actions/src/loaders.ts

@@ -1,36 +1,26 @@
 /* eslint-disable no-underscore-dangle */
 import type { LoaderFunction } from '@storybook/types';
+import { global } from '@storybook/global';
+import type { onMockCall as onMockCallType } from '@storybook/test';
 import { action } from './runtime';
 
-export const tinySpyInternalState = Symbol.for('tinyspy:spy');
+let subscribed = false;
 
-const attachActionsToFunctionMocks: LoaderFunction = (context) => {
+const logActionsWhenMockCalled: LoaderFunction = (context) => {
   const {
-    args,
     parameters: { actions },
   } = context;
   if (actions?.disable) return;
 
-  Object.entries(args)
-    .filter(
-      ([, value]) =>
-        typeof value === 'function' && '_isMockFunction' in value && value._isMockFunction
-    )
-    .forEach(([key, value]) => {
-      // See this discussion for context:
-      // https://github.com/vitest-dev/vitest/pull/5352
-      const previous =
-        value.getMockImplementation() ??
-        (tinySpyInternalState in value ? value[tinySpyInternalState]?.getOriginal() : undefined);
-      if (previous?._actionAttached !== true && previous?.isAction !== true) {
-        const implementation = (...params: unknown[]) => {
-          action(key)(...params);
-          return previous?.(...params);
-        };
-        implementation._actionAttached = true;
-        value.mockImplementation(implementation);
-      }
-    });
+  if (
+    !subscribed &&
+    '__STORYBOOK_TEST_ON_MOCK_CALL__' in global &&
+    typeof global.__STORYBOOK_TEST_ON_MOCK_CALL__ === 'function'
+  ) {
+    const onMockCall = global.__STORYBOOK_TEST_ON_MOCK_CALL__ as typeof onMockCallType;
+    onMockCall((mock, args) => action(mock.getMockName())(args));
+    subscribed = true;
+  }
 };
 
-export const loaders: LoaderFunction[] = [attachActionsToFunctionMocks];
+export const loaders: LoaderFunction[] = [logActionsWhenMockCalled];

code/addons/actions/template/stories/spies.stories.ts

@@ -0,0 +1,24 @@
+import { global as globalThis } from '@storybook/global';
+import { spyOn } from '@storybook/test';
+
+export default {
+  component: globalThis.Components.Button,
+  loaders() {
+    spyOn(console, 'log').mockName('console.log');
+  },
+  args: {
+    label: 'Button',
+  },
+  parameters: {
+    chromatic: { disable: true },
+  },
+};
+
+export const ShowSpyOnInActions = {
+  args: {
+    onClick: () => {
+      console.log('first');
+      console.log('second');
+    },
+  },
+};

code/addons/interactions/src/preview.ts

@@ -1,11 +1,4 @@
-import type {
-  ArgsEnhancer,
-  PlayFunction,
-  PlayFunctionContext,
-  Renderer,
-  StepLabel,
-} from '@storybook/types';
-import { fn, isMockFunction } from '@storybook/test';
+import type { PlayFunction, PlayFunctionContext, StepLabel } from '@storybook/types';
 import { instrument } from '@storybook/instrumenter';
 
 export const { step: runStep } = instrument(
@@ -16,50 +9,6 @@ export const { step: runStep } = instrument(
   { intercept: true }
 );
 
-export const traverseArgs = (value: unknown, depth = 0, key?: string): unknown => {
-  // Make sure to not get in infinite loops with self referencing args
-  if (depth > 5) return value;
-  if (value == null) return value;
-  if (isMockFunction(value)) {
-    // Makes sure we get the arg name in the interactions panel
-    if (key) value.mockName(key);
-    return value;
-  }
-
-  // wrap explicit actions in a spy
-  if (
-    typeof value === 'function' &&
-    'isAction' in value &&
-    value.isAction &&
-    !('implicit' in value && value.implicit)
-  ) {
-    const mock = fn(value as any);
-    if (key) mock.mockName(key);
-    return mock;
-  }
-
-  if (Array.isArray(value)) {
-    depth++;
-    return value.map((item) => traverseArgs(item, depth));
-  }
-
-  if (typeof value === 'object' && value.constructor === Object) {
-    depth++;
-    for (const [k, v] of Object.entries(value)) {
-      if (Object.getOwnPropertyDescriptor(value, k).writable) {
-        // We have to mutate the original object for this to survive HMR.
-        (value as Record<string, unknown>)[k] = traverseArgs(v, depth, k);
-      }
-    }
-    return value;
-  }
-  return value;
-};
-
-const wrapActionsInSpyFns: ArgsEnhancer<Renderer> = ({ initialArgs }) => traverseArgs(initialArgs);
-
-export const argsEnhancers = [wrapActionsInSpyFns];
-
 export const parameters = {
   throwPlayFunctionExceptions: false,
 };

code/e2e-tests/addon-actions.spec.ts

@@ -26,4 +26,26 @@ test.describe('addon-actions', () => {
     });
     await expect(logItem).toBeVisible();
   });
+
+  test('should show spies', async ({ page }) => {
+    test.skip(
+      templateName.includes('svelte') && templateName.includes('prerelease'),
+      'Svelte 5 prerelase does not support automatic actions with our current example components yet'
+    );
+    await page.goto(storybookUrl);
+    const sbPage = new SbPage(page);
+    sbPage.waitUntilLoaded();
+
+    await sbPage.navigateToStory('addons/actions/spies', 'show-spy-on-in-actions');
+
+    const root = sbPage.previewRoot();
+    const button = root.locator('button', { hasText: 'Button' });
+    await button.click();
+
+    await sbPage.viewAddonPanel('Actions');
+    const logItem = await page.locator('#storybook-panel-root #panel-tab-content', {
+      hasText: 'console.log',
+    });
+    await expect(logItem).toBeVisible();
+  });
 });

code/lib/test/package.json

@@ -56,6 +56,7 @@
     "util": "^0.12.4"
   },
   "devDependencies": {
+    "tinyspy": "^2.2.0",
     "ts-dedent": "^2.2.0",
     "type-fest": "~2.19",
     "typescript": "^5.3.2"

code/lib/test/src/index.test.ts

@@ -1,8 +1,46 @@
-import { it } from 'vitest';
-import { expect, fn } from '@storybook/test';
+import { describe, it, test } from 'vitest';
+import { expect, fn, isMockFunction, traverseArgs } from '@storybook/test';
+import { action } from '@storybook/addon-actions';
 
 it('storybook expect and fn can be used in vitest test', () => {
   const spy = fn();
   spy(1);
   expect(spy).toHaveBeenCalledWith(1);
 });
+
+describe('traverseArgs', () => {
+  const args = {
+    deep: {
+      deeper: {
+        fnKey: fn(),
+        actionKey: action('name'),
+      },
+    },
+    arg2: Object.freeze({ frozen: true }),
+  };
+
+  expect(args.deep.deeper.fnKey.getMockName()).toEqual('spy');
+
+  const traversed = traverseArgs(args) as typeof args;
+
+  test('The same structure is maintained', () =>
+    expect(traversed).toEqual({
+      deep: {
+        deeper: {
+          fnKey: args.deep.deeper.fnKey,
+          actionKey: args.deep.deeper.actionKey,
+        },
+      },
+      // We don't mutate frozen objects, but we do insert them back in the tree
+      arg2: args.arg2,
+    }));
+
+  test('The mock name is mutated to be the arg key', () =>
+    expect(traversed.deep.deeper.fnKey.getMockName()).toEqual('fnKey'));
+
+  const actionFn = traversed.deep.deeper.actionKey;
+
+  test('Actions are wrapped in a spy', () => expect(isMockFunction(actionFn)).toBeTruthy());
+  test('The spy of the action is also matching the arg key ', () =>
+    expect(isMockFunction(actionFn) && actionFn.getMockName()).toEqual('actionKey'));
+});

code/lib/test/src/index.ts

@@ -3,7 +3,15 @@ import { type LoaderFunction } from '@storybook/csf';
 import chai from 'chai';
 import { global } from '@storybook/global';
 import { expect as rawExpect } from './expect';
-import { clearAllMocks, resetAllMocks, restoreAllMocks } from './spy';
+import {
+  clearAllMocks,
+  fn,
+  isMockFunction,
+  onMockCall,
+  resetAllMocks,
+  restoreAllMocks,
+} from './spy';
+import type { Renderer } from '@storybook/types';
 
 export * from './spy';
 
@@ -36,6 +44,52 @@ const resetAllMocksLoader: LoaderFunction = ({ parameters }) => {
   }
 };
 
+export const traverseArgs = (value: unknown, depth = 0, key?: string): unknown => {
+  // Make sure to not get in infinite loops with self referencing args
+  if (depth > 5) return value;
+  if (value == null) return value;
+  if (isMockFunction(value)) {
+    // Makes sure we get the arg name in the interactions panel
+    if (key) value.mockName(key);
+    return value;
+  }
+
+  // wrap explicit actions in a spy
+  if (
+    typeof value === 'function' &&
+    'isAction' in value &&
+    value.isAction &&
+    !('implicit' in value && value.implicit)
+  ) {
+    const mock = fn(value as any);
+    if (key) mock.mockName(key);
+    return mock;
+  }
+
+  if (Array.isArray(value)) {
+    depth++;
+    return value.map((item) => traverseArgs(item, depth));
+  }
+
+  if (typeof value === 'object' && value.constructor === Object) {
+    depth++;
+    for (const [k, v] of Object.entries(value)) {
+      if (Object.getOwnPropertyDescriptor(value, k)?.writable) {
+        // We have to mutate the original object for this to survive HMR.
+        (value as Record<string, unknown>)[k] = traverseArgs(v, depth, k);
+      }
+    }
+    return value;
+  }
+  return value;
+};
+
+const nameSpiesAndWrapActionsInSpies: LoaderFunction<Renderer> = ({ initialArgs }) => {
+  traverseArgs(initialArgs);
+};
+
 // We are using this as a default Storybook loader, when the test package is used. This avoids the need for optional peer dependency workarounds.
 // eslint-disable-next-line no-underscore-dangle
-(global as any).__STORYBOOK_TEST_LOADERS__ = [resetAllMocksLoader];
+(global as any).__STORYBOOK_TEST_LOADERS__ = [resetAllMocksLoader, nameSpiesAndWrapActionsInSpies];
+// eslint-disable-next-line no-underscore-dangle
+(global as any).__STORYBOOK_TEST_ON_MOCK_CALL__ = onMockCall;

code/lib/test/src/spy.test.ts

@@ -0,0 +1,15 @@
+import { it, vi, expect, beforeEach } from 'vitest';
+import { fn, onMockCall } from './spy';
+
+const vitestSpy = vi.fn();
+
+beforeEach(() => {
+  const unsubscribe = onMockCall(vitestSpy);
+  return () => unsubscribe();
+});
+
+it('mocks are reactive', () => {
+  const storybookSpy = fn();
+  storybookSpy(1);
+  expect(vitestSpy).toHaveBeenCalledWith(storybookSpy, [1]);
+});