fix(release): do not require the npm CLI to be installed for publishing JS packages with bun#32252
fix(release): do not require the npm CLI to be installed for publishing JS packages with bun#32252binsky08 wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
I've added a fixup with the requested changes |
|
View your CI Pipeline Execution ↗ for commit dc2caf3
☁️ Nx Cloud last updated this comment at |
|
Failed to publish a PR release of this pull request, triggered by @JamesHenry. |
|
Thanks @binsky08 I've triggered a PR release for this, it's a real npm release that we can test out in your environment and make sure it's fully working before merging. There will be an automated comment on here when it's ready to try, please let me know once you've verified that release in your bun only environment |
|
Ah looks like that is not possible due to your fork not being up to date with latest nx master (it's a precaution we have in the release script). I think we can go ahead and merge when it's green, it LGTM |
There was a problem hiding this comment.
@binsky08 You have some legit test failures on here.
Our AI fixer has correctly identified one of them (obviously you can use a more modern value for the npm version but it doesn't really matter):
Head branch was pushed to by a user without write access
|
@binsky08 Nx Cloud AI nailed the final failure too I think 😁 can you apply it via the link above? (This)
|
I'm not entirely sure about the error pattern, partly because I don't fully understand the specific test (flow). Bun says something like: which seems correct to me, if the package (with the packageName from the test) does not actually exist in the registry, because the command should only be executed if This condition already existed in the previous version of the file: https://github.com/nrwl/nx/blob/master/packages/js/src/executors/release-publish/release-publish.impl.ts#L144-L147 Therefore, I am wondering:
Unfortunately I can't reproduct the error (needed to validate my second point ^) since it (earlier) fails to run the e2e test in the proposed devcontainer.
Seems to be an issue with connecting to the automatically started local registry. I tried it several times and have already waited >10 minutes. Does not work. I'm not sure how to proceed here. |
…pm dist-tag command (nrwl#32126)
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the e2e test failures by properly handling Bun's package manager command syntax differences. We updated the view command construction to only add field specifiers for npm/pnpm (not Bun/Yarn), improved error handling to support Bun's plain-text error format, and filtered npm version warnings from test snapshots.
Warning
❌ We could not verify this fix.
Suggested Fix changes
diff --git a/e2e/release/src/independent-projects.workspaces.test.ts b/e2e/release/src/independent-projects.workspaces.test.ts
index 996f5aaee2..e2f825e73d 100644
--- a/e2e/release/src/independent-projects.workspaces.test.ts
+++ b/e2e/release/src/independent-projects.workspaces.test.ts
@@ -54,6 +54,8 @@ expect.addSnapshotSerializer({
.replaceAll(e2eRegistryUrl, '{registryUrl}')
// Filter out plugin worker verbose logs
.replaceAll(/\[(isolated-plugin|plugin-worker)\].*\n/g, '')
+ // Filter out npm warnings that may vary by npm version
+ .replaceAll(/npm warn .*\n/g, '')
// We trim each line to reduce the chances of snapshot flakiness
.split('\n')
.map((r) => r.trim())
diff --git a/e2e/release/src/preserve-local-dependency-protocols.test.ts b/e2e/release/src/preserve-local-dependency-protocols.test.ts
index 00066f8940..12ff27e784 100644
--- a/e2e/release/src/preserve-local-dependency-protocols.test.ts
+++ b/e2e/release/src/preserve-local-dependency-protocols.test.ts
@@ -49,6 +49,8 @@ expect.addSnapshotSerializer({
)
// Filter out plugin worker verbose logs
.replaceAll(/\[(isolated-plugin|plugin-worker)\].*\n/g, '')
+ // Filter out npm warnings that may vary by npm version
+ .replaceAll(/npm warn .*\n/g, '')
.split('\n')
.map((r) => r.trim())
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 687a71c26c..18bfd4efef 100644
--- a/packages/js/src/executors/release-publish/release-publish.impl.ts
+++ b/packages/js/src/executors/release-publish/release-publish.impl.ts
@@ -154,8 +154,9 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
packageName,
];
- // Since only yarn does not support multiple specific fields, we'll then fetch the complete data
- if (pm !== 'yarn') {
+ // Only npm and pnpm support specifying multiple specific fields like 'versions dist-tags'
+ // yarn and bun return complete package info without field specification
+ if (pm === 'npm' || pm === 'pnpm') {
npmViewCommandSegments.push('versions dist-tags');
}
npmViewCommandSegments.push(`--json --"${registryConfigKey}=${registry}"`);
@@ -276,25 +277,47 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
}
}
} catch (err) {
- const stdoutData = JSON.parse(err.stdout?.toString() || '{}');
- // If the error is that the package doesn't exist, then we can ignore it because we will be publishing it for the first time in the next step
- if (
- !(
- stdoutData.error?.code?.includes('E404') &&
- stdoutData.error?.summary?.toLowerCase().includes('not found')
- ) &&
- !(
- err.stderr?.toString().includes('E404') &&
- err.stderr?.toString().toLowerCase().includes('not found')
- )
- ) {
- console.error(
- `Something unexpected went wrong when checking for existing dist-tags.\n`,
- err
- );
- return {
- success: false,
- };
+ try {
+ const stdoutData = JSON.parse(err.stdout?.toString() || '{}');
+ // If the error is that the package doesn't exist, then we can ignore it because we will be publishing it for the first time in the next step
+ if (
+ !(
+ stdoutData.error?.code?.includes('E404') &&
+ stdoutData.error?.summary?.toLowerCase().includes('not found')
+ ) &&
+ !(
+ err.stderr?.toString().includes('E404') &&
+ err.stderr?.toString().toLowerCase().includes('not found')
+ )
+ ) {
+ console.error(
+ `Something unexpected went wrong when checking for existing dist-tags.\n`,
+ err
+ );
+ return {
+ success: false,
+ };
+ }
+ } catch (parseErr) {
+ // If we can't parse JSON (e.g., bun returns plain text errors), check stderr for 404
+ const stderrStr = err.stderr?.toString() || '';
+ const stdoutStr = err.stdout?.toString() || '';
+ if (
+ !(
+ (stderrStr.includes('404') &&
+ stderrStr.toLowerCase().includes('not found')) ||
+ (stdoutStr.includes('404') &&
+ stdoutStr.toLowerCase().includes('not found'))
+ )
+ ) {
+ console.error(
+ `Something unexpected went wrong when checking for existing dist-tags.\n`,
+ err
+ );
+ return {
+ success: false,
+ };
+ }
}
}
}
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
Because this branch comes from a fork, it is not possible for us to apply fixes directly, but you can apply the changes locally using the available options below.
Apply changes locally with:
npx nx-cloud apply-locally MpnO-OvaY
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |




Current Behavior
nx release publishrequires the npm package manager to run the hard-codednpm view ...command even in a project that doesn't use npm as package managerExpected Behavior
nx release publishuses the detected package manager of the project (e.g. bun) to run a package manager specific command likenpm view .../bun info ...Related Issue(s)
Fixes #32126
Notes
While
nx affected --target=e2e("No tasks were run") andnx affected --target=test("Successfully ran target test for 43 projects and 2 tasks they depend on") were successful, I was not able to publish a local test version to a local registry - so unfortunately I couldn't manually verify whether this actually fixes the linked issue.