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.

Sign up for GitHub

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

Jump to bottom

nx release fails when publishing packages that have a prepublishOnly step that produces console output #22925

Closed
1 of 4 tasks
gutentag2012 opened this issue Apr 20, 2024 · 2 comments · Fixed by #23850 · May be fixed by gutentag2012/nx#1
Closed
1 of 4 tasks

nx release fails when publishing packages that have a prepublishOnly step that produces console output #22925

gutentag2012 opened this issue Apr 20, 2024 · 2 comments · Fixed by #23850 · May be fixed by gutentag2012/nx#1
Assignees
@JamesHenry
Labels
scope: release type: bug

Comments

@gutentag2012
Copy link

Current Behavior

When running the nx release command for packages that have a prepublishOnly script that e.g. builds the package, the command fails since it cannot parse the console output as a JSON

Expected Behavior

It should be possible to publish packages that have a prepublishOnly script attached

GitHub Repo

No response

Steps to Reproduce

Since this step involves actually publishing to npm, you need to create your own package on npm for this.

  1. Create a npm library that is publishable
  2. Add the "prepublishOnly": "echo 'This is only a test'"
  3. Run the nx release command
  4. Selecte yes when it asks to publish the package

Nx Report

Node   : 18.20.2
   OS     : win32-x64
   pnpm   : 8.15.6

   nx             : 18.0.2
   @nx/js         : 18.0.2
   @nx/workspace  : 18.0.2
   @nx/devkit     : 18.0.2
   @nrwl/tao      : 18.0.2
   typescript     : 5.4.3

Failure Logs

No response

Package Manager Version

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

I already have a local patch for this and would be willing to open a PR for this.

The issue is here, there the output of the publish command is parsed without stripping it of previous output.
@AgentEnder
Copy link
Member

I'm sure a PR would be appreciated 🤗, I've assigned JamesHenry to the issue as he would be the most likely eventual reviewer.

gutentag2012 pushed a commit to gutentag2012/nx that referenced this issue May 1, 2024
fix(release): extract JSON from publish output
aea0110 
Instead of parsing the raw output of the npm publish command, only use the last JSON structure
within the text

closed nrwl#22925
@gutentag2012 gutentag2012 mentioned this issue May 1, 2024
Open
gutentag2012 pushed a commit to gutentag2012/nx that referenced this issue May 1, 2024
fix(release): parse NPM output for JSON to prevent JSON.parse from th…
9669f61 
…rowing

closed nrwl#22925
@gutentag2012 gutentag2012 mentioned this issue May 1, 2024
Closed
@gutentag2012
Copy link
Author

I just opened the PR so a review would be very much appreciated :D
PR: #23112

gutentag2012 pushed a commit to gutentag2012/nx that referenced this issue May 2, 2024
@gutentag2012
fix(release): parse NPM output for JSON to prevent JSON.parse from th…
2e37549 
…rowing

closed nrwl#22925
gutentag2012 added a commit to gutentag2012/nx that referenced this issue May 2, 2024
@gutentag2012
fix(release): revert npm view json parsing back to raw string output
7087cc2 
closed nrwl#22925
@JamesHenry JamesHenry mentioned this issue May 21, 2024
Merged
FrozenPandaz pushed a commit that referenced this issue May 27, 2024
@JamesHenry
fix(js): show lifecycle script contents in publish executor, scrub ve…
aa2519f 
…rsion in dry-run (#23850)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

Lifecycle scripts related to publishing change the output of `npm
publish`, which mixes JSON with non-JSON content despite the `--json`
flag being set. Currently, we will error when attempting to parse the
whole thing as JSON.

The lifecycle scripts contents themselves are not shown to the user.

Additionally, during dry-run we have no choice but to print the version
that currently exists on disk, which can be confusing.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

We extract and parse the JSON from the `npm publish` output, even when
it is mixed with other output. We also show the lifecycle script outputs
to the user, where applicable.

During dry-run, we replace the version in the publish output with a
placeholder to avoid confusion around what would be published.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #22925
FrozenPandaz pushed a commit that referenced this issue May 28, 2024
@JamesHenry @FrozenPandaz
fix(js): show lifecycle script contents in publish executor, scrub ve…
bd7c681 
…rsion in dry-run (#23850)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

## Current Behavior
<!-- This is the behavior we have today -->

Lifecycle scripts related to publishing change the output of `npm
publish`, which mixes JSON with non-JSON content despite the `--json`
flag being set. Currently, we will error when attempting to parse the
whole thing as JSON.

The lifecycle scripts contents themselves are not shown to the user.

Additionally, during dry-run we have no choice but to print the version
that currently exists on disk, which can be confusing.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

We extract and parse the JSON from the `npm publish` output, even when
it is mixed with other output. We also show the lifecycle script outputs
to the user, where applicable.

During dry-run, we replace the version in the publish output with a
placeholder to avoid confusion around what would be published.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #22925

(cherry picked from commit aa2519f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JamesHenry JamesHenry

Labels
scope: release type: bug
Projects
None yet
Milestone
No milestone
3 participants
@JamesHenry @AgentEnder @gutentag2012