Merge pull request #21168 from storybookjs/fix/refactor-automigrations

CLI: Improve how automigrations read `main.js`

code/lib/cli/src/automigrate/fixes/angular12.test.ts

@@ -1,46 +1,44 @@
-/* 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 { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
 import { angular12 } from './angular12';
 
-// eslint-disable-next-line global-require, jest/no-mocks-import
-jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra'));
-
 const checkAngular12 = 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 angular12.check({
+    packageManager: makePackageManager(packageJson),
+    configDir: '',
   });
-  const packageManager = {
-    retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
-  } as JsPackageManager;
-  return angular12.check({ packageManager });
 };
 
 describe('angular12 fix', () => {
+  afterEach(jest.restoreAllMocks);
+
   describe('sb < 6.3', () => {
     describe('angular12 dependency', () => {
       const packageJson = {
-        dependencies: { '@storybook/react': '^6.2.0', '@angular/core': '^12.0.0' },
+        dependencies: { '@storybook/angular': '^6.2.0', '@angular/core': '^12.0.0' },
       };
       it('should fail', async () => {
         await expect(
           checkAngular12({
             packageJson,
-            main: {},
+            storybookVersion: '6.2.0',
           })
         ).rejects.toThrow();
       });
     });
     describe('no angular dependency', () => {
-      const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } };
+      const packageJson = { dependencies: { '@storybook/angular': '^6.2.0' } };
       it('should no-op', async () => {
         await expect(
           checkAngular12({
@@ -54,14 +52,15 @@ describe('angular12 fix', () => {
   describe('sb 6.3 - 7.0', () => {
     describe('angular12 dependency', () => {
       const packageJson = {
-        dependencies: { '@storybook/react': '^6.3.0', '@angular/core': '^12.0.0' },
+        dependencies: { '@storybook/angular': '^6.3.0', '@angular/core': '^12.0.0' },
       };
       describe('webpack5 builder', () => {
         it('should no-op', async () => {
           await expect(
             checkAngular12({
               packageJson,
               main: { core: { builder: 'webpack5' } },
+              storybookVersion: '6.3.0',
             })
           ).resolves.toBeFalsy();
         });
@@ -82,10 +81,11 @@ describe('angular12 fix', () => {
             checkAngular12({
               packageJson,
               main: { core: { builder: 'webpack4' } },
+              storybookVersion: '6.3.0',
             })
           ).resolves.toMatchObject({
             angularVersion: '^12.0.0',
-            storybookVersion: '^6.3.0',
+            storybookVersion: '6.3.0',
           });
         });
       });
@@ -94,11 +94,11 @@ describe('angular12 fix', () => {
           await expect(
             checkAngular12({
               packageJson,
-              main: {},
+              storybookVersion: '6.3.0',
             })
           ).resolves.toMatchObject({
             angularVersion: '^12.0.0',
-            storybookVersion: '^6.3.0',
+            storybookVersion: '6.3.0',
           });
         });
       });
@@ -108,7 +108,6 @@ describe('angular12 fix', () => {
         await expect(
           checkAngular12({
             packageJson: {},
-            main: {},
           })
         ).resolves.toBeFalsy();
       });
@@ -117,13 +116,12 @@ describe('angular12 fix', () => {
   describe('sb 7.0+', () => {
     describe('angular12 dependency', () => {
       const packageJson = {
-        dependencies: { '@storybook/react': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' },
+        dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' },
       };
       it('should no-op', async () => {
         await expect(
           checkAngular12({
             packageJson,
-            main: {},
           })
         ).resolves.toBeFalsy();
       });

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

@@ -1,15 +1,14 @@
 import chalk from 'chalk';
 import { dedent } from 'ts-dedent';
 import semver from 'semver';
-import type { ConfigFile } from '@storybook/csf-tools';
 import type { Fix } from '../types';
 import { webpack5 } from './webpack5';
+import { checkWebpack5Builder } from '../helpers/checkWebpack5Builder';
 
 interface Angular12RunOptions {
   angularVersion: string;
   // FIXME angularPresetVersion: string;
   storybookVersion: string;
-  main: ConfigFile;
 }
 
 /**
@@ -21,17 +20,16 @@ interface Angular12RunOptions {
 export const angular12: Fix<Angular12RunOptions> = {
   id: 'angular12',
 
-  async check({ packageManager }) {
-    const packageJson = packageManager.retrievePackageJson();
-    const { dependencies, devDependencies } = packageJson;
-    const angularVersion = dependencies['@angular/core'] || devDependencies['@angular/core'];
+  async check({ packageManager, configDir }) {
+    const allDependencies = packageManager.getAllDependencies();
+    const angularVersion = allDependencies['@angular/core'];
     const angularCoerced = semver.coerce(angularVersion)?.version;
 
     if (!angularCoerced || semver.lt(angularCoerced, '12.0.0')) {
       return null;
     }
 
-    const builderInfo = await webpack5.checkWebpack5Builder(packageJson);
+    const builderInfo = await checkWebpack5Builder({ packageManager, configDir });
     return builderInfo ? { angularVersion, ...builderInfo } : null;
   },
 

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' },