Merge pull request #25474 from storybookjs/jeppe/fix-package-manager-…

…issues CLI: Fix using wrong package managers in existing projects
storybookjs · Jan 9, 2024 · f854254 · f854254
2 parents c6ffc8f + b2aa22e
commit f854254
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 11 deletions.
diff --git a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts b/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.test.ts
@@ -17,6 +17,7 @@ const findUpSyncMock = vi.mocked(findUpSync);
 describe('CLASS: JsPackageManagerFactory', () => {
   beforeEach(() => {
     findUpSyncMock.mockReturnValue(undefined);
+    spawnSyncMock.mockReturnValue({ status: 1 } as any);
     delete process.env.npm_config_user_agent;
   });
 

diff --git a/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts b/code/lib/cli/src/js-package-manager/JsPackageManagerFactory.ts
@@ -30,22 +30,15 @@ export class JsPackageManagerFactory {
       return new this.PROXY_MAP[force]({ cwd });
     }
 
-    // Option 2: If the user is running a command via npx/pnpx/yarn create/etc, we infer the package manager from the command
-    const inferredPackageManager = this.inferPackageManagerFromUserAgent();
-    if (inferredPackageManager && inferredPackageManager in this.PROXY_MAP) {
-      return new this.PROXY_MAP[inferredPackageManager]({ cwd });
-    }
-
-    // Option 3: We try to infer the package manager from the closest lockfile
-    const yarnVersion = getYarnVersion(cwd);
-
+    // Option 2: We try to infer the package manager from the closest lockfile
     const closestLockfilePath = findUpSync([YARN_LOCKFILE, PNPM_LOCKFILE, NPM_LOCKFILE], {
       cwd,
     });
     const closestLockfile = closestLockfilePath && path.basename(closestLockfilePath);
 
     const hasNPMCommand = hasNPM(cwd);
     const hasPNPMCommand = hasPNPM(cwd);
+    const yarnVersion = getYarnVersion(cwd);
 
     if (yarnVersion && (closestLockfile === YARN_LOCKFILE || (!hasNPMCommand && !hasPNPMCommand))) {
       return yarnVersion === 1 ? new Yarn1Proxy({ cwd }) : new Yarn2Proxy({ cwd });
@@ -55,6 +48,18 @@ export class JsPackageManagerFactory {
       return new PNPMProxy({ cwd });
     }
 
+    if (hasNPMCommand && closestLockfile === NPM_LOCKFILE) {
+      return new NPMProxy({ cwd });
+    }
+
+    // Option 3: If the user is running a command via npx/pnpx/yarn create/etc, we infer the package manager from the command
+    const inferredPackageManager = this.inferPackageManagerFromUserAgent();
+    if (inferredPackageManager && inferredPackageManager in this.PROXY_MAP) {
+      return new this.PROXY_MAP[inferredPackageManager]({ cwd });
+    }
+
+    // Default fallback, whenever users try to use something different than NPM, PNPM, Yarn,
+    // but still have NPM installed
     if (hasNPMCommand) {
       return new NPMProxy({ cwd });
     }

diff --git a/code/lib/cli/src/upgrade.ts b/code/lib/cli/src/upgrade.ts
@@ -190,7 +190,7 @@ export const doUpgrade = async ({
     shell: true,
   });
 
-  if (check.stderr && !check.stderr.toString().includes('npm notice')) {
+  if (check.stderr && check.stderr.toString().includes('npm ERR')) {
     throw new UpgradeStorybookPackagesError({
       command,
       args: checkArgs,
@@ -208,7 +208,7 @@ export const doUpgrade = async ({
   logger.info(checkSb.stdout.toString());
   logger.info(checkSb.stderr.toString());
 
-  if (checkSb.stderr && !checkSb.stderr.toString().includes('npm notice')) {
+  if (checkSb.stderr && checkSb.stderr.toString().includes('npm ERR')) {
     throw new UpgradeStorybookPackagesError({
       command,
       args: checkSbArgs,