Skip to content
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.

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

CLI: Remove argTypesRegex automigration #26001

Merged
merged 11 commits into from
Feb 19, 2024
Merged
1 change: 1 addition & 0 deletions code/lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:*",
Expand Down
2 changes: 2 additions & 0 deletions code/lib/cli/src/automigrate/fixes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -38,6 +39,7 @@ export const allFixes: Fix[] = [
sbBinary,
sbScripts,
incompatibleAddons,
removeArgtypesRegex,
removeJestTestingLibrary,
removedGlobalClientAPIs,
mdx1to2,
Expand Down
78 changes: 78 additions & 0 deletions code/lib/cli/src/automigrate/fixes/remove-argtypes-regex.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
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',
promptOnly: true,
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:
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved

${argTypesRegex.buildCodeFrameError(`${previewConfigPath}`).message}

In Storybook 8, we recommend removing this regex, and assigning explicit spies with the ${chalk.cyan(
'fn'
)} function instead:
${formattedSnippet}

Even if you keep using the argTypesRegex, you will need to above pattern if you want to use spies in your play function.
Implicit spies (based on a combination of argTypesRegex and docgen) is not supported anymore 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.
kasperpeulen marked this conversation as resolved.
Show resolved Hide resolved
`;
},
};
2 changes: 2 additions & 0 deletions code/lib/codemod/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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();",
],
]
`);
});
144 changes: 144 additions & 0 deletions code/lib/codemod/src/transforms/find-implicit-spies.ts
Original file line number Diff line number Diff line change
@@ -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);
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
}
},
});
}

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';
1 change: 1 addition & 0 deletions code/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5354,6 +5354,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:*"
Expand Down
Loading