Skip to content

Commit

Permalink
fix(config): remove node-version and npm-version
Browse files Browse the repository at this point in the history
BREAKING CHANGE: the `node-version` and `npm-version` configs have been
removed.

These are only used sparingly by arborist to determine if optional
dependencies should be installed, and during engines checks (which
are only warnings unless `engine-strict` is true.
  • Loading branch information
wraithgar authored and fritzy committed Oct 13, 2022
1 parent d01bb92 commit a35c784
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 68 deletions.
7 changes: 5 additions & 2 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,12 @@ class Npm extends EventEmitter {

get flatOptions () {
const { flat } = this.config
// the Arborist constructor is used almost everywhere we call pacote, it's easiest
// to attach it to flatOptions so it goes everywhere without having to touch every call
// the Arborist constructor is used almost everywhere we call pacote, it's
// easiest to attach it to flatOptions so it goes everywhere without having
// to touch every call
flat.Arborist = Arborist
flat.nodeVersion = process.version
flat.npmVersion = pkg.version
if (this.command) {
flat.npmCommand = this.command
}
Expand Down
26 changes: 3 additions & 23 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1314,16 +1314,6 @@ define('node-options', {
`,
})

define('node-version', {
default: process.version,
defaultDescription: 'Node.js `process.version` value',
type: semver,
description: `
The node version to use when checking a package's \`engines\` setting.
`,
flatten,
})

define('noproxy', {
default: '',
defaultDescription: `
Expand All @@ -1344,16 +1334,6 @@ define('noproxy', {
},
})

define('npm-version', {
default: npmVersion,
defaultDescription: 'Output of `npm --version`',
type: semver,
description: `
The npm version to use when checking a package's \`engines\` setting.
`,
flatten,
})

define('offline', {
default: false,
type: Boolean,
Expand Down Expand Up @@ -2044,7 +2024,7 @@ define('tag-version-prefix', {
type: String,
description: `
If set, alters the prefix used when tagging a new version when performing
a version increment using \`npm-version\`. To remove the prefix
a version increment using \`npm version\`. To remove the prefix
altogether, set it to the empty string: \`""\`.
Because other tools may rely on the convention that npm version tags look
Expand Down Expand Up @@ -2166,8 +2146,8 @@ define('user-agent', {
inWorkspaces = true
}
flatOptions.userAgent =
value.replace(/\{node-version\}/gi, obj['node-version'])
.replace(/\{npm-version\}/gi, obj['npm-version'])
value.replace(/\{node-version\}/gi, process.version)
.replace(/\{npm-version\}/gi, npmVersion)
.replace(/\{platform\}/gi, process.platform)
.replace(/\{arch\}/gi, process.arch)
.replace(/\{workspaces\}/gi, inWorkspaces)
Expand Down
4 changes: 0 additions & 4 deletions tap-snapshots/test/lib/commands/config.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,9 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
"maxsockets": 15,
"message": "%s",
"node-options": null,
"node-version": "{NODE-VERSION}",
"noproxy": [
""
],
"npm-version": "{NPM-VERSION}",
"offline": false,
"omit": [],
"omit-lockfile-registry-resolved": false,
Expand Down Expand Up @@ -252,9 +250,7 @@ maxsockets = 15
message = "%s"
metrics-registry = "https://registry.npmjs.org/"
node-options = null
node-version = "{NODE-VERSION}"
noproxy = [""]
npm-version = "{NPM-VERSION}"
offline = false
omit = []
omit-lockfile-registry-resolved = false
Expand Down
20 changes: 1 addition & 19 deletions tap-snapshots/test/lib/docs.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,13 +1361,6 @@ Options to pass through to Node.js via the \`NODE_OPTIONS\` environment
variable. This does not impact how npm itself is executed but it does impact
how lifecycle scripts are called.
#### \`node-version\`
* Default: Node.js \`process.version\` value
* Type: SemVer string
The node version to use when checking a package's \`engines\` setting.
#### \`noproxy\`
* Default: The value of the NO_PROXY environment variable
Expand All @@ -1377,13 +1370,6 @@ Domain extensions that should bypass any proxies.
Also accepts a comma-delimited string.
#### \`npm-version\`
* Default: Output of \`npm --version\`
* Type: SemVer string
The npm version to use when checking a package's \`engines\` setting.
#### \`offline\`
* Default: false
Expand Down Expand Up @@ -1792,7 +1778,7 @@ tarball that will be compared with the local files by default.
* Type: String
If set, alters the prefix used when tagging a new version when performing a
version increment using \`npm-version\`. To remove the prefix altogether, set
version increment using \`npm version\`. To remove the prefix altogether, set
it to the empty string: \`""\`.
Because other tools may rely on the convention that npm version tags look
Expand Down Expand Up @@ -2190,9 +2176,7 @@ Array [
"maxsockets",
"message",
"node-options",
"node-version",
"noproxy",
"npm-version",
"offline",
"omit",
"omit-lockfile-registry-resolved",
Expand Down Expand Up @@ -2326,9 +2310,7 @@ Array [
"loglevel",
"maxsockets",
"message",
"node-version",
"noproxy",
"npm-version",
"offline",
"omit",
"omit-lockfile-registry-resolved",
Expand Down
5 changes: 3 additions & 2 deletions test/fixtures/sandbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const { promisify } = require('util')
const mkdirp = require('mkdirp-infer-owner')
const rimraf = promisify(require('rimraf'))
const mockLogs = require('./mock-logs')
const pkg = require('../../package.json')

const chain = new Map()
const sandboxes = new Map()
Expand Down Expand Up @@ -45,8 +46,6 @@ const _logs = Symbol('sandbox.logs')

// these config keys can be redacted widely
const redactedDefaults = [
'node-version',
'npm-version',
'tmp',
]

Expand Down Expand Up @@ -155,6 +154,8 @@ class Sandbox extends EventEmitter {
.split(normalize(homedir())).join('{REALHOME}')
.split(this[_proxy].platform).join('{PLATFORM}')
.split(this[_proxy].arch).join('{ARCH}')
.replace(new RegExp(process.version, 'g'), '{NODE-VERSION}')
.replace(new RegExp(pkg.version, 'g'), '{NPM-VERSION}')

// We do the defaults after everything else so that they don't cause the
// other cleaners to miss values we would have clobbered here. For
Expand Down
14 changes: 7 additions & 7 deletions test/lib/commands/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,21 +330,21 @@ t.test('config get no args', async t => {
t.test('config get single key', async t => {
const sandbox = new Sandbox(t)

await sandbox.run('config', ['get', 'node-version'])
t.equal(sandbox.output, sandbox.config.get('node-version'), 'should get the value')
await sandbox.run('config', ['get', 'all'])
t.equal(sandbox.output, `${sandbox.config.get('all')}`, 'should get the value')
})

t.test('config get multiple keys', async t => {
const sandbox = new Sandbox(t)

await sandbox.run('config', ['get', 'node-version', 'npm-version'])
await sandbox.run('config', ['get', 'yes', 'all'])
t.ok(
sandbox.output.includes(`node-version=${sandbox.config.get('node-version')}`),
'outputs node-version'
sandbox.output.includes(`yes=${sandbox.config.get('yes')}`),
'outputs yes'
)
t.ok(
sandbox.output.includes(`npm-version=${sandbox.config.get('npm-version')}`),
'outputs npm-version'
sandbox.output.includes(`all=${sandbox.config.get('all')}`),
'outputs all'
)
})

Expand Down
13 changes: 2 additions & 11 deletions test/lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const t = require('tap')
const { resolve } = require('path')
const mockGlobals = require('../../../fixtures/mock-globals')
const pkg = require('../../../../package.json')

// have to fake the node version, or else it'll only pass on this one
mockGlobals(t, { 'process.version': 'v14.8.0', 'process.env.NODE_ENV': undefined })
Expand All @@ -9,14 +10,6 @@ const mockDefs = (mocks = {}) => t.mock('../../../../lib/utils/config/definition

const isWin = (isWindows) => ({ '../../../../lib/utils/is-windows.js': { isWindows } })

t.test('node and npm versions', t => {
const definitions = mockDefs()
const pkg = require('../../../../package.json')
t.equal(definitions['npm-version'].default, pkg.version, 'npm-version default')
t.equal(definitions['node-version'].default, process.version, 'node-version default')
t.end()
})

t.test('basic flattening function camelCases from css-case', t => {
const flat = {}
const obj = { 'prefer-online': true }
Expand Down Expand Up @@ -745,11 +738,9 @@ t.test('detect CI', t => {
t.test('user-agent', t => {
const obj = {
'user-agent': mockDefs()['user-agent'].default,
'npm-version': '1.2.3',
'node-version': '9.8.7',
}
const flat = {}
const expectNoCI = `npm/1.2.3 node/9.8.7 ` +
const expectNoCI = `npm/${pkg.version} node/${process.version} ` +
`${process.platform} ${process.arch} workspaces/false`
mockDefs()['user-agent'].flatten('user-agent', obj, flat)
t.equal(flat.userAgent, expectNoCI)
Expand Down

0 comments on commit a35c784

Please sign in to comment.