Merge pull request #383 from storybookjs/improve-types

Improve type safety and code quality

.storybook/test-runner.ts

@@ -39,7 +39,7 @@ const config: TestRunnerConfig = {
     });
 
     const elementHandler = (await page.$('#root')) || (await page.$('#storybook-root'));
-    const innerHTML = await elementHandler.innerHTML();
+    const innerHTML = await elementHandler?.innerHTML();
     // HTML snapshot tests
     expect(innerHTML).toMatchSnapshot();
   },

package.json

@@ -53,7 +53,6 @@
     "@babel/preset-env": "^7.19.4",
     "@babel/preset-react": "^7.18.6",
     "@babel/preset-typescript": "^7.18.6",
-    "@jest/types": "^29.6.3",
     "@storybook/addon-coverage": "^0.0.9",
     "@storybook/addon-essentials": "^7.5.3",
     "@storybook/addon-interactions": "^7.5.3",
@@ -96,6 +95,7 @@
     "@babel/generator": "^7.22.5",
     "@babel/template": "^7.22.5",
     "@babel/types": "^7.22.5",
+    "@jest/types": "^29.6.3",
     "@storybook/core-common": "^7.0.0-beta.0 || ^7.0.0-rc.0 || ^7.0.0",
     "@storybook/csf": "^0.1.1",
     "@storybook/csf-tools": "^7.0.0-beta.0 || ^7.0.0-rc.0 || ^7.0.0",

src/config/jest-playwright.ts

@@ -1,7 +1,8 @@
 import path from 'path';
 import { getProjectRoot } from '@storybook/core-common';
+import type { Config } from '@jest/types';
 
-const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH || '@storybook/test-runner';
+const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH ?? '@storybook/test-runner';
 
 /**
  * IMPORTANT NOTE:
@@ -15,7 +16,7 @@ const TEST_RUNNER_PATH = process.env.STORYBOOK_TEST_RUNNER_PATH || '@storybook/t
  * This function does the same thing as `preset: 'jest-playwright-preset` but makes sure that the
  * necessary moving parts are all required within the correct path.
  * */
-const getJestPlaywrightConfig = () => {
+const getJestPlaywrightConfig = (): Config.InitialOptions => {
   const presetBasePath = path.dirname(
     require.resolve('jest-playwright-preset', {
       paths: [path.join(__dirname, '../node_modules')],
@@ -28,18 +29,18 @@ const getJestPlaywrightConfig = () => {
   );
   return {
     runner: path.join(presetBasePath, 'runner.js'),
-    globalSetup: require.resolve(TEST_RUNNER_PATH + '/playwright/global-setup.js'),
-    globalTeardown: require.resolve(TEST_RUNNER_PATH + '/playwright/global-teardown.js'),
-    testEnvironment: require.resolve(TEST_RUNNER_PATH + '/playwright/custom-environment.js'),
+    globalSetup: require.resolve(`${TEST_RUNNER_PATH}/playwright/global-setup.js`),
+    globalTeardown: require.resolve(`${TEST_RUNNER_PATH}/playwright/global-teardown.js`),
+    testEnvironment: require.resolve(`${TEST_RUNNER_PATH}/playwright/custom-environment.js`),
     setupFilesAfterEnv: [
-      require.resolve(TEST_RUNNER_PATH + '/playwright/jest-setup.js'),
+      require.resolve(`${TEST_RUNNER_PATH}/playwright/jest-setup.js`),
       expectPlaywrightPath,
       path.join(presetBasePath, 'lib', 'extends.js'),
     ],
   };
 };
 
-export const getJestConfig = () => {
+export const getJestConfig = (): Config.InitialOptions => {
   const {
     TEST_ROOT,
     TEST_MATCH,
@@ -69,16 +70,16 @@ export const getJestConfig = () => {
 
   const reporters = STORYBOOK_JUNIT ? ['default', jestJunitPath] : ['default'];
 
-  const testMatch = (STORYBOOK_STORIES_PATTERN && STORYBOOK_STORIES_PATTERN.split(';')) || [];
+  const testMatch = STORYBOOK_STORIES_PATTERN?.split(';') ?? [];
 
-  let config = {
+  const config: Config.InitialOptions = {
     rootDir: getProjectRoot(),
     roots: TEST_ROOT ? [TEST_ROOT] : undefined,
     reporters,
     testMatch,
     transform: {
       '^.+\\.(story|stories)\\.[jt]sx?$': require.resolve(
-        TEST_RUNNER_PATH + '/playwright/transform'
+        `${TEST_RUNNER_PATH}/playwright/transform`
       ),
       '^.+\\.[jt]sx?$': swcJestPath,
     },

src/csf/__snapshots__/transformCsf.test.ts.snap

@@ -229,3 +229,12 @@ if (!require.main) {
   });
 }"
 `;
+
+exports[`transformCsf returns empty result if there are no stories 1`] = `
+"
+      export default {
+        title: 'Button',
+      };
+    
+"
+`;

src/csf/transformCsf.test.ts

@@ -16,6 +16,18 @@ describe('transformCsf', () => {
     expect(result).toEqual(expectedCode);
   });
 
+  it('returns empty result if there are no stories', () => {
+    const csfCode = `
+      export default {
+        title: 'Button',
+      };
+    `;
+
+    const result = transformCsf(csfCode, { testPrefixer });
+
+    expect(result).toMatchSnapshot();
+  });
+
   it('calls the testPrefixer function for each test', () => {
     const csfCode = `
       export default {

src/csf/transformCsf.ts

@@ -54,11 +54,11 @@ const makePlayTest = ({
   metaOrStoryPlay?: boolean;
   testPrefix: TestPrefixer;
   shouldSkip?: boolean;
-}): t.Statement[] => {
+}): t.ExpressionStatement[] => {
   return [
     t.expressionStatement(
       t.callExpression(shouldSkip ? t.identifier('it.skip') : t.identifier('it'), [
-        t.stringLiteral(!!metaOrStoryPlay ? 'play-test' : 'smoke-test'),
+        t.stringLiteral(metaOrStoryPlay ? 'play-test' : 'smoke-test'),
         prefixFunction(key, title, testPrefix),
       ])
     ),
@@ -69,7 +69,7 @@ const makeDescribe = (
   key: string,
   tests: t.Statement[],
   beforeEachBlock?: t.ExpressionStatement
-): t.Statement | null => {
+): t.ExpressionStatement => {
   const blockStatements = beforeEachBlock ? [beforeEachBlock, ...tests] : tests;
   return t.expressionStatement(
     t.callExpression(t.identifier('describe'), [
@@ -100,22 +100,25 @@ export const transformCsf = (
 ) => {
   const { includeTags, excludeTags, skipTags } = getTagOptions();
 
-  const csf = loadCsf(code, { makeTitle: makeTitle || ((userTitle: string) => userTitle) });
+  const csf = loadCsf(code, { makeTitle: makeTitle ?? ((userTitle: string) => userTitle) });
   csf.parse();
 
   const storyExports = Object.keys(csf._stories);
   const title = csf.meta?.title;
 
-  const storyAnnotations = storyExports.reduce((acc, key) => {
-    const annotations = csf._storyAnnotations[key];
-    acc[key] = {};
-    if (annotations?.play) {
-      acc[key].play = annotations.play;
-    }
+  const storyAnnotations = storyExports.reduce<Record<string, { play?: t.Node; tags?: string[] }>>(
+    (acc, key) => {
+      const annotations = csf._storyAnnotations[key];
+      acc[key] = {};
+      if (annotations?.play) {
+        acc[key].play = annotations.play;
+      }
 
-    acc[key].tags = csf._stories[key].tags || csf.meta?.tags || [];
-    return acc;
-  }, {} as Record<string, { play?: t.Node; tags?: string[] }>);
+      acc[key].tags = csf._stories[key].tags || csf.meta?.tags || [];
+      return acc;
+    },
+    {}
+  );
 
   const allTests = storyExports
     .filter((key) => {
@@ -148,7 +151,6 @@ export const transformCsf = (
       if (tests.length) {
         return makeDescribe(key, tests);
       }
-      return null;
     })
     .filter(Boolean) as babel.types.Statement[];
 

src/playwright/transformPlaywright.test.ts

@@ -21,8 +21,8 @@ jest.mock('@storybook/core-common', () => ({
 jest.mock('../util/getTestRunnerConfig');
 
 expect.addSnapshotSerializer({
-  print: (val: any) => val.trim(),
-  test: (val: any) => true,
+  print: (val: unknown) => (typeof val === 'string' ? val.trim() : String(val)),
+  test: () => true,
 });
 
 describe('Playwright', () => {

src/playwright/transformPlaywright.ts

@@ -13,8 +13,9 @@ const coverageErrorMessage = dedent`
   More info: https://github.com/storybookjs/test-runner#setting-up-code-coverage
 `;
 
-export const testPrefixer = template(
-  `
+export const testPrefixer: TestPrefixer = (context) => {
+  return template(
+    `
     console.log({ id: %%id%%, title: %%title%%, name: %%name%%, storyExport: %%storyExport%% });
     async () => {
       const testFn = async() => {
@@ -66,14 +67,15 @@ export const testPrefixer = template(
       }
     }
   `,
-  {
-    plugins: ['jsx'],
-  }
-) as unknown as TestPrefixer;
+    {
+      plugins: ['jsx'],
+    }
+  )({ ...context });
+};
 
 const makeTitleFactory = (filename: string) => {
   const { workingDir, normalizedStoriesEntries } = getStorybookMetadata();
-  const filePath = './' + relative(workingDir, filename);
+  const filePath = `./${relative(workingDir, filename)}`;
 
   return (userTitle: string) =>
     userOrAutoTitle(filePath, normalizedStoriesEntries, userTitle) as string;

src/playwright/transformPlaywrightJson.test.ts

@@ -1,4 +1,11 @@
-import { transformPlaywrightJson } from './transformPlaywrightJson';
+import {
+  UnsupportedVersion,
+  V3StoriesIndex,
+  V4Index,
+  makeDescribe,
+  transformPlaywrightJson,
+} from './transformPlaywrightJson';
+import * as t from '@babel/types';
 
 jest.mock('../util/getTestRunnerConfig');
 
@@ -18,23 +25,20 @@ describe('Playwright Json', () => {
             id: 'example-header--logged-in',
             title: 'Example/Header',
             name: 'Logged In',
-            importPath: './stories/basic/Header.stories.js',
             tags: ['play-fn'],
           },
           'example-header--logged-out': {
             id: 'example-header--logged-out',
             title: 'Example/Header',
             name: 'Logged Out',
-            importPath: './stories/basic/Header.stories.js',
           },
           'example-page--logged-in': {
             id: 'example-page--logged-in',
             title: 'Example/Page',
             name: 'Logged In',
-            importPath: './stories/basic/Page.stories.js',
           },
         },
-      };
+      } satisfies V4Index;
       expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
         {
           "example-header": "describe("Example/Header", () => {
@@ -355,16 +359,14 @@ describe('Playwright Json', () => {
             id: 'example-introduction--page',
             title: 'Example/Introduction',
             name: 'Page',
-            importPath: './stories/basic/Introduction.stories.mdx',
           },
           'example-page--logged-in': {
             id: 'example-page--logged-in',
             title: 'Example/Page',
             name: 'Logged In',
-            importPath: './stories/basic/Page.stories.js',
           },
         },
-      };
+      } satisfies V4Index;
       expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
         {
           "example-page": "describe("Example/Page", () => {
@@ -433,9 +435,6 @@ describe('Playwright Json', () => {
             id: 'example-header--logged-in',
             title: 'Example/Header',
             name: 'Logged In',
-            importPath: './stories/basic/Header.stories.js',
-            kind: 'Example/Header',
-            story: 'Logged In',
             parameters: {
               __id: 'example-header--logged-in',
               docsOnly: false,
@@ -446,9 +445,6 @@ describe('Playwright Json', () => {
             id: 'example-header--logged-out',
             title: 'Example/Header',
             name: 'Logged Out',
-            importPath: './stories/basic/Header.stories.js',
-            kind: 'Example/Header',
-            story: 'Logged Out',
             parameters: {
               __id: 'example-header--logged-out',
               docsOnly: false,
@@ -459,17 +455,14 @@ describe('Playwright Json', () => {
             id: 'example-page--logged-in',
             title: 'Example/Page',
             name: 'Logged In',
-            importPath: './stories/basic/Page.stories.js',
-            kind: 'Example/Page',
-            story: 'Logged In',
             parameters: {
               __id: 'example-page--logged-in',
               docsOnly: false,
               fileName: './stories/basic/Page.stories.js',
             },
           },
         },
-      };
+      } satisfies V3StoriesIndex;
       expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
         {
           "example-header": "describe("Example/Header", () => {
@@ -638,9 +631,6 @@ describe('Playwright Json', () => {
             id: 'example-introduction--page',
             title: 'Example/Introduction',
             name: 'Page',
-            importPath: './stories/basic/Introduction.stories.mdx',
-            kind: 'Example/Introduction',
-            story: 'Page',
             parameters: {
               __id: 'example-introduction--page',
               docsOnly: true,
@@ -651,17 +641,14 @@ describe('Playwright Json', () => {
             id: 'example-page--logged-in',
             title: 'Example/Page',
             name: 'Logged In',
-            importPath: './stories/basic/Page.stories.js',
-            kind: 'Example/Page',
-            story: 'Logged In',
             parameters: {
               __id: 'example-page--logged-in',
               docsOnly: false,
               fileName: './stories/basic/Page.stories.js',
             },
           },
         },
-      };
+      } satisfies V3StoriesIndex;
       expect(transformPlaywrightJson(input)).toMatchInlineSnapshot(`
         {
           "example-page": "describe("Example/Page", () => {
@@ -721,3 +708,39 @@ describe('Playwright Json', () => {
     });
   });
 });
+
+describe('unsupported index', () => {
+  it('throws an error for unsupported versions', () => {
+    const unsupportedVersion = { v: 1 } satisfies UnsupportedVersion;
+    expect(() => transformPlaywrightJson(unsupportedVersion)).toThrowError(
+      `Unsupported version ${unsupportedVersion.v}`
+    );
+  });
+});
+
+describe('makeDescribe', () => {
+  it('should generate a skipped describe block with a no-op test when stmts is empty', () => {
+    const title = 'Test Title';
+    const stmts: t.Statement[] = []; // Empty array
+
+    const result = makeDescribe(title, stmts);
+
+    // Create the expected AST manually for a skipped describe block with a no-op test
+    const noOpIt = t.expressionStatement(
+      t.callExpression(t.identifier('it'), [
+        t.stringLiteral('no-op'),
+        t.arrowFunctionExpression([], t.blockStatement([])),
+      ])
+    );
+
+    const expectedAST = t.expressionStatement(
+      t.callExpression(t.memberExpression(t.identifier('describe'), t.identifier('skip')), [
+        t.stringLiteral(title),
+        t.arrowFunctionExpression([], t.blockStatement([noOpIt])),
+      ])
+    );
+
+    // Compare the generated AST with the expected AST
+    expect(result).toEqual(expectedAST);
+  });
+});