From 431fe2a021e3768bdf903c46dd7386b91cfcb8fe Mon Sep 17 00:00:00 2001 From: James Henry Date: Tue, 10 Sep 2024 17:56:56 +0400 Subject: [PATCH] feat(release): allow local dependency version protocols to be preserved, pnpm publish support (#27787) --- e2e/release/src/custom-registries.test.ts | 9 +- .../release-publish/release-publish.impl.ts | 64 ++++-- .../release-version/release-version.spec.ts | 212 ++++++++++++++++++ .../release-version/release-version.ts | 59 +---- .../is-locally-linked-package-version.ts | 47 ++++ .../nx/src/command-line/release/version.ts | 6 + 6 files changed, 334 insertions(+), 63 deletions(-) create mode 100644 packages/js/src/utils/is-locally-linked-package-version.ts diff --git a/e2e/release/src/custom-registries.test.ts b/e2e/release/src/custom-registries.test.ts index 2bf88f66bce38..e273b81bcee1b 100644 --- a/e2e/release/src/custom-registries.test.ts +++ b/e2e/release/src/custom-registries.test.ts @@ -17,14 +17,21 @@ describe('nx release - custom npm registries', () => { const verdaccioPort = 7191; const customRegistryUrl = `http://localhost:${verdaccioPort}`; const scope = 'scope'; + let previousPackageManager: string; beforeAll(async () => { + previousPackageManager = process.env.SELECTED_PM; + // We are testing some more advanced scoped registry features that only npm has within this file + process.env.SELECTED_PM = 'npm'; newProject({ unsetProjectNameAndRootFormat: false, packages: ['@nx/js'], }); }, 60000); - afterAll(() => cleanupProject()); + afterAll(() => { + cleanupProject(); + process.env.SELECTED_PM = previousPackageManager; + }); it('should respect registry configuration for each package', async () => { updateJson('nx.json', (nxJson) => { diff --git a/packages/js/src/executors/release-publish/release-publish.impl.ts b/packages/js/src/executors/release-publish/release-publish.impl.ts index 476d009448e3e..b19a5ae70fd00 100644 --- a/packages/js/src/executors/release-publish/release-publish.impl.ts +++ b/packages/js/src/executors/release-publish/release-publish.impl.ts @@ -1,12 +1,17 @@ -import { ExecutorContext, readJsonFile } from '@nx/devkit'; +import { + detectPackageManager, + ExecutorContext, + readJsonFile, +} from '@nx/devkit'; import { execSync } from 'child_process'; import { env as appendLocalEnv } from 'npm-run-path'; import { join } from 'path'; +import { isLocallyLinkedPackageVersion } from '../../utils/is-locally-linked-package-version'; import { parseRegistryOptions } from '../../utils/npm-config'; +import { extractNpmPublishJsonData } from './extract-npm-publish-json-data'; import { logTar } from './log-tar'; import { PublishExecutorSchema } from './schema'; import chalk = require('chalk'); -import { extractNpmPublishJsonData } from './extract-npm-publish-json-data'; const LARGE_BUFFER = 1024 * 1000000; @@ -26,6 +31,7 @@ export default async function runExecutor( options: PublishExecutorSchema, context: ExecutorContext ) { + const pm = detectPackageManager(); /** * We need to check both the env var and the option because the executor may have been triggered * indirectly via dependsOn, in which case the env var will be set, but the option will not. @@ -44,6 +50,31 @@ export default async function runExecutor( const packageJson = readJsonFile(packageJsonPath); const packageName = packageJson.name; + /** + * pnpm supports dynamically updating locally linked packages during its packing phase, but other package managers do not. + * Therefore, protect the user from publishing invalid packages by checking if it contains local dependency protocols. + */ + if (pm !== 'pnpm') { + const depTypes = ['dependencies', 'devDependencies', 'peerDependencies']; + for (const depType of depTypes) { + const deps = packageJson[depType]; + if (deps) { + for (const depName in deps) { + if (isLocallyLinkedPackageVersion(deps[depName])) { + console.error( + `Error: Cannot publish package "${packageName}" because it contains a local dependency protocol in its "${depType}", and your package manager is ${pm}. + +Please update the local dependency on "${depName}" to be a valid semantic version (e.g. using \`nx release\`) before publishing, or switch to pnpm as a package manager, which supports dynamically replacing these protocols during publishing.` + ); + return { + success: false, + }; + } + } + } + } + } + // If package and project name match, we can make log messages terser let packageTxt = packageName === context.projectName @@ -88,7 +119,7 @@ export default async function runExecutor( * request with. * * Therefore, so as to not produce misleading output in dry around dist-tags being altered, we do not - * perform the npm view step, and just show npm publish's dry-run output. + * perform the npm view step, and just show npm/pnpm publish's dry-run output. */ if (!isDryRun && !options.firstRelease) { const currentVersion = packageJson.version; @@ -208,27 +239,30 @@ export default async function runExecutor( /** * NOTE: If this is ever changed away from running the command at the workspace root and pointing at the package root (e.g. back - * to running from the package root directly), then special attention should be paid to the fact that npm publish will nest its + * to running from the package root directly), then special attention should be paid to the fact that npm/pnpm publish will nest its * JSON output under the name of the package in that case (and it would need to be handled below). */ - const npmPublishCommandSegments = [ - `npm publish "${packageRoot}" --json --"${registryConfigKey}=${registry}" --tag=${tag}`, + const publishCommandSegments = [ + pm === 'pnpm' + ? // Unlike npm, pnpm publish does not support a custom registryConfigKey option, and will error on uncommitted changes by default if --no-git-checks is not set + `pnpm publish "${packageRoot}" --json --registry="${registry}" --tag=${tag} --no-git-checks` + : `npm publish "${packageRoot}" --json --"${registryConfigKey}=${registry}" --tag=${tag}`, ]; if (options.otp) { - npmPublishCommandSegments.push(`--otp=${options.otp}`); + publishCommandSegments.push(`--otp=${options.otp}`); } if (options.access) { - npmPublishCommandSegments.push(`--access=${options.access}`); + publishCommandSegments.push(`--access=${options.access}`); } if (isDryRun) { - npmPublishCommandSegments.push(`--dry-run`); + publishCommandSegments.push(`--dry-run`); } try { - const output = execSync(npmPublishCommandSegments.join(' '), { + const output = execSync(publishCommandSegments.join(' '), { maxBuffer: LARGE_BUFFER, env: processEnv(true), cwd: context.root, @@ -236,14 +270,14 @@ export default async function runExecutor( }); /** - * We cannot JSON.parse the output directly because if the user is using lifecycle scripts, npm will mix its publish output with the JSON output all on stdout. + * We cannot JSON.parse the output directly because if the user is using lifecycle scripts, npm/pnpm will mix its publish output with the JSON output all on stdout. * Additionally, we want to capture and show the lifecycle script outputs as beforeJsonData and afterJsonData and print them accordingly below. */ const { beforeJsonData, jsonData, afterJsonData } = extractNpmPublishJsonData(output.toString()); if (!jsonData) { console.error( - 'The npm publish output data could not be extracted. Please report this issue on https://github.com/nrwl/nx' + `The ${pm} publish output data could not be extracted. Please report this issue on https://github.com/nrwl/nx` ); return { success: false, @@ -294,7 +328,7 @@ export default async function runExecutor( try { const stdoutData = JSON.parse(err.stdout?.toString() || '{}'); - console.error('npm publish error:'); + console.error(`${pm} publish error:`); if (stdoutData.error?.summary) { console.error(stdoutData.error.summary); } @@ -303,7 +337,7 @@ export default async function runExecutor( } if (context.isVerbose) { - console.error('npm publish stdout:'); + console.error(`${pm} publish stdout:`); console.error(JSON.stringify(stdoutData, null, 2)); } @@ -316,7 +350,7 @@ export default async function runExecutor( }; } catch (err) { console.error( - 'Something unexpected went wrong when processing the npm publish output\n', + `Something unexpected went wrong when processing the ${pm} publish output\n`, err ); return { diff --git a/packages/js/src/generators/release-version/release-version.spec.ts b/packages/js/src/generators/release-version/release-version.spec.ts index bd0633c839b8e..4436066e334e6 100644 --- a/packages/js/src/generators/release-version/release-version.spec.ts +++ b/packages/js/src/generators/release-version/release-version.spec.ts @@ -10,6 +10,15 @@ const processExitSpy = jest return originalExit(...args); }); +const mockDetectPackageManager = jest.fn(); +jest.mock('@nx/devkit', () => { + const devkit = jest.requireActual('@nx/devkit'); + return { + ...devkit, + detectPackageManager: mockDetectPackageManager, + }; +}); + import { ProjectGraph, Tree, output, readJson } from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; import * as enquirer from 'enquirer'; @@ -1749,6 +1758,209 @@ Valid values are: "auto", "", "~", "^", "="`, }); }); }); + + describe('preserveLocalDependencyProtocols', () => { + it('should preserve local `workspace:` references when preserveLocalDependencyProtocols is true', async () => { + // Supported package manager for workspace: protocol + mockDetectPackageManager.mockReturnValue('pnpm'); + + projectGraph = createWorkspaceWithPackageDependencies(tree, { + 'package-a': { + projectRoot: 'packages/package-a', + packageName: 'package-a', + version: '1.0.0', + packageJsonPath: 'packages/package-a/package.json', + localDependencies: [ + { + projectName: 'package-b', + dependencyCollection: 'dependencies', + version: 'workspace:*', + }, + ], + }, + 'package-b': { + projectRoot: 'packages/package-b', + packageName: 'package-b', + version: '1.0.0', + packageJsonPath: 'packages/package-b/package.json', + localDependencies: [], + }, + }); + + expect(readJson(tree, 'packages/package-a/package.json')) + .toMatchInlineSnapshot(` + { + "dependencies": { + "package-b": "workspace:*", + }, + "name": "package-a", + "version": "1.0.0", + } + `); + expect(readJson(tree, 'packages/package-b/package.json')) + .toMatchInlineSnapshot(` + { + "name": "package-b", + "version": "1.0.0", + } + `); + + expect( + await releaseVersionGenerator(tree, { + projects: [projectGraph.nodes['package-b']], // version only package-b + projectGraph, + specifier: '2.0.0', + currentVersionResolver: 'disk', + specifierSource: 'prompt', + releaseGroup: createReleaseGroup('independent'), + updateDependents: 'auto', + preserveLocalDependencyProtocols: true, + }) + ).toMatchInlineSnapshot(` + { + "callback": [Function], + "data": { + "package-a": { + "currentVersion": "1.0.0", + "dependentProjects": [], + "newVersion": "1.0.1", + }, + "package-b": { + "currentVersion": "1.0.0", + "dependentProjects": [ + { + "dependencyCollection": "dependencies", + "rawVersionSpec": "workspace:*", + "source": "package-a", + "target": "package-b", + "type": "static", + }, + ], + "newVersion": "2.0.0", + }, + }, + } + `); + + expect(readJson(tree, 'packages/package-a/package.json')) + .toMatchInlineSnapshot(` + { + "dependencies": { + "package-b": "workspace:*", + }, + "name": "package-a", + "version": "1.0.1", + } + `); + + expect(readJson(tree, 'packages/package-b/package.json')) + .toMatchInlineSnapshot(` + { + "name": "package-b", + "version": "2.0.0", + } + `); + }); + + it('should preserve local `file:` references when preserveLocalDependencyProtocols is true', async () => { + projectGraph = createWorkspaceWithPackageDependencies(tree, { + 'package-a': { + projectRoot: 'packages/package-a', + packageName: 'package-a', + version: '1.0.0', + packageJsonPath: 'packages/package-a/package.json', + localDependencies: [ + { + projectName: 'package-b', + dependencyCollection: 'dependencies', + version: 'file:../package-b', + }, + ], + }, + 'package-b': { + projectRoot: 'packages/package-b', + packageName: 'package-b', + version: '1.0.0', + packageJsonPath: 'packages/package-b/package.json', + localDependencies: [], + }, + }); + + expect(readJson(tree, 'packages/package-a/package.json')) + .toMatchInlineSnapshot(` + { + "dependencies": { + "package-b": "file:../package-b", + }, + "name": "package-a", + "version": "1.0.0", + } + `); + expect(readJson(tree, 'packages/package-b/package.json')) + .toMatchInlineSnapshot(` + { + "name": "package-b", + "version": "1.0.0", + } + `); + + expect( + await releaseVersionGenerator(tree, { + projects: [projectGraph.nodes['package-b']], // version only package-b + projectGraph, + specifier: '2.0.0', + currentVersionResolver: 'disk', + specifierSource: 'prompt', + releaseGroup: createReleaseGroup('independent'), + updateDependents: 'auto', + preserveLocalDependencyProtocols: true, + }) + ).toMatchInlineSnapshot(` + { + "callback": [Function], + "data": { + "package-a": { + "currentVersion": "1.0.0", + "dependentProjects": [], + "newVersion": "1.0.1", + }, + "package-b": { + "currentVersion": "1.0.0", + "dependentProjects": [ + { + "dependencyCollection": "dependencies", + "rawVersionSpec": "file:../package-b", + "source": "package-a", + "target": "package-b", + "type": "static", + }, + ], + "newVersion": "2.0.0", + }, + }, + } + `); + + expect(readJson(tree, 'packages/package-a/package.json')) + .toMatchInlineSnapshot(` + { + "dependencies": { + "package-b": "file:../package-b", + }, + "name": "package-a", + "version": "1.0.1", + } + `); + + expect(readJson(tree, 'packages/package-b/package.json')) + .toMatchInlineSnapshot(` + { + "name": "package-b", + "version": "2.0.0", + } + `); + }); + }); }); function createReleaseGroup( diff --git a/packages/js/src/generators/release-version/release-version.ts b/packages/js/src/generators/release-version/release-version.ts index ff3c41b568711..583fdf253877e 100644 --- a/packages/js/src/generators/release-version/release-version.ts +++ b/packages/js/src/generators/release-version/release-version.ts @@ -1,10 +1,7 @@ import { - PackageManager, ProjectGraphProjectNode, Tree, - detectPackageManager, formatFiles, - getPackageManagerVersion, joinPathFragments, output, readJson, @@ -38,7 +35,8 @@ import { } from 'nx/src/command-line/release/version'; import { interpolate } from 'nx/src/tasks-runner/utils'; import * as ora from 'ora'; -import { ReleaseType, gt, inc, lt, prerelease } from 'semver'; +import { ReleaseType, gt, inc, prerelease } from 'semver'; +import { isLocallyLinkedPackageVersion } from '../../utils/is-locally-linked-package-version'; import { parseRegistryOptions } from '../../utils/npm-config'; import { ReleaseVersionGeneratorSchema } from './schema'; import { @@ -764,10 +762,16 @@ To fix this you will either need to add a package.json file at that location, or } } - // Apply the new version of the dependency to the dependent - const newDepVersion = `${versionPrefix}${newDependencyVersion}`; - json[dependentProject.dependencyCollection][dependencyPackageName] = - newDepVersion; + // Apply the new version of the dependency to the dependent (if not preserving locally linked package protocols) + const shouldUpdateDependency = !( + isLocallyLinkedPackageVersion(currentDependencyVersion) && + options.preserveLocalDependencyProtocols + ); + if (shouldUpdateDependency) { + const newDepVersion = `${versionPrefix}${newDependencyVersion}`; + json[dependentProject.dependencyCollection][dependencyPackageName] = + newDepVersion; + } // Bump the dependent's version if applicable and record it in the version data if (forceVersionBump) { @@ -989,42 +993,3 @@ class ProjectLogger { }); } } - -let pm: PackageManager | undefined; -let pmVersion: string | undefined; - -const localPackageProtocols = [ - 'file:', // all package managers - 'workspace:', // not npm - 'portal:', // modern yarn only -]; - -function isLocallyLinkedPackageVersion(version: string): boolean { - // Not using a supported local protocol - if (!localPackageProtocols.some((protocol) => version.startsWith(protocol))) { - return false; - } - // Supported by all package managers - if (version.startsWith('file:')) { - return true; - } - // Determine specific package manager in use - if (!pm) { - pm = detectPackageManager(); - pmVersion = getPackageManagerVersion(pm); - } - if (pm === 'npm' && version.startsWith('workspace:')) { - throw new Error( - `The "workspace:" protocol is not yet supported by npm (https://github.com/npm/rfcs/issues/765). Please ensure you have a valid setup according to your package manager before attempting to release packages.` - ); - } - if ( - version.startsWith('portal:') && - (pm !== 'yarn' || lt(pmVersion, '2.0.0')) - ) { - throw new Error( - `The "portal:" protocol is only supported by yarn@2.0.0 and above. Please ensure you have a valid setup according to your package manager before attempting to release packages.` - ); - } - return true; -} diff --git a/packages/js/src/utils/is-locally-linked-package-version.ts b/packages/js/src/utils/is-locally-linked-package-version.ts new file mode 100644 index 0000000000000..1c12787f18e00 --- /dev/null +++ b/packages/js/src/utils/is-locally-linked-package-version.ts @@ -0,0 +1,47 @@ +import { + detectPackageManager, + getPackageManagerVersion, + PackageManager, +} from '@nx/devkit'; +// import { lt } from 'semver'; + +let pm: PackageManager | undefined; +// let pmVersion: string | undefined; + +const localPackageProtocols = [ + 'file:', // all package managers + 'workspace:', // not npm + // TODO: Support portal protocol at the project graph level before enabling here + // 'portal:', // modern yarn only +]; + +export function isLocallyLinkedPackageVersion(version: string): boolean { + // Not using a supported local protocol + if (!localPackageProtocols.some((protocol) => version.startsWith(protocol))) { + return false; + } + // Supported by all package managers + if (version.startsWith('file:')) { + return true; + } + // Determine specific package manager in use + if (!pm) { + pm = detectPackageManager(); + // pmVersion = getPackageManagerVersion(pm); + } + if (pm === 'npm' && version.startsWith('workspace:')) { + throw new Error( + `The "workspace:" protocol is not yet supported by npm (https://github.com/npm/rfcs/issues/765). Please ensure you have a valid setup according to your package manager before attempting to release packages.` + ); + } + // TODO: Support portal protocol at the project graph level before enabling here + // if ( + // version.startsWith('portal:') && + // (pm !== 'yarn' || lt(pmVersion, '2.0.0')) + // ) { + // throw new Error( + // `The "portal:" protocol is only supported by yarn@2.0.0 and above. Please ensure you have a valid setup according to your package manager before attempting to release packages.` + // ); + // } + return true; +} diff --git a/packages/nx/src/command-line/release/version.ts b/packages/nx/src/command-line/release/version.ts index 07742a5dfcc57..70b5984636632 100644 --- a/packages/nx/src/command-line/release/version.ts +++ b/packages/nx/src/command-line/release/version.ts @@ -94,6 +94,12 @@ export interface ReleaseVersionGeneratorSchema { * large monorepos which have a large number of projects, especially when only a subset are released together. */ logUnchangedProjects?: boolean; + /** + * Whether or not to keep local dependency protocols (e.g. file:, workspace:) when updating dependencies in + * package.json files. This is `false` by default as not all package managers support publishing with these protocols + * still present in the package.json. + */ + preserveLocalDependencyProtocols?: boolean; } export interface NxReleaseVersionResult {