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

Preserve builtin conf when installing npm globally #2184

Closed
wants to merge 4 commits into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Nov 17, 2020

When a file named 'npmrc' is in the root of the npm module that is
currently running, it adds config values that override the defaults (but
not the global or user configs).

This is a way for a system package installer to tell the npm that it
installs to put its globals somewhere other than the default. In order
to keep these configs around when users self-update npm with npm i -g npm, these config values must be "sticky", and ride along into the
newly globally installed npm.

This commit restores this behavior, fixing self-updating npm for Windows
users, and any other systems that may make use of this functionality.

Fixes: #2002

References

When a file named 'npmrc' is in the root of the npm module that is
currently running, it adds config values that override the defaults (but
not the global or user configs).

This is a way for a system package installer to tell the npm that it
installs to put its globals somewhere other than the default.  In order
to keep these configs around when users self-update npm with `npm i -g
npm`, these config values must be "sticky", and ride along into the
newly globally installed npm.

This commit restores this behavior, fixing self-updating npm for Windows
users, and any other systems that may make use of this functionality.

Fixes: #2002
@isaacs isaacs requested a review from a team as a code owner November 17, 2020 01:22
We have a lot of tests that were not handling errors in callbacks, not
defining variables, defining variables that weren't used, and just
generally not formatted like the rest of npm.

Hazard of moving fast.  Thankfully, machines can help.
@isaacs
Copy link
Contributor Author

isaacs commented Nov 17, 2020

Added two commits onto the one that fixed this bug. The first to catch the tests that it broke, and the second to make it easier to catch similar test breakage in the future.

@ljharb
Copy link
Contributor

ljharb commented Nov 17, 2020

To be clear, this restores the npm 6 behavior exactly wrt npmrc files?

@darcyclarke darcyclarke added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Nov 17, 2020
@isaacs
Copy link
Contributor Author

isaacs commented Nov 17, 2020

To be clear, this restores the npm 6 behavior exactly wrt npmrc files?

With respect to the "builtin" config that is bundled with npm itself by system installers, yes.

@isaacs isaacs force-pushed the isaacs/builtin-config-preservation branch from 61ea78c to e21f268 Compare November 17, 2020 19:26
@ruyadorno ruyadorno mentioned this pull request Nov 17, 2020
@ruyadorno ruyadorno closed this in bc9afb1 Nov 17, 2020
@nlf nlf deleted the isaacs/builtin-config-preservation branch March 28, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] After update to npm 7 it seems no global packages are recognized
4 participants