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(release): parse NPM output for JSON to prevent JSON.parse from throwing #23112

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gutentag2012
Copy link

closed #22925

Current Behavior

If the npm publish command is run and has any other console output than the JSON, e.g. output from a prepublishOnly step, the pnpm release command fails.

Expected Behavior

It should be possible to have a prepublishOnly step without this command failing.

Related Issue(s)

Fixes #22925

@gutentag2012 gutentag2012 requested a review from a team as a code owner May 1, 2024 11:03
@gutentag2012 gutentag2012 requested a review from Coly010 May 1, 2024 11:03
Copy link

vercel bot commented May 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview May 2, 2024 6:03pm

@JamesHenry
Copy link
Collaborator

JamesHenry commented May 1, 2024

You are applying this change (which I am a bit skeptical about, see the next paragraph) to the npm view command which is surely not the intention?

I imagine the npm CLI output is producing only JSON when --json is passed but on a specific output channel (e.g. probably the JSON is on stderr and the other output is on stdout (or vice versa)). Please can you verify this? Then we wouldn't need potentially brittle regex parsing of the full output

Copy link
Collaborator

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

See comment

@gutentag2012
Copy link
Author

Yes, you are completely right, I meant to apply this regex to the stdout of the npm publish --json command in line 209.
We would still need pass this output through the regex though since the npm publish output contains output from the prepublishOnly steps, so we do have to clean the output somehow.
If you have a different suggestion than using a regex I am open for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nx release fails when publishing packages that have a prepublishOnly step that produces console output
2 participants