Merge pull request #18905 from storybookjs/fix/remove-dependencies

CLI: fix remove dependencies logic
storybookjs · Aug 10, 2022 · 0c2ee6d · 0c2ee6d
2 parents da08392 + a13e9b4
commit 0c2ee6d
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 4 deletions.
diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts
@@ -154,8 +154,13 @@ export abstract class JsPackageManager {
       const { packageJson } = options;
 
       dependencies.forEach((dep) => {
-        delete packageJson[dep];
-      }, {});
+        if (packageJson.devDependencies) {
+          delete packageJson.devDependencies[dep];
+        }
+        if (packageJson.dependencies) {
+          delete packageJson.dependencies[dep];
+        }
+      });
 
       writePackageJson(packageJson);
     } else {

diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.test.ts b/code/lib/cli/src/js-package-manager/NPMProxy.test.ts
@@ -1,4 +1,5 @@
 import { NPMProxy } from './NPMProxy';
+import * as PackageJsonHelper from './PackageJsonHelper';
 
 describe('NPM Proxy', () => {
   let npmProxy: NPMProxy;
@@ -102,6 +103,34 @@ describe('NPM Proxy', () => {
         );
       });
     });
+    describe('skipInstall', () => {
+      it('should only change package.json without running install', () => {
+        const executeCommandSpy = jest.spyOn(npmProxy, 'executeCommand').mockReturnValue('7.0.0');
+        const writePackageSpy = jest
+          .spyOn(PackageJsonHelper, 'writePackageJson')
+          .mockImplementation(jest.fn);
+
+        npmProxy.removeDependencies(
+          {
+            skipInstall: true,
+            packageJson: {
+              devDependencies: {
+                '@storybook/manager-webpack5': 'x.x.x',
+                '@storybook/react': 'x.x.x',
+              },
+            },
+          },
+          ['@storybook/manager-webpack5']
+        );
+
+        expect(writePackageSpy).toHaveBeenCalledWith({
+          devDependencies: {
+            '@storybook/react': 'x.x.x',
+          },
+        });
+        expect(executeCommandSpy).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('latestVersion', () => {

diff --git a/code/lib/cli/src/js-package-manager/Yarn1Proxy.test.ts b/code/lib/cli/src/js-package-manager/Yarn1Proxy.test.ts
@@ -1,4 +1,5 @@
 import { Yarn1Proxy } from './Yarn1Proxy';
+import * as PackageJsonHelper from './PackageJsonHelper';
 
 describe('Yarn 1 Proxy', () => {
   let yarn1Proxy: Yarn1Proxy;
@@ -58,7 +59,32 @@ describe('Yarn 1 Proxy', () => {
       );
     });
 
-    it.todo('with devDep it should update package json without running yarn remove');
+    it('skipInstall should only change package.json without running install', () => {
+      const executeCommandSpy = jest.spyOn(yarn1Proxy, 'executeCommand').mockReturnValue('7.0.0');
+      const writePackageSpy = jest
+        .spyOn(PackageJsonHelper, 'writePackageJson')
+        .mockImplementation(jest.fn);
+
+      yarn1Proxy.removeDependencies(
+        {
+          skipInstall: true,
+          packageJson: {
+            devDependencies: {
+              '@storybook/manager-webpack5': 'x.x.x',
+              '@storybook/react': 'x.x.x',
+            },
+          },
+        },
+        ['@storybook/manager-webpack5']
+      );
+
+      expect(writePackageSpy).toHaveBeenCalledWith({
+        devDependencies: {
+          '@storybook/react': 'x.x.x',
+        },
+      });
+      expect(executeCommandSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe('latestVersion', () => {

diff --git a/code/lib/cli/src/js-package-manager/Yarn2Proxy.test.ts b/code/lib/cli/src/js-package-manager/Yarn2Proxy.test.ts
@@ -1,4 +1,5 @@
 import { Yarn2Proxy } from './Yarn2Proxy';
+import * as PackageJsonHelper from './PackageJsonHelper';
 
 describe('Yarn 2 Proxy', () => {
   let yarn2Proxy: Yarn2Proxy;
@@ -57,7 +58,33 @@ describe('Yarn 2 Proxy', () => {
         expect.any(String)
       );
     });
-    it.todo('with devDep it should update package json without running yarn remove');
+
+    it('skipInstall should only change package.json without running install', () => {
+      const executeCommandSpy = jest.spyOn(yarn2Proxy, 'executeCommand').mockReturnValue('7.0.0');
+      const writePackageSpy = jest
+        .spyOn(PackageJsonHelper, 'writePackageJson')
+        .mockImplementation(jest.fn);
+
+      yarn2Proxy.removeDependencies(
+        {
+          skipInstall: true,
+          packageJson: {
+            devDependencies: {
+              '@storybook/manager-webpack5': 'x.x.x',
+              '@storybook/react': 'x.x.x',
+            },
+          },
+        },
+        ['@storybook/manager-webpack5']
+      );
+
+      expect(writePackageSpy).toHaveBeenCalledWith({
+        devDependencies: {
+          '@storybook/react': 'x.x.x',
+        },
+      });
+      expect(executeCommandSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe('latestVersion', () => {