CLI: Tests and bugfixes for webpack5 fix

__mocks__/fs-extra.js

@@ -0,0 +1,29 @@
+const fs = jest.createMockFromModule('fs-extra');
+
+// This is a custom function that our tests can use during setup to specify
+// what the files on the "mock" filesystem should look like when any of the
+// `fs` APIs are used.
+let mockFiles = Object.create(null);
+
+// eslint-disable-next-line no-underscore-dangle
+function __setMockFiles(newMockFiles) {
+  mockFiles = newMockFiles;
+}
+
+// A custom version of `readdirSync` that reads from the special mocked out
+// file list set via __setMockFiles
+const readFile = async (filePath) => mockFiles[filePath];
+const readFileSync = (filePath = '') => mockFiles[filePath];
+const existsSync = (filePath) => !!mockFiles[filePath];
+const lstatSync = (filePath) => ({
+  isFile: () => !!mockFiles[filePath],
+});
+
+// eslint-disable-next-line no-underscore-dangle
+fs.__setMockFiles = __setMockFiles;
+fs.readFile = readFile;
+fs.readFileSync = readFileSync;
+fs.existsSync = existsSync;
+fs.lstatSync = lstatSync;
+
+module.exports = fs;

lib/cli/src/fix/fixes/webpack5.test.ts

@@ -0,0 +1,121 @@
+/* eslint-disable no-underscore-dangle */
+import { JsPackageManager } from '../../js-package-manager';
+import { webpack5 } from './webpack5';
+
+// eslint-disable-next-line global-require, jest/no-mocks-import
+jest.mock('fs-extra', () => require('../../../../../__mocks__/fs-extra'));
+
+const checkWebpack5 = async ({ packageJson, main }) => {
+  // eslint-disable-next-line global-require
+  require('fs-extra').__setMockFiles({
+    '.storybook/main.js': `module.exports = ${JSON.stringify(main)};`,
+  });
+  const packageManager = {
+    retrievePackageJson: () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
+  } as JsPackageManager;
+  return webpack5.check({ packageManager });
+};
+
+describe('webpack5 fix', () => {
+  describe('sb < 6.3', () => {
+    describe('webpack5 dependency', () => {
+      const packageJson = { dependencies: { '@storybook/react': '^6.2.0', webpack: '^5.0.0' } };
+      it('should fail', async () => {
+        await expect(
+          checkWebpack5({
+            packageJson,
+            main: {},
+          })
+        ).rejects.toThrow();
+      });
+    });
+    describe('no webpack5 dependency', () => {
+      const packageJson = { dependencies: { '@storybook/react': '^6.2.0' } };
+      it('should no-op', async () => {
+        await expect(
+          checkWebpack5({
+            packageJson,
+            main: {},
+          })
+        ).resolves.toBeFalsy();
+      });
+    });
+  });
+  describe('sb 6.3 - 7.0', () => {
+    describe('webpack5 dependency', () => {
+      const packageJson = { dependencies: { '@storybook/react': '^6.3.0', webpack: '^5.0.0' } };
+      describe('webpack5 builder', () => {
+        it('should no-op', async () => {
+          await expect(
+            checkWebpack5({
+              packageJson,
+              main: { core: { builder: 'webpack5' } },
+            })
+          ).resolves.toBeFalsy();
+        });
+      });
+      describe('custom builder', () => {
+        it('should no-op', async () => {
+          await expect(
+            checkWebpack5({
+              packageJson,
+              main: { core: { builder: 'storybook-builder-vite' } },
+            })
+          ).resolves.toBeFalsy();
+        });
+      });
+      describe('no webpack5 builder', () => {
+        it('should add webpack5 builder', async () => {
+          await expect(
+            checkWebpack5({
+              packageJson,
+              main: {},
+            })
+          ).resolves.toMatchObject({
+            webpackVersion: '^5.0.0',
+            storybookVersion: '^6.3.0',
+          });
+        });
+      });
+    });
+    describe('no webpack dependency', () => {
+      it('should no-op', async () => {
+        await expect(
+          checkWebpack5({
+            packageJson: {},
+            main: {},
+          })
+        ).resolves.toBeFalsy();
+      });
+    });
+    describe('webpack4 dependency', () => {
+      it('should no-op', async () => {
+        await expect(
+          checkWebpack5({
+            packageJson: {
+              dependencies: {
+                webpack: '4',
+              },
+            },
+            main: {},
+          })
+        ).resolves.toBeFalsy();
+      });
+    });
+  });
+  describe('sb 7.0+', () => {
+    describe('webpack5 dependency', () => {
+      const packageJson = {
+        dependencies: { '@storybook/react': '^7.0.0-alpha.0', webpack: '^5.0.0' },
+      };
+      it('should no-op', async () => {
+        await expect(
+          checkWebpack5({
+            packageJson,
+            main: {},
+          })
+        ).resolves.toBeFalsy();
+      });
+    });
+  });
+});

lib/cli/src/fix/fixes/webpack5.ts

@@ -36,13 +36,14 @@ export const webpack5: Fix<Webpack5RunOptions> & CheckBuilder = {
 
   async checkWebpack5Builder(packageJson: PackageJsonWithDepsAndDevDeps) {
     const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson);
+
     const storybookCoerced = semver.coerce(storybookVersion).version;
     if (!storybookCoerced) {
       logger.warn(`Unable to determine storybook version, skipping webpack5 fix.`);
       return null;
     }
 
-    if (semver.lte(storybookCoerced, '6.3.0')) {
+    if (semver.lt(storybookCoerced, '6.3.0')) {
       logger.warn(
         'Detected SB 6.3 or below, please upgrade storybook if you want to use webpack5.'
       );
@@ -57,7 +58,6 @@ export const webpack5: Fix<Webpack5RunOptions> & CheckBuilder = {
       logger.warn('Unable to find storybook main.js config');
       return null;
     }
-
     const main = await readConfig(mainConfig);
     const builder = main.getFieldValue(['core', 'builder']);
     if (builder && builder !== 'webpack4') {
@@ -77,8 +77,8 @@ export const webpack5: Fix<Webpack5RunOptions> & CheckBuilder = {
 
     if (
       !webpackCoerced ||
-      semver.gte(webpackCoerced, '5.0.0') ||
-      semver.lt(webpackCoerced, '6.0.0')
+      semver.lt(webpackCoerced, '5.0.0') ||
+      semver.gte(webpackCoerced, '6.0.0')
     )
       return null;
 

lib/csf-tools/src/ConfigFile.ts

@@ -7,15 +7,19 @@ import { babelParse } from './babelParse';
 
 const logger = console;
 
+const propKey = (p: t.ObjectProperty) => {
+  if (t.isIdentifier(p.key)) return p.key.name;
+  if (t.isStringLiteral(p.key)) return p.key.value;
+  return null;
+};
+
 const _getPath = (path: string[], node: t.Node): t.Node | undefined => {
   if (path.length === 0) {
     return node;
   }
   if (t.isObjectExpression(node)) {
     const [first, ...rest] = path;
-    const field = node.properties.find((p: t.ObjectProperty) => {
-      return t.isIdentifier(p.key) && p.key.name === first;
-    });
+    const field = node.properties.find((p: t.ObjectProperty) => propKey(p) === first);
     if (field) {
       return _getPath(rest, (field as t.ObjectProperty).value);
     }
@@ -60,9 +64,9 @@ const _makeObjectExpression = (path: string[], value: t.Expression): t.Expressio
 
 const _updateExportNode = (path: string[], expr: t.Expression, existing: t.ObjectExpression) => {
   const [first, ...rest] = path;
-  const existingField = existing.properties.find((p: t.ObjectProperty) => {
-    return t.isIdentifier(p.key) && p.key.name === first;
-  }) as t.ObjectProperty;
+  const existingField = existing.properties.find(
+    (p: t.ObjectProperty) => propKey(p) === first
+  ) as t.ObjectProperty;
   if (!existingField) {
     existing.properties.push(
       t.objectProperty(t.identifier(first), _makeObjectExpression(rest, expr))
@@ -125,12 +129,13 @@ export class ConfigFile {
               if (t.isObjectExpression(right)) {
                 self._exportsObject = right;
                 right.properties.forEach((p: t.ObjectProperty) => {
-                  if (t.isIdentifier(p.key)) {
+                  const exportName = propKey(p);
+                  if (exportName) {
                     let exportVal = p.value;
                     if (t.isIdentifier(exportVal)) {
                       exportVal = _findVarInitialization(exportVal.name, parent as t.Program);
                     }
-                    self._exports[p.key.name] = exportVal as t.Expression;
+                    self._exports[exportName] = exportVal as t.Expression;
                   }
                 });
               } else {