Skip to content

Commit

Permalink
Merge branch 'next' into norbert/remove-logging-feature-from-autobloc…
Browse files Browse the repository at this point in the history
…kers
  • Loading branch information
ndelangen authored Feb 19, 2024
2 parents 7db99a4 + 15ed50a commit c827d17
Show file tree
Hide file tree
Showing 7 changed files with 302 additions and 0 deletions.
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
81 changes: 81 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,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
`;
},
};
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);
}
},
});
}

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 @@ -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:*"
Expand Down

0 comments on commit c827d17

Please sign in to comment.