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

Support *all* conf keys in publishConfig #2066

Closed
wants to merge 1 commit into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Oct 28, 2020

This adds a flatOptions.flatten() method, which takes an object full of
config keys, and turns it into an options object. This method expects
an object that already inherits from npm's defaults, and is thus
expected to be internal only.

This commit also removes some config keys which were used by npm
dependencies at the start of the v7 beta process, but are no longer:

  • all lockfile configs (since we don't use lockfiles any more! for
    anything! and good riddance, they're a rats' nest of race conditions)
  • cacheMax/cacheMin (we only use preferOffline/offline/online now, so
    these are strictly legacy support as input and never shared with deps)

Once this lands in cli, we can remove the publishConfig handling logic
in npm-registry-fetch, as it will be redundant.

References

This adds a flatOptions.flatten() method, which takes an object full of
config keys, and turns it into an options object.  This method expects
an object that already inherits from npm's defaults, and is thus
expected to be internal only.

This commit also removes some config keys which were used by npm
dependencies at the start of the v7 beta process, but are no longer:

- all lockfile configs (since we don't use lockfiles any more! for
  anything! and good riddance, they're a rats' nest of race conditions)
- cacheMax/cacheMin (we only use preferOffline/offline/online now, so
  these are strictly legacy support as input and never shared with deps)

Once this lands in cli, we can remove the publishConfig handling logic
in npm-registry-fetch, as it will be redundant.
@isaacs isaacs requested a review from nlf October 28, 2020 17:43
@isaacs isaacs requested a review from a team as a code owner October 28, 2020 17:43
Copy link
Contributor

@nlf nlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good!

@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 Oct 28, 2020
isaacs added a commit to npm/npm-registry-fetch that referenced this pull request Oct 28, 2020
Note: SemVer Major breaking change

This removes the 'publishConfig' option, as it will no longer be used
once npm/cli#2066 lands.
isaacs added a commit to npm/npm-registry-fetch that referenced this pull request Oct 28, 2020
Note: SemVer Major breaking change

This removes the 'publishConfig' option, as it will no longer be used
once npm/cli#2066 lands.
@darcyclarke darcyclarke added release: next These items should be addressed in the next release and removed release: next These items should be addressed in the next release labels Oct 30, 2020
nlf pushed a commit to npm/npm-registry-fetch that referenced this pull request Nov 3, 2020
Note: SemVer Major breaking change

This removes the 'publishConfig' option, as it will no longer be used
once npm/cli#2066 lands.

PR-URL: #36
Credit: @isaacs
Close: #36
Reviewed-by: @nlf
@darcyclarke darcyclarke closed this Nov 6, 2020
@isaacs
Copy link
Contributor Author

isaacs commented Nov 6, 2020

Landed in 7.0.8

@ruyadorno
Copy link
Contributor

landed in 6cd3cd0

@nlf nlf deleted the isaacs/publishConfig-support-all-keys branch March 28, 2022 16:58
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.

4 participants