Merge pull request #22642 from storybookjs/valentin/invalid-version-null

CLI: Fix "Invalid version null" issues by improved version detection

code/frameworks/nextjs/src/utils.ts

@@ -94,7 +94,15 @@ export const addScopedAlias = (baseConfig: WebpackConfig, name: string, alias?:
  * scopedResolve('styled-jsx') === '/some/path/node_modules/styled-jsx'
  */
 export const scopedResolve = (id: string): string => {
-  const scopedModulePath = require.resolve(id, { paths: [path.resolve()] });
+  let scopedModulePath;
+
+  try {
+    // TODO: Remove in next major release (SB 8.0) and use the statement in the catch block per default instead
+    scopedModulePath = require.resolve(id, { paths: [path.resolve()] });
+  } catch (e) {
+    scopedModulePath = require.resolve(id);
+  }
+
   const moduleFolderStrPosition = scopedModulePath.lastIndexOf(
     id.replace(/\//g /* all '/' occurances */, path.sep)
   );

code/lib/cli/package.json

@@ -65,6 +65,8 @@
     "@storybook/telemetry": "7.1.0-alpha.35",
     "@storybook/types": "7.1.0-alpha.35",
     "@types/semver": "^7.3.4",
+    "@yarnpkg/fslib": "^2.10.3",
+    "@yarnpkg/libzip": "^2.3.0",
     "chalk": "^4.1.0",
     "commander": "^6.2.1",
     "cross-spawn": "^7.0.3",

code/lib/cli/src/add.ts

@@ -11,6 +11,7 @@ import {
   useNpmWarning,
   type PackageManagerName,
 } from './js-package-manager';
+import { getStorybookVersion } from './utils';
 
 const logger = console;
 
@@ -83,7 +84,7 @@ export async function add(
   const packageJson = await packageManager.retrievePackageJson();
   const [addonName, versionSpecifier] = getVersionSpecifier(addon);
 
-  const { mainConfig, version: storybookVersion } = getStorybookInfo(packageJson);
+  const { mainConfig } = getStorybookInfo(packageJson);
   if (!mainConfig) {
     logger.error('Unable to find storybook main.js config');
     return;
@@ -97,8 +98,9 @@ export async function add(
 
   // add to package.json
   const isStorybookAddon = addonName.startsWith('@storybook/');
+  const storybookVersion = await getStorybookVersion(packageManager);
   const version = versionSpecifier || (isStorybookAddon ? storybookVersion : latestVersion);
-  const addonWithVersion = `${addonName}@${version}`;
+  const addonWithVersion = `${addonName}@^${version}`;
   logger.log(`Installing ${addonWithVersion}`);
   await packageManager.addDependencies({ installAsDevDependencies: true }, [addonWithVersion]);
 

code/lib/cli/src/automigrate/fixes/add-react.test.ts

@@ -1,11 +1,17 @@
+import type { StorybookConfig } from '@storybook/types';
 import type { JsPackageManager, PackageJson } from '../../js-package-manager';
 import { addReact } from './add-react';
 
 const checkAddReact = async (packageJson: PackageJson) => {
   const packageManager = {
     retrievePackageJson: async () => ({ dependencies: {}, devDependencies: {}, ...packageJson }),
   } as JsPackageManager;
-  return addReact.check({ packageManager });
+
+  return addReact.check({
+    packageManager,
+    mainConfig: {} as StorybookConfig,
+    storybookVersion: '7.0.0',
+  });
 };
 
 describe('addReact fix', () => {

code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.test.ts

@@ -1,25 +1,20 @@
 import type { StorybookConfig } from '@storybook/types';
-import type { PackageJson } from '../../js-package-manager';
-import { makePackageManager, mockStorybookData } from '../helpers/testing-helpers';
+import type { JsPackageManager } from '../../js-package-manager';
 import { angularBuildersMultiproject } from './angular-builders-multiproject';
 import * as helpers from '../../helpers';
 import * as angularHelpers from '../../generators/ANGULAR/helpers';
 
 const checkAngularBuilders = async ({
-  packageJson,
-  main: mainConfig = {},
-  storybookVersion = '7.0.0',
+  packageManager,
+  mainConfig = {},
 }: {
-  packageJson: PackageJson;
-  main?: Partial<StorybookConfig> & Record<string, unknown>;
-  storybookVersion?: string;
+  packageManager: Partial<JsPackageManager>;
+  mainConfig?: Partial<StorybookConfig>;
 }) => {
-  mockStorybookData({ mainConfig, storybookVersion });
-
-  // mock file system (look at eslint plugin test)
-
   return angularBuildersMultiproject.check({
-    packageManager: makePackageManager(packageJson),
+    packageManager: packageManager as any,
+    mainConfig: mainConfig as any,
+    storybookVersion: '7.0.0',
   });
 };
 
@@ -34,51 +29,69 @@ jest.mock('../../generators/ANGULAR/helpers', () => ({
 }));
 
 describe('is Nx project', () => {
+  const packageManager = {
+    getPackageVersion: () => {
+      return null;
+    },
+  } as Partial<JsPackageManager>;
+
   beforeEach(() => {
-    (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(true);
+    (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(true);
   });
 
   it('should return null', async () => {
-    const packageJson = {
-      dependencies: { '@storybook/angular': '^7.0.0-alpha.0' },
-    };
-
-    await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
+    await expect(checkAngularBuilders({ packageManager })).resolves.toBeNull();
   });
 });
 
 describe('is not Nx project', () => {
   beforeEach(() => {
-    (helpers.isNxProject as any as jest.SpyInstance).mockReturnValue(false);
+    (helpers.isNxProject as any as jest.SpyInstance).mockResolvedValue(false);
   });
 
   describe('angular builders', () => {
     afterEach(jest.restoreAllMocks);
 
     describe('Angular not found', () => {
-      const packageJson = {
-        dependencies: { '@storybook/angular': '^7.0.0-alpha.0' },
-      };
+      const packageManager = {
+        getPackageVersion: jest.fn().mockResolvedValue(null),
+      } as Partial<JsPackageManager>;
 
       it('should return null', async () => {
-        await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
+        await expect(
+          checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } })
+        ).resolves.toBeNull();
       });
     });
 
     describe('Angular < 14.0.0', () => {
-      const packageJson = {
-        dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^12.0.0' },
-      };
+      const packageManager = {
+        getPackageVersion: (packageName) => {
+          if (packageName === '@angular/core') {
+            return Promise.resolve('12.0.0');
+          }
+
+          return null;
+        },
+      } as Partial<JsPackageManager>;
 
       it('should return null', async () => {
-        await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
+        await expect(
+          checkAngularBuilders({ packageManager, mainConfig: { framework: '@storybook/angular' } })
+        ).resolves.toBeNull();
       });
     });
 
     describe('Angular >= 14.0.0', () => {
-      const packageJson = {
-        dependencies: { '@storybook/angular': '^7.0.0-alpha.0', '@angular/core': '^15.0.0' },
-      };
+      const packageManager = {
+        getPackageVersion: (packageName) => {
+          if (packageName === '@angular/core') {
+            return Promise.resolve('15.0.0');
+          }
+
+          return null;
+        },
+      } as Partial<JsPackageManager>;
 
       describe('has one Storybook builder defined', () => {
         beforeEach(() => {
@@ -89,7 +102,12 @@ describe('is not Nx project', () => {
         });
 
         it('should return null', async () => {
-          await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
+          await expect(
+            checkAngularBuilders({
+              packageManager,
+              mainConfig: { framework: '@storybook/angular' },
+            })
+          ).resolves.toBeNull();
         });
       });
 
@@ -106,7 +124,12 @@ describe('is not Nx project', () => {
         });
 
         it('should return null', async () => {
-          await expect(checkAngularBuilders({ packageJson })).resolves.toBeNull();
+          await expect(
+            checkAngularBuilders({
+              packageManager,
+              mainConfig: { framework: '@storybook/angular' },
+            })
+          ).resolves.toBeNull();
         });
       });
 
@@ -124,7 +147,12 @@ describe('is not Nx project', () => {
         });
 
         it('should return an empty object', async () => {
-          await expect(checkAngularBuilders({ packageJson })).resolves.toMatchObject({});
+          await expect(
+            checkAngularBuilders({
+              packageManager,
+              mainConfig: { framework: '@storybook/angular' },
+            })
+          ).resolves.toMatchObject({});
         });
       });
     });

code/lib/cli/src/automigrate/fixes/angular-builders-multiproject.ts

@@ -4,6 +4,7 @@ import chalk from 'chalk';
 import type { Fix } from '../types';
 import { isNxProject } from '../../helpers';
 import { AngularJSON } from '../../generators/ANGULAR/helpers';
+import { getFrameworkPackageName } from '../helpers/mainConfigFile';
 
 // eslint-disable-next-line @typescript-eslint/no-empty-interface
 interface AngularBuildersMultiprojectRunOptions {}
@@ -12,25 +13,17 @@ export const angularBuildersMultiproject: Fix<AngularBuildersMultiprojectRunOpti
   id: 'angular-builders-multiproject',
   promptOnly: true,
 
-  async check({ packageManager }) {
-    const packageJSON = await packageManager.retrievePackageJson();
-
+  async check({ packageManager, mainConfig }) {
     // Skip in case of NX
-    if (isNxProject(packageJSON)) {
-      return null;
-    }
-    const allDependencies = await packageManager.getAllDependencies();
-
-    const angularVersion = allDependencies['@angular/core'];
-    const angularCoerced = semver.coerce(angularVersion)?.version;
-
-    // skip non-angular projects
-    if (!angularCoerced) {
-      return null;
-    }
-
-    // Is Angular version lower than 14? -> throw an error (only supports ng 14)
-    if (semver.lt(angularCoerced, '14.0.0')) {
+    const angularVersion = await packageManager.getPackageVersion('@angular/core');
+    const frameworkPackageName = getFrameworkPackageName(mainConfig);
+
+    if (
+      (await isNxProject(packageManager)) ||
+      frameworkPackageName !== '@storybook/angular' ||
+      !angularVersion ||
+      semver.lt(angularVersion, '14.0.0')
+    ) {
       return null;
     }