-
Notifications
You must be signed in to change notification settings - Fork 4k
wip: create a flat options object to pass to deps #809
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
Closed
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8491b48
[email protected]
d7a6fce
[email protected]
7191983
test: removing test for no scoped deps, dropping constraint for v7 of…
5e5df8e
[email protected]
claudiahdz 4941626
[email protected]
claudiahdz e316bcc
[email protected]
claudiahdz c583d16
[email protected]
claudiahdz 1fea2de
[email protected]
claudiahdz 4a40897
[email protected]
1421928
feat: convert figgy config into POJO, when passed to pacote
2265302
wip: create a flat options object to pass to deps
isaacs c032a29
update several deps to non-figgy-pudding versions
isaacs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| // return a flattened config object with canonical names suitable for | ||
| // passing to dependencies like arborist, pacote, npm-registry-fetch, etc. | ||
|
|
||
| const log = require('npmlog') | ||
| const crypto = require('crypto') | ||
| const npmSession = crypto.randomBytes(8).toString('hex') | ||
| log.verbose('npm-session', npmSession) | ||
| const {join} = require('path') | ||
|
|
||
| const buildOmitList = npm => { | ||
| const include = npm.config.get('include') || [] | ||
| const omit = new Set((npm.config.get('omit') || []) | ||
| .filter(type => !include.includes(type))) | ||
| const only = npm.config.get('only') | ||
|
|
||
| if (/^prod(uction)?$/.test(only) || npm.config.get('production')) { | ||
| omit.add('dev') | ||
| } | ||
|
|
||
| if (/dev/.test(npm.config.get('also'))) { | ||
| omit.delete('dev') | ||
| } | ||
|
|
||
| if (npm.config.get('dev')) { | ||
| omit.delete('dev') | ||
| } | ||
|
|
||
| if (npm.config.get('optional') === false) { | ||
| omit.add('optional') | ||
| } | ||
|
|
||
| npm.config.set('omit', [...omit]) | ||
| return [...omit] | ||
| } | ||
|
|
||
| const flatOptions = npm => npm.flatOptions || Object.freeze({ | ||
| // Note that many of these do not come from configs or cli flags | ||
| // per se, though they may be implied or defined by them. | ||
| log, | ||
| npmSession, | ||
| dmode: npm.modes.exec, | ||
| fmode: npm.modes.file, | ||
| umask: npm.modes.umask, | ||
| hashAlgorithm: 'sha1', // XXX should this be sha512? | ||
| color: !!npm.color, | ||
| includeDeprecated: false, | ||
|
|
||
| projectScope: npm.projectScope, | ||
| npmVersion: npm.version, | ||
| nodeVersion: npm.config.get('node-version'), | ||
| npmCommand: npm.command, | ||
|
|
||
| tmp: npm.tmp, | ||
| cache: join(npm.config.get('cache'), '_cacache'), | ||
| prefix: npm.prefix, | ||
| globalPrefix: npm.globalPrefix, | ||
| localPrefix: npm.localPrefix, | ||
| global: npm.config.get('global'), | ||
|
|
||
| metricsRegistry: npm.config.get('metrics-registry'), | ||
| sendMetrics: npm.config.get('send-metrics'), | ||
| registry: npm.config.get('registry'), | ||
| get scope () { | ||
| log.warn('FIXME', 'using opts.scope instead of opts.projectScope') | ||
| return npm.projectScope | ||
| }, | ||
| access: npm.config.get('access'), | ||
| alwaysAuth: npm.config.get('always-auth'), | ||
| audit: npm.config.get('audit'), | ||
| auditLevel: npm.config.get('audit-level'), | ||
| authType: npm.config.get('auth-type'), | ||
| before: npm.config.get('before'), | ||
| browser: npm.config.get('browser'), | ||
| ca: npm.config.get('ca'), | ||
| cafile: npm.config.get('cafile'), | ||
| cert: npm.config.get('cert'), | ||
| key: npm.config.get('key'), | ||
|
|
||
| // XXX remove these when we don't use lockfile any more, once | ||
| // arborist is handling the installation process | ||
| cacheLockRetries: npm.config.get('cache-lock-retries'), | ||
| cacheLockStale: npm.config.get('cache-lock-stale'), | ||
| cacheLockWait: npm.config.get('cache-lock-wait'), | ||
| lockFile: { | ||
| retries: npm.config.get('cache-lock-retries'), | ||
| stale: npm.config.get('cache-lock-stale'), | ||
| wait: npm.config.get('cache-lock-wait') | ||
| }, | ||
|
|
||
| // XXX remove these once no longer used | ||
| get cacheMax () { | ||
| log.warn('FIXME', 'using deprecated cacheMax option, should use offline/preferOffline/preferOnline') | ||
| return npm.config.get('cache-max') | ||
| }, | ||
| get cacheMin () { | ||
| log.warn('FIXME', 'using deprecated cacheMin option, should use offline/preferOffline/preferOnline') | ||
| return npm.config.get('cache-min') | ||
| }, | ||
|
|
||
| // token creation options | ||
| cidr: npm.config.get('cidr'), | ||
| readOnly: npm.config.get('read-only'), | ||
|
|
||
| // npm version options | ||
| preid: npm.config.get('preid'), | ||
| tagVersionPrefix: npm.config.get('tag-version-prefix'), | ||
| allowSameVersion: npm.config.get('allow-same-version'), | ||
|
|
||
| // npm version git options | ||
| message: npm.config.get('message'), | ||
| commitHooks: npm.config.get('commit-hooks'), | ||
| gitTagVersion: npm.config.get('git-tag-version'), | ||
| signGitCommit: npm.config.get('sign-git-commit'), | ||
| signGitTag: npm.config.get('sign-git-tag'), | ||
|
|
||
| // only used for npm ls in v7, not update | ||
| depth: npm.config.get('depth'), | ||
|
|
||
| // options for npm search | ||
| // XXX need to refactor these to a cleaner search: { ... } block, | ||
| // since libnpmsearch needs them in a different format anyway, or | ||
| // maybe just not include them here at all, and construct an options | ||
| // pojo in lib/search.js instead. | ||
| description: npm.config.get('description'), | ||
| searchexclude: npm.config.get('searchexclude'), | ||
| searchlimit: npm.config.get('searchlimit'), | ||
| searchopts: npm.config.get('searchopts'), | ||
| searchstaleness: npm.config.get('searchstaleness'), | ||
|
|
||
| dryRun: npm.config.get('dry-run'), | ||
| engineStrict: npm.config.get('engine-strict'), | ||
|
|
||
| retry: { | ||
| retries: npm.config.get('fetch-retries'), | ||
| factor: npm.config.get('fetch-retry-factor'), | ||
| maxTimeout: npm.config.get('fetch-retry-maxtimeout'), | ||
| minTimeout: npm.config.get('fetch-retry-mintimeout') | ||
| }, | ||
|
|
||
| force: npm.config.get('force'), | ||
|
|
||
| formatPackageLock: npm.config.get('format-package-lock'), | ||
| fund: npm.config.get('fund'), | ||
|
|
||
| // binary locators | ||
| git: npm.config.get('git'), | ||
| npmBin: require.main.filename, | ||
| nodeBin: process.env.NODE || process.execPath, | ||
| viewer: npm.config.get('viewer'), | ||
| editor: npm.config.get('editor'), | ||
|
|
||
| // configs that affect how we build trees | ||
| binLinks: npm.config.get('bin-links'), | ||
| rebuildBundle: npm.config.get('rebuild-bundle'), | ||
| packageLock: npm.config.get('package-lock'), | ||
| packageLockOnly: npm.config.get('package-lock-only'), | ||
| globalStyle: npm.config.get('global-style'), | ||
| legacyBundling: npm.config.get('legacy-bundling'), | ||
| omit: buildOmitList(npm), | ||
|
|
||
| // used to build up the appropriate {add:{...}} options to Arborist.reify | ||
| save: npm.config.get('save'), | ||
| saveBundle: npm.config.get('save-bundle'), | ||
| saveDev: npm.config.get('save-dev'), | ||
| saveOptional: npm.config.get('save-optional'), | ||
| savePeer: npm.config.get('save-peer'), | ||
| saveProd: npm.config.get('save-prod'), | ||
| saveExact: npm.config.get('save-exact'), | ||
| savePrefix: npm.config.get('save-prefix'), | ||
|
|
||
| // configs for npm-registry-fetch | ||
| otp: npm.config.get('otp'), | ||
| offline: npm.config.get('offline'), | ||
| preferOffline: getPreferOffline(npm), | ||
| preferOnline: getPreferOnline(npm), | ||
| strictSSL: npm.config.get('strict-ssl'), | ||
| defaultTag: npm.config.get('tag'), | ||
| get tag () { | ||
| log.warn('FIXME', 'using tag option, should be defaultTag') | ||
| return npm.config.get('tag') | ||
| }, | ||
| userAgent: npm.config.get('user-agent'), | ||
|
|
||
| ...getScopesAndAuths(npm) | ||
| }) | ||
|
|
||
| const getPreferOnline = npm => { | ||
| const po = npm.config.get('prefer-online') | ||
| if (po !== undefined) { | ||
| return po | ||
| } | ||
| return npm.config.get('cache-max') <= 0 | ||
| } | ||
|
|
||
| const getPreferOffline = npm => { | ||
| const po = npm.config.get('prefer-offline') | ||
| if (po !== undefined) { | ||
| return po | ||
| } | ||
| return npm.config.get('cache-min') >= 9999 | ||
| } | ||
|
|
||
| // pull out all the @scope:<key> and //host:key config fields | ||
| // these are used by npm-registry-fetch for authing against registries | ||
| const getScopesAndAuths = npm => { | ||
| const scopesAndAuths = {} | ||
| // pull out all the @scope:... configs into a flat object. | ||
| for (const key in npm.config.list[0]) { | ||
| if (/@.*:registry$/i.test(key) || /^\/\//.test(key)) { | ||
| scopesAndAuths[key] = npm.config.get(key) | ||
| } | ||
| } | ||
| return scopesAndAuths | ||
| } | ||
|
|
||
| module.exports = flatOptions | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking as someone who maintains a CLI (lerna) that implements npm publishing using current
libnpm*packages (with an alarming amount of copypasta from npm's source) and attempts to stay as consistent as possible with npm CLI patterns, extracting "How npm reads and serializes its configuration" into a separate package would be a great boon.I realize it's extremely low on the priority list, but I just wanted to register the sentiment while it's top-of-mind, and before I inevitably feel the urge to make more copypasta...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would be really nice, I agree. (Also speaking as someone who maintains and uses a lot of npm deps ;)
We had once upon a time a package called
npmconfthat housed all of the config reading and management as well as all the default values. The problem was that every time we wanted to add or modify a config value, it meant an update in a bunch of disparate places, and it was kind of a pain to manage, so we folded it back in. This gets more convoluted when we have a bunch of other modules expecting to get apreferOnlineoption and need to know to convert that fromconfig.get('prefer-online')(or evenconfig.get('cache-min')<=0). The goal of figgy-pudding was to make this easier, but in practice, has made it much harder, which is why we're moving to the simplest possible approach, and from there can build up something a bit more maintainable. Just taking an inventory of all our configs as part of this upgrade process has been really enlightening. The clever technical solution didn't actually get at the root of the problem, and doing the work to dredge it all up and clean the actual mess has been sorely needed.Some day, I'd really like to consider dropping nopt in favor of something a bit more simple and declarative, like jackspeak or even yargs, and then have a package that doesn't handle defaults, but just reads and updates config files, so that the set of defaults and expected types can still live in npm/cli maybe. It's a weird sort of animal to cut in half, though, because I do want to be able to provide some feedback to the user that
--umask=notanumberis an invalid config value. So I'm not sure exactly what that'll look like. We may be in this "create a pojo and share it" mode for a while, most likely at least through the v7 release line. It just makes it so much easier to manage our deps if they can get an options object like any other JavaScript module out there does, without the extra translation layer of fp.