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

fix(config): remove node-version and npm-version #5691

Merged
merged 1 commit into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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