From 93bcd5209c9b0cb6271b2de9dccccc00681fba14 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 9 Aug 2022 14:12:29 +0200 Subject: [PATCH 1/2] CLI: fix remove dependencies logic --- .../js-package-manager/JsPackageManager.ts | 2 +- .../src/js-package-manager/NPMProxy.test.ts | 29 +++++++++++++++++++ .../src/js-package-manager/Yarn1Proxy.test.ts | 28 +++++++++++++++++- .../src/js-package-manager/Yarn2Proxy.test.ts | 29 ++++++++++++++++++- 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index efd00173c627..1a0ab0f41510 100644 --- a/code/lib/cli/src/js-package-manager/JsPackageManager.ts +++ b/code/lib/cli/src/js-package-manager/JsPackageManager.ts @@ -154,7 +154,7 @@ export abstract class JsPackageManager { const { packageJson } = options; dependencies.forEach((dep) => { - delete packageJson[dep]; + delete packageJson.devDependencies[dep]; }, {}); writePackageJson(packageJson); diff --git a/code/lib/cli/src/js-package-manager/NPMProxy.test.ts b/code/lib/cli/src/js-package-manager/NPMProxy.test.ts index dd4da9507d0e..789e1f6ee53e 100644 --- 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 index b705c9031640..c6655cd597bb 100644 --- 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 index 74d4e7cf01af..30c90f9be32b 100644 --- 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', () => { From a13e9b44f7dafdd3220977a41c350f3bb4f422d0 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Tue, 9 Aug 2022 15:09:16 +0200 Subject: [PATCH 2/2] fix: remode dependencies as well --- code/lib/cli/src/js-package-manager/JsPackageManager.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/code/lib/cli/src/js-package-manager/JsPackageManager.ts b/code/lib/cli/src/js-package-manager/JsPackageManager.ts index 1a0ab0f41510..e9d517b715a3 100644 --- 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.devDependencies[dep]; - }, {}); + if (packageJson.devDependencies) { + delete packageJson.devDependencies[dep]; + } + if (packageJson.dependencies) { + delete packageJson.dependencies[dep]; + } + }); writePackageJson(packageJson); } else {