Merge pull request #26001 from storybookjs/kasper/remove-argtypes-regex

Automigration: Remove argTypesRegex

code/lib/cli/package.json

@@ -56,6 +56,7 @@
     "prep": "node --loader ../../../scripts/node_modules/esbuild-register/loader.js -r ../../../scripts/node_modules/esbuild-register/register.js ../../../scripts/prepare/bundle.ts"
   },
   "dependencies": {
+    "@babel/core": "^7.23.0",
     "@babel/types": "^7.23.0",
     "@ndelangen/get-tarball": "^3.0.7",
     "@storybook/codemod": "workspace:*",

code/lib/cli/src/automigrate/fixes/index.ts

@@ -21,6 +21,7 @@ import { wrapRequire } from './wrap-require';
 import { reactDocgen } from './react-docgen';
 import { removeReactDependency } from './prompt-remove-react';
 import { storyshotsMigration } from './storyshots-migration';
+import { removeArgtypesRegex } from './remove-argtypes-regex';
 import { webpack5CompilerSetup } from './webpack5-compiler-setup';
 import { removeJestTestingLibrary } from './remove-jest-testing-library';
 
@@ -38,6 +39,7 @@ export const allFixes: Fix[] = [
   sbBinary,
   sbScripts,
   incompatibleAddons,
+  removeArgtypesRegex,
   removeJestTestingLibrary,
   removedGlobalClientAPIs,
   mdx1to2,

code/lib/cli/src/automigrate/fixes/remove-argtypes-regex.ts

@@ -0,0 +1,81 @@
+import type { Fix } from '../types';
+import * as fs from 'node:fs/promises';
+import * as babel from '@babel/core';
+import type { BabelFile, NodePath } from '@babel/core';
+import { babelParse } from '@storybook/csf-tools';
+import dedent from 'ts-dedent';
+import chalk from 'chalk';
+
+export const removeArgtypesRegex: Fix<{ argTypesRegex: NodePath; previewConfigPath: string }> = {
+  id: 'remove-argtypes-regex',
+  promptType: 'manual',
+  async check({ previewConfigPath }) {
+    if (!previewConfigPath) return null;
+
+    const previewFile = await fs.readFile(previewConfigPath, { encoding: 'utf-8' });
+
+    // @ts-expect-error File is not yet exposed, see https://github.com/babel/babel/issues/11350#issuecomment-644118606
+    const file: BabelFile = new babel.File(
+      { filename: previewConfigPath },
+      { code: previewFile, ast: babelParse(previewFile) }
+    );
+
+    let argTypesRegex;
+
+    file.path.traverse({
+      Identifier: (path) => {
+        if (path.node.name === 'argTypesRegex') {
+          argTypesRegex = path;
+        }
+      },
+    });
+
+    return argTypesRegex ? { argTypesRegex, previewConfigPath } : null;
+  },
+  prompt({ argTypesRegex, previewConfigPath }) {
+    const snippet = dedent`
+      import { fn } from '@storybook/test';
+      export default {
+        args: { onClick: fn() }, // will log to the action panel when clicked
+      };`;
+
+    // @ts-expect-error File is not yet exposed, see https://github.com/babel/babel/issues/11350#issuecomment-644118606
+    const file: BabelFile = new babel.File(
+      { file: 'story.tsx' },
+      { code: snippet, ast: babelParse(snippet) }
+    );
+
+    let formattedSnippet;
+    file.path.traverse({
+      Identifier: (path) => {
+        if (path.node.name === 'fn') {
+          formattedSnippet = path.buildCodeFrameError(``).message;
+        }
+      },
+    });
+
+    return dedent`
+      ${chalk.bold('Attention')}: We've detected that you're using argTypesRegex:
+      
+      ${argTypesRegex.buildCodeFrameError(`${previewConfigPath}`).message}
+
+      In Storybook 8, we recommend removing this regex.
+      Assign explicit spies with the ${chalk.cyan('fn')} function instead:      
+      ${formattedSnippet}
+      
+      The above pattern is needed when using spies in the play function, ${chalk.bold(
+        'even'
+      )} if you keep using argTypesRegex.
+      Implicit spies (based on a combination of argTypesRegex and docgen) is not supported in Storybook 8.
+      
+      Use the following command to check for spy usages in your play functions:
+       ${chalk.cyan(
+         'npx storybook migrate find-implicit-spies --glob="**/*.stories.@(js|jsx|ts|tsx)"'
+       )}
+       
+      Make sure to assign an explicit ${chalk.cyan('fn')} to your args for those usages. 
+      
+      For more information please visit our migration guide: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#implicit-actions-can-not-be-used-during-rendering-for-example-in-the-play-function
+    `;
+  },
+};

code/lib/codemod/package.json

@@ -29,6 +29,7 @@
     "./dist/transforms/add-component-parameters.js": "./dist/transforms/add-component-parameters.js",
     "./dist/transforms/csf-2-to-3.js": "./dist/transforms/csf-2-to-3.js",
     "./dist/transforms/csf-hoist-story-annotations.js": "./dist/transforms/csf-hoist-story-annotations.js",
+    "./dist/transforms/find-implicit-spies.js": "./dist/transforms/find-implicit-spies.js",
     "./dist/transforms/move-builtin-addons.js": "./dist/transforms/move-builtin-addons.js",
     "./dist/transforms/mdx-to-csf.js": "./dist/transforms/mdx-to-csf.js",
     "./dist/transforms/migrate-to-test-package.js": "./dist/transforms/migrate-to-test-package.js",
@@ -93,6 +94,7 @@
       "./src/transforms/add-component-parameters.js",
       "./src/transforms/csf-2-to-3.ts",
       "./src/transforms/csf-hoist-story-annotations.js",
+      "./src/transforms/find-implicit-spies.ts",
       "./src/transforms/mdx-to-csf.ts",
       "./src/transforms/migrate-to-test-package.ts",
       "./src/transforms/move-builtin-addons.js",

code/lib/codemod/src/transforms/__tests__/find-implicit-spies.test.ts

@@ -0,0 +1,71 @@
+import { beforeEach, expect, test, vi } from 'vitest';
+import transform from '../find-implicit-spies';
+import dedent from 'ts-dedent';
+import ansiRegex from 'ansi-regex';
+
+expect.addSnapshotSerializer({
+  print: (val, print) => print((val as string).replace(ansiRegex(), '')),
+  test: (value) => typeof value === 'string' && ansiRegex().test(value),
+});
+
+const tsTransform = async (source: string) => transform({ source, path: 'Component.stories.tsx' });
+
+const warn = vi.spyOn(console, 'warn');
+
+beforeEach(() => {
+  warn.mockImplementation(() => {});
+});
+
+test('Warn for possible implicit actions', async () => {
+  const input = dedent`
+    export default { title: 'foo/bar', args: {onClick: fn() }, argTypes: { onHover: {action: true} } };
+    const Template = (args) => { };
+    export const A = Template.bind({});
+    A.args = { onBla: fn() };
+    A.play = async ({ args }) => {
+      await userEvent.click(screen.getByRole("button"));
+      await expect(args.onImplicit).toHaveBeenCalled();
+      await expect(args.onClick).toHaveBeenCalled();
+      await expect(args.onHover).toHaveBeenCalled();
+      await expect(args.onBla).toHaveBeenCalled();
+    };
+    
+    export const B = { 
+      args: {onBla: fn() },
+      play: async ({ args }) => {
+        await userEvent.click(screen.getByRole("button"));
+        await expect(args.onImplicit).toHaveBeenCalled();
+        await expect(args.onClick).toHaveBeenCalled();
+        await expect(args.onHover).toHaveBeenCalled();
+        await expect(args.onBla).toHaveBeenCalled();
+      }
+    };
+  `;
+
+  await tsTransform(input);
+
+  expect(warn.mock.calls).toMatchInlineSnapshot(`
+    [
+      [
+        "Component.stories.tsx Possible implicit spy found
+       5 | A.play = async ({ args }) => {
+       6 |   await userEvent.click(screen.getByRole("button"));
+    >  7 |   await expect(args.onImplicit).toHaveBeenCalled();
+         |                     ^^^^^^^^^^
+       8 |   await expect(args.onClick).toHaveBeenCalled();
+       9 |   await expect(args.onHover).toHaveBeenCalled();
+      10 |   await expect(args.onBla).toHaveBeenCalled();",
+      ],
+      [
+        "Component.stories.tsx Possible implicit spy found
+      15 |   play: async ({ args }) => {
+      16 |     await userEvent.click(screen.getByRole("button"));
+    > 17 |     await expect(args.onImplicit).toHaveBeenCalled();
+         |                       ^^^^^^^^^^
+      18 |     await expect(args.onClick).toHaveBeenCalled();
+      19 |     await expect(args.onHover).toHaveBeenCalled();
+      20 |     await expect(args.onBla).toHaveBeenCalled();",
+      ],
+    ]
+  `);
+});

code/lib/codemod/src/transforms/find-implicit-spies.ts

@@ -0,0 +1,144 @@
+/* eslint-disable no-underscore-dangle */
+import type { FileInfo } from 'jscodeshift';
+import { loadCsf } from '@storybook/csf-tools';
+import type { BabelFile } from '@babel/core';
+import * as babel from '@babel/core';
+import { isIdentifier, isObjectExpression, isObjectProperty } from '@babel/types';
+
+function findImplicitSpies(path: babel.NodePath, file: string, keys: string[]) {
+  path.traverse({
+    Identifier: (identifier) => {
+      if (!keys.includes(identifier.node.name) && /^on[A-Z].*/.test(identifier.node.name)) {
+        console.warn(identifier.buildCodeFrameError(`${file} Possible implicit spy found`).message);
+      }
+    },
+  });
+}
+
+function getAnnotationKeys(file: BabelFile, storyName: string, annotationName: string) {
+  const argKeys: string[] = [];
+
+  file.path.traverse({
+    // CSF2 play function Story.args =
+    AssignmentExpression: (path) => {
+      const left = path.get('left');
+      if (!left.isMemberExpression()) return;
+      const object = left.get('object');
+
+      if (!(object.isIdentifier() && object.node.name === storyName)) return;
+
+      const property = left.get('property');
+      const right = path.get('right');
+      if (
+        property.isIdentifier() &&
+        property.node.name === annotationName &&
+        right.isObjectExpression()
+      ) {
+        argKeys.push(
+          ...right.node.properties.flatMap((value) =>
+            isObjectProperty(value) && isIdentifier(value.key) ? [value.key.name] : []
+          )
+        );
+      }
+    },
+    // CSF3 const Story = {args: () => {} };
+    VariableDeclarator: (path) => {
+      const id = path.get('id');
+      const init = path.get('init');
+      if (!(id.isIdentifier() && id.node.name === storyName) || !init.isObjectExpression()) return;
+
+      const args = init
+        .get('properties')
+        .flatMap((it) => (it.isObjectProperty() ? [it] : []))
+        .find((it) => {
+          const argKey = it.get('key');
+          return argKey.isIdentifier() && argKey.node.name === annotationName;
+        });
+
+      if (!args) return;
+      const argsValue = args.get('value');
+
+      if (!argsValue || !argsValue.isObjectExpression()) return;
+      argKeys.push(
+        ...argsValue.node.properties.flatMap((value) =>
+          isObjectProperty(value) && isIdentifier(value.key) ? [value.key.name] : []
+        )
+      );
+    },
+  });
+
+  return argKeys;
+}
+
+const getObjectExpressionKeys = (node: babel.Node | undefined) => {
+  return isObjectExpression(node)
+    ? node.properties.flatMap((value) =>
+        isObjectProperty(value) && isIdentifier(value.key) ? [value.key.name] : []
+      )
+    : [];
+};
+
+export default async function transform(info: FileInfo) {
+  const csf = loadCsf(info.source, { makeTitle: (title) => title });
+  const fileNode = csf._ast;
+  // @ts-expect-error File is not yet exposed, see https://github.com/babel/babel/issues/11350#issuecomment-644118606
+  const file: BabelFile = new babel.File(
+    { filename: info.path },
+    { code: info.source, ast: fileNode }
+  );
+
+  csf.parse();
+
+  const metaKeys = [
+    ...getObjectExpressionKeys(csf._metaAnnotations.args),
+    ...getObjectExpressionKeys(csf._metaAnnotations.argTypes),
+  ];
+
+  Object.entries(csf.stories).forEach(([key, { name }]) => {
+    if (!name) return;
+    const allKeys = [
+      ...metaKeys,
+      ...getAnnotationKeys(file, name, 'args'),
+      ...getAnnotationKeys(file, name, 'argTypes'),
+    ];
+
+    file.path.traverse({
+      // CSF2 play function Story.play =
+      AssignmentExpression: (path) => {
+        const left = path.get('left');
+        if (!left.isMemberExpression()) return;
+        const object = left.get('object');
+
+        if (!(object.isIdentifier() && object.node.name === name)) return;
+
+        const property = left.get('property');
+        if (property.isIdentifier() && property.node.name === 'play') {
+          findImplicitSpies(path, info.path, allKeys);
+        }
+      },
+
+      // CSF3 play function: const Story = {play: () => {} };
+      VariableDeclarator: (path) => {
+        const id = path.get('id');
+        const init = path.get('init');
+        if (!(id.isIdentifier() && id.node.name === name) || !init.isObjectExpression()) return;
+
+        const play = init
+          .get('properties')
+          .flatMap((it) => (it.isObjectProperty() ? [it] : []))
+          .find((it) => {
+            const argKey = it.get('key');
+            return argKey.isIdentifier() && argKey.node.name === 'play';
+          });
+
+        if (play) {
+          findImplicitSpies(play, info.path, allKeys);
+        }
+      },
+    });
+  });
+
+  return;
+}
+
+export const parser = 'tsx';

code/yarn.lock

@@ -5437,6 +5437,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@storybook/cli@workspace:lib/cli"
   dependencies:
+    "@babel/core": "npm:^7.23.0"
     "@babel/types": "npm:^7.23.0"
     "@ndelangen/get-tarball": "npm:^3.0.7"
     "@storybook/codemod": "workspace:*"