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

[BUG] npm-pack --json emits prepare script commands, making json invalid #7354

Closed
2 tasks done
Tracked by #810
miiihi opened this issue Apr 8, 2024 · 8 comments · Fixed by #7439
Closed
2 tasks done
Tracked by #810

[BUG] npm-pack --json emits prepare script commands, making json invalid #7354

miiihi opened this issue Apr 8, 2024 · 8 comments · Fixed by #7439
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 10.x

Comments

@miiihi
Copy link

miiihi commented Apr 8, 2024

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

npm pack --json in the latest npm version (10.5.0) emits prepare script commands and thus making the output invalid json.
This is a breaking change from previous npm version, which broke our deploy workflows.

image

Expected Behavior

npm pack --json should always output valid json, as it was in the previous versions.

image

Steps To Reproduce

Create a package which includes a prepare script.
Run npm pack --json

Environment

  • npm:
  • Node.js:
  • OS Name:
  • System Model Name:
  • npm config:
; copy and paste output from `npm config ls` here
@miiihi miiihi added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Apr 8, 2024
@ljharb
Copy link
Contributor

ljharb commented Apr 8, 2024

It should be outputting on stderr in --json mode, so this may indeed be a bug. I'll take a look at this.

@miiihi
Copy link
Author

miiihi commented Apr 8, 2024

It's outputting on stdout...

@ljharb
Copy link
Contributor

ljharb commented Apr 12, 2024

Seems like this is an overall bug of --foreground-scripts when --json is passed - #7158 didn't cause it, but it exposed it.

@miiihi you can do npm --silent pack --json, i believe, as a workaround in the meantime?

@lukekarrys lukekarrys added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Apr 12, 2024
@lukekarrys lukekarrys self-assigned this Apr 12, 2024
dherman added a commit to neon-bindings/neon-rs that referenced this issue Apr 15, 2024
dherman added a commit to neon-bindings/neon that referenced this issue Apr 15, 2024
- no need for `fetch-depth: 0` when checking out git
- fix GH_TOKEN for `gh` cli to use the built-in ${{ github.token }}
- add `--silent` to `npm pack` to work around npm/cli#7354
@transitive-bullshit
Copy link

@ljharb I'm seeing the same issue with npm pack --dry-run --json --silent

See my temporary workaround here sindresorhus/np#743

@voxpelli
Copy link
Contributor

--silent removes the:

> test@4.2.0 prepare
> husky

But when eg. husky outputs to stdout itself (as husky v8 does), then that will still get emitted to stdout when --silent is used (which makes sense, as that's how --silent is supposed to work)

Solution for husky in particular is to update to husky v9, that together with --silent on npm pack no longer emits anything when used in prepare

@transitive-bullshit
Copy link

Makes sense.

In the project I'm testing with, https://github.com/dexaai/dexter, it has a prepare script with pnpm run build which runs tsc.

So it looks like this is possibly just a change in the way npm pack --silent --json handles prepare script stdout? Afaict this used to work as expected for parsing npm pack json output, and something seems to have changed recently, but I'm not sure exactly what.

@lukekarrys
Copy link
Contributor

You should be able to set --foreground-scripts=false which will suppress all output from the prepare script.

That's just a workaround though and this should get fixed in the next release. The fix will be to put all that output on stderr when in --json mode.

@lukekarrys
Copy link
Contributor

I've made a PR to move npm's banners from stdout to stderr when --json is set. I left a comment in the PR but I wanted to highlight here as well since there is still the possibility that a run-script will output non-JSON.

From #7439 (comment):

This highlights an important note about this change, as it only redirects the banner from @npmcli/run-script to stderr when --json is set. The user's run-script is spawned with stdio: inherit so if that process sends output to stdout it could still break in json mode. This is not something npm should change since we want to allow spawned process to write to whatever stream they want.

If a user really wants to ensure that the script cannot output anything, they should also use --foreground-scripts=false or --silent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 10.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants