refactor and cleanup automigrations

code/lib/cli/src/automigrate/fixes/autodocs-true.test.ts

@@ -0,0 +1,46 @@
+import type { StorybookConfig } from '@storybook/types';
+import type { PackageJson } from '../../js-package-manager';
+import { autodocsTrue } from './autodocs-true';
+import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
+
+const checkAutodocs = async ({
+  packageJson = {},
+  main: mainConfig,
+  storybookVersion = '7.0.0',
+}: {
+  packageJson?: PackageJson;
+  main: Partial<StorybookConfig> & Record<string, unknown>;
+  storybookVersion?: string;
+}) => {
+  mockStorybookData({ mainConfig, storybookVersion });
+
+  return autodocsTrue.check({
+    packageManager: makePackageManager(packageJson),
+  });
+};
+
+describe('autodocs-true fix', () => {
+  afterEach(jest.restoreAllMocks);
+
+  it('should skip when docs.autodocs is already defined', async () => {
+    await expect(checkAutodocs({ main: { docs: { autodocs: 'tag' } } })).resolves.toBeFalsy();
+  });
+
+  it('should throw when docs.docsPage contains invalid value', async () => {
+    const main = { docs: { docsPage: 123 } } as any;
+    await expect(checkAutodocs({ main })).rejects.toThrow();
+  });
+
+  it('should prompt when using docs.docsPage legacy property', async () => {
+    const main = { docs: { docsPage: true } } as any;
+    await expect(checkAutodocs({ main })).resolves.toEqual({
+      value: 'tag',
+    });
+  });
+
+  it('should prompt when not using docs.autodocs', async () => {
+    await expect(checkAutodocs({ main: {} })).resolves.toEqual({
+      value: true,
+    });
+  });
+});

code/lib/cli/src/automigrate/fixes/autodocs-true.ts

@@ -1,17 +1,14 @@
 import chalk from 'chalk';
 import { dedent } from 'ts-dedent';
 
-import type { ConfigFile } from '@storybook/csf-tools';
-import { readConfig, writeConfig } from '@storybook/csf-tools';
-import { getStorybookInfo } from '@storybook/core-common';
 import type { StorybookConfig } from '@storybook/types';
 
 import type { Fix } from '../types';
+import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile';
 
 const logger = console;
 
 interface AutodocsTrueFrameworkRunOptions {
-  main: ConfigFile;
   value?: StorybookConfig['docs']['autodocs'];
 }
 
@@ -21,35 +18,31 @@ interface AutodocsTrueFrameworkRunOptions {
 export const autodocsTrue: Fix<AutodocsTrueFrameworkRunOptions> = {
   id: 'autodocsTrue',
 
-  async check({ packageManager }) {
-    const packageJson = packageManager.retrievePackageJson();
+  async check({ packageManager, configDir }) {
+    const { mainConfig } = await getStorybookData({ packageManager, configDir });
 
-    const { mainConfig } = getStorybookInfo(packageJson);
-
-    if (!mainConfig) {
-      logger.warn('Unable to find storybook main.js config, skipping');
-      return null;
-    }
-
-    const main = await readConfig(mainConfig);
-    const docs = main.getFieldValue(['docs']);
+    const { docs } = mainConfig;
 
     const docsPageToAutodocsMapping = {
       true: 'tag' as const,
       automatic: true,
       false: false,
     };
+
+    // @ts-expect-error docsPage does not exist anymore but we need to account for legacy code
     if (docs?.docsPage) {
+      // @ts-expect-error same as above
       const oldValue = docs?.docsPage.toString();
-      if (!(oldValue in docsPageToAutodocsMapping))
+      if (!(oldValue in docsPageToAutodocsMapping)) {
         throw new Error(`Unexpected value for docs.docsPage: ${oldValue}`);
+      }
+
       return {
-        main,
         value: docsPageToAutodocsMapping[oldValue as keyof typeof docsPageToAutodocsMapping],
       };
     }
 
-    return docs?.autodocs === undefined ? { main } : null;
+    return docs?.autodocs === undefined ? { value: true } : null;
   },
 
   prompt({ value }) {
@@ -90,12 +83,13 @@ export const autodocsTrue: Fix<AutodocsTrueFrameworkRunOptions> = {
     `;
   },
 
-  async run({ result: { main, value }, dryRun }) {
+  async run({ result: { value }, dryRun, mainConfigPath }) {
     logger.info(`✅ Setting 'docs.autodocs' to true in main.js`);
     if (!dryRun) {
-      main.removeField(['docs', 'docsPage']);
-      main.setFieldValue(['docs', 'autodocs'], value ?? true);
-      await writeConfig(main);
+      await updateMainConfig({ mainConfigPath, dryRun }, async (main) => {
+        main.removeField(['docs', 'docsPage']);
+        main.setFieldValue(['docs', 'autodocs'], value ?? true);
+      });
     }
   },
 };

code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.test.ts

@@ -1,47 +1,40 @@
-/* eslint-disable no-underscore-dangle */
 /// <reference types="@types/jest" />;
 
 import type { StorybookConfig } from '@storybook/types';
-import path from 'path';
-import type { JsPackageManager, PackageJson } from '../../js-package-manager';
+import type { PackageJson } from '../../js-package-manager';
 import { ansiRegex } from '../helpers/cleanLog';
+import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
 import type { BareMdxStoriesGlobRunOptions } from './bare-mdx-stories-glob';
 import { bareMdxStoriesGlob } from './bare-mdx-stories-glob';
 
-// eslint-disable-next-line global-require, jest/no-mocks-import
-jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra'));
-
 const checkBareMdxStoriesGlob = async ({
   packageJson,
-  main,
+  main: mainConfig,
+  storybookVersion = '7.0.0',
 }: {
   packageJson: PackageJson;
-  main?: Partial<StorybookConfig>;
+  main?: Partial<StorybookConfig> & Record<string, unknown>;
+  storybookVersion?: string;
 }) => {
-  if (main) {
-    // eslint-disable-next-line global-require
-    require('fs-extra').__setMockFiles({
-      [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`,
-    });
-  } else {
-    // eslint-disable-next-line global-require
-    require('fs-extra').__setMockFiles({});
-  }
-  const packageManager = {
-    retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
-  } as JsPackageManager;
+  mockStorybookData({ mainConfig, storybookVersion });
 
-  return bareMdxStoriesGlob.check({ packageManager });
+  return bareMdxStoriesGlob.check({
+    packageManager: makePackageManager(packageJson),
+  });
 };
 
 describe('bare-mdx fix', () => {
+  afterEach(jest.restoreAllMocks);
+
   describe('should no-op', () => {
     it('in SB < v7.0.0', async () => {
       const packageJson = {
         dependencies: { '@storybook/react': '^6.2.0' },
       };
       const main = { stories: ['../**/*.stories.mdx'] };
-      await expect(checkBareMdxStoriesGlob({ packageJson, main })).resolves.toBeFalsy();
+      await expect(
+        checkBareMdxStoriesGlob({ packageJson, main, storybookVersion: '6.5.0' })
+      ).resolves.toBeFalsy();
     });
 
     describe('in SB >= v7.0.0', () => {
@@ -60,24 +53,6 @@ describe('bare-mdx fix', () => {
         await expect(checkBareMdxStoriesGlob({ packageJson, main })).rejects.toThrow();
       });
 
-      it('with variable references in stories field', async () => {
-        // eslint-disable-next-line global-require
-        require('fs-extra').__setMockFiles({
-          [path.join('.storybook', 'main.js')]: `
-          const globVar = '../**/*.stories.mdx';
-          module.exports = { stories: [globVar] };`,
-        });
-
-        const packageManager = {
-          retrievePackageJson: () => ({
-            dependencies: { '@storybook/react': '^7.0.0' },
-            devDependencies: {},
-          }),
-        } as unknown as JsPackageManager;
-
-        await expect(bareMdxStoriesGlob.check({ packageManager })).rejects.toThrow();
-      });
-
       it('without .stories.mdx in globs', async () => {
         const packageJson = {
           dependencies: { '@storybook/react': '^7.0.0' },

code/lib/cli/src/automigrate/fixes/bare-mdx-stories-glob.ts

@@ -1,18 +1,13 @@
 import chalk from 'chalk';
 import dedent from 'ts-dedent';
 import semver from 'semver';
-import type { ConfigFile } from '@storybook/csf-tools';
-import { readConfig, writeConfig } from '@storybook/csf-tools';
-import { getStorybookInfo } from '@storybook/core-common';
-import type { StoriesEntry } from 'lib/types/src';
+import type { StoriesEntry } from '@storybook/types';
+import { getStorybookData, updateMainConfig } from '../helpers/mainConfigFile';
 import type { Fix } from '../types';
 
 const logger = console;
 
-const fixId = 'bare-mdx-stories-glob';
-
 export interface BareMdxStoriesGlobRunOptions {
-  main: ConfigFile;
   existingStoriesEntries: StoriesEntry[];
   nextStoriesEntries: StoriesEntry[];
 }
@@ -35,41 +30,24 @@ const getNextGlob = (glob: string) => {
 };
 
 export const bareMdxStoriesGlob: Fix<BareMdxStoriesGlobRunOptions> = {
-  id: fixId,
-  async check({ packageManager }) {
-    const packageJson = packageManager.retrievePackageJson();
-
-    const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson);
-    if (!mainConfig) {
-      logger.warn('Unable to find storybook main.js config, skipping');
-      return null;
-    }
-
-    const sbVersionCoerced = storybookVersion && semver.coerce(storybookVersion)?.version;
-    if (!sbVersionCoerced) {
-      throw new Error(dedent`
-        ❌ Unable to determine storybook version.
-        🤔 Are you running automigrate from your project directory? Please specify your Storybook config directory with the --config-dir flag.
-      `);
-    }
+  id: 'bare-mdx-stories-glob',
+  async check({ packageManager, configDir }) {
+    const { storybookVersion, mainConfig } = await getStorybookData({
+      configDir,
+      packageManager,
+    });
 
-    if (!semver.gte(sbVersionCoerced, '7.0.0')) {
+    if (!semver.gte(storybookVersion, '7.0.0')) {
       return null;
     }
 
-    const main = await readConfig(mainConfig);
-
-    let existingStoriesEntries;
-
-    try {
-      existingStoriesEntries = main.getFieldValue(['stories']) as StoriesEntry[];
-    } catch (e) {
-      // throws in next null check below
-    }
+    const existingStoriesEntries = mainConfig.stories as StoriesEntry[];
 
     if (!existingStoriesEntries) {
       throw new Error(dedent`
-      ❌ Unable to determine Storybook stories globs, skipping ${chalk.cyan(fixId)} fix.
+      ❌ Unable to determine Storybook stories globs in ${chalk.blue(
+        mainConfig
+      )}, skipping ${chalk.cyan(this.id)} fix.
       
       In Storybook 7, we have deprecated defining stories in MDX files, and consequently have changed the suffix to simply .mdx.
 
@@ -112,7 +90,7 @@ export const bareMdxStoriesGlob: Fix<BareMdxStoriesGlobRunOptions> = {
       return null;
     }
 
-    return { existingStoriesEntries, nextStoriesEntries, main };
+    return { existingStoriesEntries, nextStoriesEntries };
   },
 
   prompt({ existingStoriesEntries, nextStoriesEntries }) {
@@ -138,13 +116,14 @@ export const bareMdxStoriesGlob: Fix<BareMdxStoriesGlobRunOptions> = {
   `;
   },
 
-  async run({ dryRun, result: { nextStoriesEntries, main } }) {
+  async run({ dryRun, mainConfigPath, result: { nextStoriesEntries } }) {
     logger.info(dedent`✅ Setting 'stories' config:
       ${JSON.stringify(nextStoriesEntries, null, 2)}`);
 
-    main.setFieldValue(['stories'], nextStoriesEntries);
     if (!dryRun) {
-      await writeConfig(main);
+      await updateMainConfig({ mainConfigPath, dryRun }, async (main) => {
+        main.setFieldValue(['stories'], nextStoriesEntries);
+      });
     }
   },
 };

code/lib/cli/src/automigrate/fixes/builder-vite.test.ts

@@ -1,30 +1,27 @@
-/* eslint-disable no-underscore-dangle */
-import * as path from 'path';
 import type { StorybookConfig } from '@storybook/types';
-import type { JsPackageManager, PackageJson } from '../../js-package-manager';
+import type { PackageJson } from '../../js-package-manager';
 import { builderVite } from './builder-vite';
-
-// eslint-disable-next-line global-require, jest/no-mocks-import
-jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra'));
+import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
 
 const checkBuilderVite = async ({
   packageJson = {},
-  main,
+  main: mainConfig,
+  storybookVersion = '7.0.0',
 }: {
   packageJson?: PackageJson;
-  main: Partial<StorybookConfig>;
+  main: Partial<StorybookConfig> & Record<string, unknown>;
+  storybookVersion?: string;
 }) => {
-  // eslint-disable-next-line global-require
-  require('fs-extra').__setMockFiles({
-    [path.join('.storybook', 'main.js')]: `module.exports = ${JSON.stringify(main)};`,
+  mockStorybookData({ mainConfig, storybookVersion });
+
+  return builderVite.check({
+    packageManager: makePackageManager(packageJson),
   });
-  const packageManager = {
-    retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
-  } as JsPackageManager;
-  return builderVite.check({ packageManager });
 };
 
 describe('builder-vite fix', () => {
+  afterEach(jest.restoreAllMocks);
+
   describe('storybook-builder-vite', () => {
     it('using storybook-builder-vite', async () => {
       const main = { core: { builder: 'storybook-builder-vite' } };