-
Notifications
You must be signed in to change notification settings - Fork 4k
fix(pack, publish): default foreground-scripts to true #7158
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
Conversation
4188dcb to
5f39e7c
Compare
|
Check out the |
|
Let's put a pin in the base command update, since we'd want to include the other things for which the default is different (like save). |
|
Smoke tests will likely need snapshots updated. |
5f39e7c to
81ec367
Compare
|
How do I update smoke test snapshots? |
cd1dcd8 to
ec31fce
Compare
npm run snap -w smoke-tests |
ec31fce to
05cad8e
Compare
|
I've updated the smoke test snapshots but they're still failing. |
05cad8e to
0cf39d1
Compare
|
Smoke tests were failing because they made an assumption that the output of I fixed this in #7256. Once that lands, this PR can be rebased and should go green. For reference here's what ❯ node . pack 2> /dev/null
> [email protected] prepack
> node . run build -w docs
> @npmcli/[email protected] build
> node bin/build.js
Wrote 251 files
npm-10.4.0.tgz |
0cf39d1 to
5995b95
Compare
|
This change is problematic imo and it actually broke our tooling, because the stdout even with the With (We were actually using the fact that it just used to print the tarball name to stdout, and I can see how that is maybe not the most robust idea since it's not a guaranteed API but with the Edit: I just realized that Update: should have looked up before posting, there's already a bug linked there that was closed and mentions using |
|
@simonhaenisch the hooks are under your control - you can silence their output however you like unrelated to npm. You can also set |
Fixes #6816
This fixes a regression in both npm 9 and 10.
An alternative approach to adding a constructor could be, add something in BaseCommand that runs super, and then reads the default from the derived command class, and then sets the default - that way it'd be more declarative. Happy to change to something like that, if preferred.