From 8bde48fc7a33101929534562e2d8fff2065ec54a Mon Sep 17 00:00:00 2001 From: Austin Fahsl Date: Fri, 1 Mar 2024 10:15:55 -0700 Subject: [PATCH] fix(release): skip lock file update if workspaces are not enabled (#22055) --- docs/generated/devkit/README.md | 1 + docs/generated/devkit/isWorkspacesEnabled.md | 16 +++++++ .../packages/devkit/documents/nx_devkit.md | 1 + e2e/release/src/independent-projects.test.ts | 28 +++---------- e2e/release/src/lock-file-updates.test.ts | 39 +++++++++++++++++ .../release-version/utils/update-lock-file.ts | 11 +++++ packages/nx/src/devkit-exports.ts | 1 + packages/nx/src/utils/package-manager.spec.ts | 42 +++++++++++++++++++ packages/nx/src/utils/package-manager.ts | 21 +++++++++- 9 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 docs/generated/devkit/isWorkspacesEnabled.md diff --git a/docs/generated/devkit/README.md b/docs/generated/devkit/README.md index b0f2fa2468f5d..25f19c02c7193 100644 --- a/docs/generated/devkit/README.md +++ b/docs/generated/devkit/README.md @@ -122,6 +122,7 @@ It only uses language primitives and immutable objects - [glob](../../devkit/documents/glob) - [hashArray](../../devkit/documents/hashArray) - [installPackagesTask](../../devkit/documents/installPackagesTask) +- [isWorkspacesEnabled](../../devkit/documents/isWorkspacesEnabled) - [joinPathFragments](../../devkit/documents/joinPathFragments) - [moveFilesToNewDirectory](../../devkit/documents/moveFilesToNewDirectory) - [names](../../devkit/documents/names) diff --git a/docs/generated/devkit/isWorkspacesEnabled.md b/docs/generated/devkit/isWorkspacesEnabled.md new file mode 100644 index 0000000000000..3223208dd1b0a --- /dev/null +++ b/docs/generated/devkit/isWorkspacesEnabled.md @@ -0,0 +1,16 @@ +# Function: isWorkspacesEnabled + +▸ **isWorkspacesEnabled**(`packageManager?`, `root?`): `boolean` + +Returns true if the workspace is using npm workspaces, yarn workspaces, or pnpm workspaces. + +#### Parameters + +| Name | Type | Default value | Description | +| :--------------- | :-------------------------------------------------------- | :-------------- | :------------------------------------------------------------------------------------------ | +| `packageManager` | [`PackageManager`](../../devkit/documents/PackageManager) | `undefined` | The package manager to use. If not provided, it will be detected based on the lock file. | +| `root` | `string` | `workspaceRoot` | The directory the commands will be ran inside of. Defaults to the current workspace's root. | + +#### Returns + +`boolean` diff --git a/docs/generated/packages/devkit/documents/nx_devkit.md b/docs/generated/packages/devkit/documents/nx_devkit.md index b0f2fa2468f5d..25f19c02c7193 100644 --- a/docs/generated/packages/devkit/documents/nx_devkit.md +++ b/docs/generated/packages/devkit/documents/nx_devkit.md @@ -122,6 +122,7 @@ It only uses language primitives and immutable objects - [glob](../../devkit/documents/glob) - [hashArray](../../devkit/documents/hashArray) - [installPackagesTask](../../devkit/documents/installPackagesTask) +- [isWorkspacesEnabled](../../devkit/documents/isWorkspacesEnabled) - [joinPathFragments](../../devkit/documents/joinPathFragments) - [moveFilesToNewDirectory](../../devkit/documents/moveFilesToNewDirectory) - [names](../../devkit/documents/names) diff --git a/e2e/release/src/independent-projects.test.ts b/e2e/release/src/independent-projects.test.ts index 47d33fc1e9a6d..8d1cc646ecbf5 100644 --- a/e2e/release/src/independent-projects.test.ts +++ b/e2e/release/src/independent-projects.test.ts @@ -139,9 +139,6 @@ describe('nx release - independent projects', () => { "scripts": { - NX Updating {package-manager} lock file - - NX Staging changed files with git @@ -174,9 +171,6 @@ describe('nx release - independent projects', () => { + - NX Updating {package-manager} lock file - - NX Staging changed files with git @@ -213,9 +207,6 @@ describe('nx release - independent projects', () => { } - NX Updating {package-manager} lock file - - NX Staging changed files with git @@ -255,15 +246,12 @@ describe('nx release - independent projects', () => { "scripts": { - NX Updating {package-manager} lock file - - Updating {lock-file} with the following command: - {lock-file-command} + Skipped lock file update because {package-manager} workspaces are not enabled. NX Committing changes with git Staging files in git with the following command: - git add {project-name}/package.json {lock-file} + git add {project-name}/package.json Committing files in git with the following command: git commit --message chore(release): publish --message - project: {project-name} 999.9.9-version-git-operations-test.2 @@ -363,20 +351,14 @@ describe('nx release - independent projects', () => { "scripts": { - NX Updating {package-manager} lock file - - Updating {lock-file} with the following command: - {lock-file-command} - - NX Updating {package-manager} lock file + Skipped lock file update because {package-manager} workspaces are not enabled. - Updating {lock-file} with the following command: - {lock-file-command} + Skipped lock file update because {package-manager} workspaces are not enabled. NX Committing changes with git Staging files in git with the following command: - git add {project-name}/package.json {project-name}/package.json {project-name}/package.json {lock-file} + git add {project-name}/package.json {project-name}/package.json {project-name}/package.json Committing files in git with the following command: git commit --message chore(release): publish --message - project: {project-name} 999.9.9-version-git-operations-test.3 --message - project: {project-name} 999.9.9-version-git-operations-test.3 --message - release-group: fixed 999.9.9-version-git-operations-test.3 diff --git a/e2e/release/src/lock-file-updates.test.ts b/e2e/release/src/lock-file-updates.test.ts index 122edc90b716c..55da4e278105a 100644 --- a/e2e/release/src/lock-file-updates.test.ts +++ b/e2e/release/src/lock-file-updates.test.ts @@ -93,6 +93,11 @@ describe('nx release lock file updates', () => { it('should update package-lock.json when package manager is npm', async () => { initializeProject('npm'); + updateJson('package.json', (json) => { + json.workspaces = [pkg1, pkg2, pkg3]; + return json; + }); + runCommand(`npm install`); // workaround for NXC-143 @@ -116,6 +121,40 @@ describe('nx release lock file updates', () => { `); }); + it('should not update package-lock.json when package manager is npm and workspaces are not enabled', async () => { + initializeProject('npm'); + + updateJson('package.json', (json) => { + delete json.workspaces; + return json; + }); + + runCommand(`npm install`); + + // workaround for NXC-143 + runCLI('reset'); + + runCommand(`git add .`); + runCommand(`git commit -m "chore: initial commit"`); + + const versionOutput = runCLI(`release version 999.9.9 --verbose`); + + expect( + versionOutput.match( + /Skipped lock file update because npm workspaces are not enabled./g + ).length + ).toBe(1); + + const filesChanges = runCommand('git diff --name-only HEAD'); + + expect(filesChanges).toMatchInlineSnapshot(` + {project-name}/package.json + {project-name}/package.json + {project-name}/package.json + + `); + }); + it('should not update lock file when package manager is yarn classic', async () => { initializeProject('yarn'); diff --git a/packages/js/src/generators/release-version/utils/update-lock-file.ts b/packages/js/src/generators/release-version/utils/update-lock-file.ts index 9eb4c5ffa65bb..94ac233a757e0 100644 --- a/packages/js/src/generators/release-version/utils/update-lock-file.ts +++ b/packages/js/src/generators/release-version/utils/update-lock-file.ts @@ -2,6 +2,7 @@ import { detectPackageManager, getPackageManagerCommand, getPackageManagerVersion, + isWorkspacesEnabled, output, } from '@nx/devkit'; import { execSync } from 'child_process'; @@ -46,6 +47,16 @@ export async function updateLockFile( return []; } + const workspacesEnabled = isWorkspacesEnabled(packageManager, cwd); + if (!workspacesEnabled) { + if (verbose) { + console.log( + `\nSkipped lock file update because ${packageManager} workspaces are not enabled.` + ); + } + return []; + } + const isDaemonEnabled = daemonClient.enabled(); if (!dryRun && isDaemonEnabled) { // if not in dry-run temporarily stop the daemon, as it will error if the lock file is updated diff --git a/packages/nx/src/devkit-exports.ts b/packages/nx/src/devkit-exports.ts index cd2b92ec18efe..4680022e19875 100644 --- a/packages/nx/src/devkit-exports.ts +++ b/packages/nx/src/devkit-exports.ts @@ -100,6 +100,7 @@ export { getPackageManagerCommand, detectPackageManager, getPackageManagerVersion, + isWorkspacesEnabled, } from './utils/package-manager'; /** diff --git a/packages/nx/src/utils/package-manager.spec.ts b/packages/nx/src/utils/package-manager.spec.ts index f89d71345267b..3ba3c7d8acfe1 100644 --- a/packages/nx/src/utils/package-manager.spec.ts +++ b/packages/nx/src/utils/package-manager.spec.ts @@ -1,7 +1,9 @@ import * as fs from 'fs'; import * as configModule from '../config/configuration'; +import * as projectGraphFileUtils from '../project-graph/file-utils'; import { detectPackageManager, + isWorkspacesEnabled, modifyYarnRcToFitNewDirectory, modifyYarnRcYmlToFitNewDirectory, } from './package-manager'; @@ -76,6 +78,46 @@ describe('package-manager', () => { }); }); + describe('isWorkspacesEnabled', () => { + it('should return true if package manager is pnpm and pnpm-workspace.yaml exists', () => { + jest.spyOn(fs, 'existsSync').mockReturnValueOnce(true); + expect(isWorkspacesEnabled('pnpm')).toEqual(true); + }); + + it('should return false if package manager is pnpm and pnpm-workspace.yaml does not exist', () => { + jest.spyOn(fs, 'existsSync').mockReturnValueOnce(false); + expect(isWorkspacesEnabled('pnpm')).toEqual(false); + }); + + it('should return true if package manager is yarn and workspaces exists in package.json', () => { + jest + .spyOn(projectGraphFileUtils, 'readPackageJson') + .mockReturnValueOnce({ workspaces: ['packages/*'] }); + expect(isWorkspacesEnabled('yarn')).toEqual(true); + }); + + it('should return false if package manager is yarn and workspaces does not exist in package.json', () => { + jest + .spyOn(projectGraphFileUtils, 'readPackageJson') + .mockReturnValueOnce({}); + expect(isWorkspacesEnabled('yarn')).toEqual(false); + }); + + it('should return true if package manager is npm and workspaces exists in package.json', () => { + jest + .spyOn(projectGraphFileUtils, 'readPackageJson') + .mockReturnValueOnce({ workspaces: ['packages/*'] }); + expect(isWorkspacesEnabled('npm')).toEqual(true); + }); + + it('should return false if package manager is npm and workspaces does not exist in package.json', () => { + jest + .spyOn(projectGraphFileUtils, 'readPackageJson') + .mockReturnValueOnce({}); + expect(isWorkspacesEnabled('npm')).toEqual(false); + }); + }); + describe('modifyYarnRcYmlToFitNewDirectory', () => { it('should update paths properly', () => { expect( diff --git a/packages/nx/src/utils/package-manager.ts b/packages/nx/src/utils/package-manager.ts index 5075cd959f4b7..eebfc8d33fedf 100644 --- a/packages/nx/src/utils/package-manager.ts +++ b/packages/nx/src/utils/package-manager.ts @@ -6,8 +6,9 @@ import { gte, lt } from 'semver'; import { dirSync } from 'tmp'; import { promisify } from 'util'; import { readNxJson } from '../config/configuration'; +import { readPackageJson } from '../project-graph/file-utils'; import { readFileIfExisting, writeJsonFile } from './fileutils'; -import { readModulePackageJson } from './package-json'; +import { PackageJson, readModulePackageJson } from './package-json'; import { workspaceRoot } from './workspace-root'; const execAsync = promisify(exec); @@ -43,6 +44,24 @@ export function detectPackageManager(dir: string = ''): PackageManager { ); } +/** + * Returns true if the workspace is using npm workspaces, yarn workspaces, or pnpm workspaces. + * @param packageManager The package manager to use. If not provided, it will be detected based on the lock file. + * @param root The directory the commands will be ran inside of. Defaults to the current workspace's root. + */ +export function isWorkspacesEnabled( + packageManager: PackageManager = detectPackageManager(), + root: string = workspaceRoot +): boolean { + if (packageManager === 'pnpm') { + return existsSync(join(root, 'pnpm-workspace.yaml')); + } + + // yarn and pnpm both use the same 'workspaces' property in package.json + const packageJson: PackageJson = readPackageJson(); + return !!packageJson?.workspaces; +} + /** * Returns commands for the package manager used in the workspace. * By default, the package manager is derived based on the lock file,