nx icon indicating copy to clipboard operation
nx copied to clipboard

fix(release): parse NPM output for JSON to prevent JSON.parse from throwing

Open gutentag2012 opened this issue 1 year ago • 4 comments

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 avatar May 01 '24 11:05 gutentag2012

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

vercel[bot] avatar May 01 '24 11:05 vercel[bot]

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

JamesHenry avatar May 01 '24 12:05 JamesHenry

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

gutentag2012 avatar May 01 '24 13:05 gutentag2012

Sorry for the delay @gutentag2012. I have been able to test out my suggestion above and sadly the npm publish output does not behave how I hoped.

I was hoping that npm was going to be consistent around valid JSON output on at least one channel (stdout or stderr) but it does indeed mix the outputs on stdout.

There's a few other things I wanted to cover in this area, so I have gone ahead and opened PR: https://github.com/nrwl/nx/pull/23850

This adds a (hopefully) more robust extraction method, actually prints the output of your lifecycle scripts (and unrelated to this modifies the version in the output in dry-mode, which is a long standing point of potential confusion that I wanted to address).

Thanks so much for pointing me towards this issue and opening this PR to get the ball rolling, I appreciate it!

JamesHenry avatar May 21 '24 13:05 JamesHenry

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.

github-actions[bot] avatar May 27 '24 00:05 github-actions[bot]