Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): do not attempt to publish private npm packages #19299

Merged
merged 6 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 194 additions & 0 deletions e2e/release/src/private-js-packages.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
import {
cleanupProject,
newProject,
runCLI,
tmpProjPath,
uniq,
updateJson,
} from '@nx/e2e/utils';
import { execSync } from 'child_process';

expect.addSnapshotSerializer({
serialize(str: string) {
return (
str
// Remove all output unique to specific projects to ensure deterministic snapshots
.replaceAll(`/private/${tmpProjPath()}`, '')
.replaceAll(tmpProjPath(), '')
.replaceAll('/private/', '')
.replaceAll(/public-pkg-\d+/g, '{public-project-name}')
.replaceAll(/private-pkg\d+/g, '{private-project-name}')
.replaceAll(
/integrity:\s*.*/g,
'integrity: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX'
)
.replaceAll(/\b[0-9a-f]{40}\b/g, '{SHASUM}')
.replaceAll(/\d*B index\.js/g, 'XXB index.js')
.replaceAll(/\d*B project\.json/g, 'XXB project.json')
.replaceAll(/\d*B package\.json/g, 'XXXB package.json')
.replaceAll(/size:\s*\d*\s?B/g, 'size: XXXB')
.replaceAll(/\d*\.\d*\s?kB/g, 'XXX.XXX kb')
// We trim each line to reduce the chances of snapshot flakiness
.split('\n')
.map((r) => r.trim())
.join('\n')
);
},
test(val: string) {
return val != null && typeof val === 'string';
},
});

describe('nx release - private JS packages', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to keep this in 1 file so that we don't have to create a new workspace for this test?

let publicPkg1: string;
let publicPkg2: string;
let privatePkg: string;

beforeAll(() => {
newProject({
unsetProjectNameAndRootFormat: false,
});

publicPkg1 = uniq('public-pkg-1');
runCLI(`generate @nx/workspace:npm-package ${publicPkg1}`);

publicPkg2 = uniq('public-pkg-2');
runCLI(`generate @nx/workspace:npm-package ${publicPkg2}`);

privatePkg = uniq('private-pkg');
runCLI(`generate @nx/workspace:npm-package ${privatePkg}`);
updateJson(`${privatePkg}/package.json`, (json) => {
json.private = true;
return json;
});

/**
* Update public-pkg2 to depend on private-pkg.
*
* At the time of writing this is not something we protect the user against,
* so we expect this to not cause any issues, and public-pkg2 will be published.
*
* TODO: these tests will need to be updated when we add support for protecting against this
*/
updateJson(`${publicPkg2}/package.json`, (json) => {
json.dependencies ??= {};
json.dependencies[`@proj/${privatePkg}`] = '0.0.0';
return json;
});
});
afterAll(() => cleanupProject());

it('should skip private packages and log a warning', async () => {
runCLI(`release version 999.9.9`);

// This is the verdaccio instance that the e2e tests themselves are working from
const e2eRegistryUrl = execSync('npm config get registry')
.toString()
.trim();

// Thanks to the custom serializer above, the publish output should be deterministic
const publicPkg1PublishOutput = runCLI(`release publish -p ${publicPkg1}`);
expect(publicPkg1PublishOutput).toMatchInlineSnapshot(`

> NX Your filter "{public-project-name}" matched the following projects:

- {public-project-name}


> NX Running target nx-release-publish for project {public-project-name}:

- {public-project-name}



> nx run {public-project-name}:nx-release-publish


📦 @proj/{public-project-name}@999.9.9
=== Tarball Contents ===

XXB index.js
XXXB package.json
XXB project.json
=== Tarball Details ===
name: @proj/{public-project-name}
version: 999.9.9
filename: proj-{public-project-name}-999.9.9.tgz
package size: XXXB
unpacked size: XXXB
shasum: {SHASUM}
integrity: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
total files: 3

Published to ${e2eRegistryUrl} with tag "latest"



> NX Successfully ran target nx-release-publish for project {public-project-name}



`);

// This will include the private package publish output as it is a dependency
const publicPkg2PublishOutput = runCLI(`release publish -p ${publicPkg2}`);
expect(publicPkg2PublishOutput).toMatchInlineSnapshot(`

> NX Your filter "{public-project-name}" matched the following projects:

- {public-project-name}


> NX Running target nx-release-publish for project {public-project-name} and 1 task it depends on:

- {public-project-name}



> nx run {private-project-name}:nx-release-publish

Skipping package "@proj/{private-project-name}" from project "{private-project-name}", because it has \`"private": true\` in /{private-project-name}/package.json

> nx run {public-project-name}:nx-release-publish


📦 @proj/{public-project-name}@999.9.9
=== Tarball Contents ===

XXB index.js
XXXB package.json
XXB project.json
=== Tarball Details ===
name: @proj/{public-project-name}
version: 999.9.9
filename: proj-{public-project-name}-999.9.9.tgz
package size: XXXB
unpacked size: XXXB
shasum: {SHASUM}
integrity: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
total files: 3

Published to ${e2eRegistryUrl} with tag "latest"



> NX Successfully ran target nx-release-publish for project {public-project-name} and 1 task it depends on



`);

// The two public packages should have been published
expect(
execSync(`npm view @proj/${publicPkg1} version`).toString().trim()
).toEqual('999.9.9');
expect(
execSync(`npm view @proj/${publicPkg2} version`).toString().trim()
).toEqual('999.9.9');

// The private package should have never been published
expect(() => execSync(`npm view @proj/${privatePkg} version`)).toThrowError(
/npm ERR! code E404/
);
}, 500000);
});
29 changes: 19 additions & 10 deletions packages/js/src/executors/release-publish/release-publish.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,25 @@ export default async function runExecutor(
options.packageRoot ?? projectConfig.root
);

const packageJsonPath = joinPathFragments(packageRoot, 'package.json');
const projectPackageJson = readJsonFile(packageJsonPath);
const name = projectPackageJson.name;

// If package and project name match, we can make log messages terser
let packageTxt =
name === context.projectName
? `package "${name}"`
: `package "${name}" from project "${context.projectName}"`;

if (projectPackageJson.private === true) {
console.warn(
`Skipping ${packageTxt}, because it has \`"private": true\` in ${packageJsonPath}`
);
return {
success: true,
};
}

const npmPublishCommandSegments = [`npm publish --json`];

if (options.registry) {
Expand Down Expand Up @@ -79,10 +98,6 @@ export default async function runExecutor(
};
} catch (err) {
try {
const projectPackageJson = readJsonFile(
joinPathFragments(packageRoot, 'package.json')
);
const name = projectPackageJson.name;
const currentVersion = projectPackageJson.version;

const stdoutData = JSON.parse(err.stdout?.toString() || '{}');
Expand All @@ -93,12 +108,6 @@ export default async function runExecutor(
'You cannot publish over the previously published versions'
))
) {
// If package and project name match, make it terser
let packageTxt =
name === context.projectName
? `package "${name}"`
: `package "${name}" from project "${context.projectName}"`;

console.warn(
`Skipping ${packageTxt}, as v${currentVersion} has already been published to ${registry} with tag "${tag}"`
);
Expand Down