Skip to content

Commit 28019d5

Browse files
lukekarryslukekarrys
committed
fix(cleanup): move cli specific files to separate dir
1 parent 469f788 commit 28019d5

18 files changed

+70
-70
lines changed

‎.eslintrc.local.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ module.exports = {
2525
overrides: es6Files({
2626
'index.js': ['lib/cli.js'],
2727
'bin/npm-cli.js': ['lib/cli.js'],
28-
'lib/cli.js': ['lib/es6/validate-engines.js'],
29-
'lib/es6/validate-engines.js': ['package.json'],
28+
'lib/cli.js': ['lib/cli/validate-engines.js'],
29+
'lib/cli/validate-engines.js': ['package.json'],
3030
// TODO: This file should also have its requires restricted as well since it
3131
// is an entry point but it currently pulls in config definitions which have
3232
// a large require graph, so that is not currently feasible. A future config

‎lib/cli.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const validateEngines = require('./es6/validate-engines.js')
2-
const cliEntry = require('node:path').resolve(__dirname, 'cli-entry.js')
1+
const validateEngines = require('./cli/validate-engines.js')
2+
const cliEntry = require('node:path').resolve(__dirname, 'cli/entry.js')
33

44
module.exports = (process) => validateEngines(process, () => require(cliEntry))

‎lib/cli-entry.js ‎lib/cli/entry.js

+24-3
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ module.exports = async (process, validateEngines) => {
1111
process.argv.splice(1, 1, 'npm', '-g')
1212
}
1313

14+
// Patch the global fs module here at the app level
15+
require('graceful-fs').gracefulify(require('fs'))
16+
1417
const satisfies = require('semver/functions/satisfies')
15-
const exitHandler = require('./utils/exit-handler.js')
16-
const Npm = require('./npm.js')
18+
const exitHandler = require('./exit-handler.js')
19+
const Npm = require('../npm.js')
1720
const npm = new Npm()
1821
exitHandler.setNpm(npm)
1922

@@ -58,11 +61,29 @@ module.exports = async (process, validateEngines) => {
5861
return exitHandler()
5962
}
6063

64+
// this is async but we dont await it, since its ok if it doesnt
65+
// finish before the command finishes running. it uses command and argv
66+
// so it must be initiated here, after the command name is set
67+
const updateNotifier = require('./update-notifier.js')
68+
// eslint-disable-next-line promise/catch-or-return
69+
updateNotifier(npm).then((msg) => (npm.updateNotification = msg))
70+
71+
// Options are prefixed by a hyphen-minus (-, \u2d).
72+
// Other dash-type chars look similar but are invalid.
73+
const nonDashArgs = npm.argv.filter(a => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(a))
74+
if (nonDashArgs.length) {
75+
log.error(
76+
'arg',
77+
'Argument starts with non-ascii dash, this is probably invalid:',
78+
nonDashArgs.join(', ')
79+
)
80+
}
81+
6182
await npm.exec(cmd)
6283
return exitHandler()
6384
} catch (err) {
6485
if (err.code === 'EUNKNOWNCOMMAND') {
65-
const didYouMean = require('./utils/did-you-mean.js')
86+
const didYouMean = require('../utils/did-you-mean.js')
6687
const suggestions = await didYouMean(npm.localPrefix, cmd)
6788
output.standard(`Unknown command: "${cmd}"${suggestions}\n`)
6889
output.standard('To see a list of supported npm commands, run:\n npm help')

‎lib/utils/exit-handler.js ‎lib/cli/exit-handler.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const { log, output, time } = require('proc-log')
2-
const errorMessage = require('./error-message.js')
2+
const errorMessage = require('../utils/error-message.js')
33
const { redactLog: replaceInfo } = require('@npmcli/redact')
44

55
let npm = null // set by the cli

‎lib/utils/update-notifier.js ‎lib/cli/update-notifier.js

File renamed without changes.

‎lib/es6/validate-engines.js ‎lib/cli/validate-engines.js

File renamed without changes.

‎lib/npm.js

+1-30
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,13 @@ const { resolve, dirname, join } = require('node:path')
22
const Config = require('@npmcli/config')
33
const which = require('which')
44
const fs = require('node:fs/promises')
5-
6-
// Patch the global fs module here at the app level
7-
require('graceful-fs').gracefulify(require('fs'))
8-
95
const { definitions, flatten, shorthands } = require('@npmcli/config/lib/definitions')
106
const usage = require('./utils/npm-usage.js')
117
const LogFile = require('./utils/log-file.js')
128
const Timers = require('./utils/timers.js')
139
const Display = require('./utils/display.js')
1410
const { log, time } = require('proc-log')
1511
const { redactLog: replaceInfo } = require('@npmcli/redact')
16-
const updateNotifier = require('./utils/update-notifier.js')
1712
const pkg = require('../package.json')
1813
const { deref } = require('./utils/cmd-list.js')
1914

@@ -42,7 +37,6 @@ class Npm {
4237
#title = 'npm'
4338
#argvClean = []
4439
#npmRoot = null
45-
#warnedNonDashArg = false
4640

4741
#chalk = null
4842
#logChalk = null
@@ -109,30 +103,7 @@ class Npm {
109103
// passed in directly.
110104
async exec (cmd, args = this.argv) {
111105
const command = this.setCmd(cmd)
112-
113-
const timeEnd = time.start(`command:${cmd}`)
114-
115-
// this is async but we dont await it, since its ok if it doesnt
116-
// finish before the command finishes running. it uses command and argv
117-
// so it must be initiated here, after the command name is set
118-
// eslint-disable-next-line promise/catch-or-return
119-
updateNotifier(this).then((msg) => (this.updateNotification = msg))
120-
121-
// Options are prefixed by a hyphen-minus (-, \u2d).
122-
// Other dash-type chars look similar but are invalid.
123-
if (!this.#warnedNonDashArg) {
124-
const nonDashArgs = args.filter(a => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(a))
125-
if (nonDashArgs.length) {
126-
this.#warnedNonDashArg = true
127-
log.error(
128-
'arg',
129-
'Argument starts with non-ascii dash, this is probably invalid:',
130-
nonDashArgs.join(', ')
131-
)
132-
}
133-
}
134-
135-
return command.cmdExec(args).finally(timeEnd)
106+
return time.start(`command:${cmd}`, () => command.cmdExec(args))
136107
}
137108

138109
async load () {

‎smoke-tests/test/npm-replace-global.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ t.test('fail when updating with lazy require', async t => {
190190
// exit-handler is the last thing called in the code
191191
// so an uncached lazy require within the exit handler will always throw
192192
await fs.writeFile(
193-
join(paths.globalNodeModules, 'npm/lib/utils/exit-handler.js'),
193+
join(paths.globalNodeModules, 'npm/lib/cli/exit-handler.js'),
194194
`module.exports = () => require('./LAZY_REQUIRE_CANARY');module.exports.setNpm = () => {}`,
195195
'utf-8'
196196
)

‎tap-snapshots/test/lib/utils/update-notifier.js.test.cjs ‎tap-snapshots/test/lib/cli/update-notifier.js.test.cjs

+12-12
Original file line numberDiff line numberDiff line change
@@ -5,95 +5,95 @@
55
* Make sure to inspect the output below. Do not ignore changes!
66
*/
77
'use strict'
8-
exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=always > must match snapshot 1`] = `
8+
exports[`test/lib/cli/update-notifier.js TAP notification situations 122.420.69 - color=always > must match snapshot 1`] = `
99
1010
New major version of npm available! 122.420.69 -> 123.420.69
1111
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
1212
To update run: npm install -g npm@123.420.69
1313
1414
`
1515

16-
exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=false > must match snapshot 1`] = `
16+
exports[`test/lib/cli/update-notifier.js TAP notification situations 122.420.69 - color=false > must match snapshot 1`] = `
1717
1818
New major version of npm available! 122.420.69 -> 123.420.69
1919
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
2020
To update run: npm install -g npm@123.420.69
2121
2222
`
2323

24-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=always > must match snapshot 1`] = `
24+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.419.69 - color=always > must match snapshot 1`] = `
2525
2626
New minor version of npm available! 123.419.69 -> 123.420.69
2727
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
2828
To update run: npm install -g npm@123.420.69
2929
3030
`
3131

32-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=false > must match snapshot 1`] = `
32+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.419.69 - color=false > must match snapshot 1`] = `
3333
3434
New minor version of npm available! 123.419.69 -> 123.420.69
3535
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
3636
To update run: npm install -g npm@123.420.69
3737
3838
`
3939

40-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=always > must match snapshot 1`] = `
40+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.68 - color=always > must match snapshot 1`] = `
4141
4242
New patch version of npm available! 123.420.68 -> 123.420.69
4343
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
4444
To update run: npm install -g npm@123.420.69
4545
4646
`
4747

48-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=false > must match snapshot 1`] = `
48+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.68 - color=false > must match snapshot 1`] = `
4949
5050
New patch version of npm available! 123.420.68 -> 123.420.69
5151
Changelog: https://github.com/npm/cli/releases/tag/v123.420.69
5252
To update run: npm install -g npm@123.420.69
5353
5454
`
5555

56-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=always > must match snapshot 1`] = `
56+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.70 - color=always > must match snapshot 1`] = `
5757
5858
New minor version of npm available! 123.420.70 -> 123.421.70
5959
Changelog: https://github.com/npm/cli/releases/tag/v123.421.70
6060
To update run: npm install -g npm@123.421.70
6161
6262
`
6363

64-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=false > must match snapshot 1`] = `
64+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.70 - color=false > must match snapshot 1`] = `
6565
6666
New minor version of npm available! 123.420.70 -> 123.421.70
6767
Changelog: https://github.com/npm/cli/releases/tag/v123.421.70
6868
To update run: npm install -g npm@123.421.70
6969
7070
`
7171

72-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=always > must match snapshot 1`] = `
72+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.421.69 - color=always > must match snapshot 1`] = `
7373
7474
New patch version of npm available! 123.421.69 -> 123.421.70
7575
Changelog: https://github.com/npm/cli/releases/tag/v123.421.70
7676
To update run: npm install -g npm@123.421.70
7777
7878
`
7979

80-
exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=false > must match snapshot 1`] = `
80+
exports[`test/lib/cli/update-notifier.js TAP notification situations 123.421.69 - color=false > must match snapshot 1`] = `
8181
8282
New patch version of npm available! 123.421.69 -> 123.421.70
8383
Changelog: https://github.com/npm/cli/releases/tag/v123.421.70
8484
To update run: npm install -g npm@123.421.70
8585
8686
`
8787

88-
exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=always > must match snapshot 1`] = `
88+
exports[`test/lib/cli/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=always > must match snapshot 1`] = `
8989
9090
New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999
9191
Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999
9292
To update run: npm install -g npm@124.0.0-beta.99999
9393
9494
`
9595

96-
exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=false > must match snapshot 1`] = `
96+
exports[`test/lib/cli/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=false > must match snapshot 1`] = `
9797
9898
New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999
9999
Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999

‎test/fixtures/mock-npm.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,13 @@ const setGlobalNodeModules = (globalDir) => {
4848
}
4949

5050
const buildMocks = (t, mocks) => {
51-
const allMocks = {
52-
'{LIB}/utils/update-notifier.js': async () => {},
53-
...mocks,
54-
}
51+
const allMocks = { ...mocks }
5552
// The definitions must be mocked since they are a singleton that reads from
5653
// process and environs to build defaults in order to break the requiure
5754
// cache. We also need to mock them with any mocks that were passed in for the
5855
// test in case those mocks are for things like ci-info which is used there.
5956
const definitions = '@npmcli/config/lib/definitions'
6057
allMocks[definitions] = tmock(t, definitions, allMocks)
61-
6258
return allMocks
6359
}
6460

‎test/lib/cli.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const tmock = require('../fixtures/tmock')
33

44
t.test('returns cli-entry function', async t => {
55
const cli = tmock(t, '{LIB}/cli.js', {
6-
'{LIB}/cli-entry.js': () => 'ENTRY',
6+
'{LIB}/cli/entry.js': () => 'ENTRY',
77
})
88

99
t.equal(cli(process), 'ENTRY')

‎test/lib/cli-entry.js ‎test/lib/cli/entry.js

+18-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const t = require('tap')
2-
const { load: loadMockNpm } = require('../fixtures/mock-npm.js')
3-
const tmock = require('../fixtures/tmock.js')
4-
const validateEngines = require('../../lib/es6/validate-engines.js')
2+
const { load: loadMockNpm } = require('../../fixtures/mock-npm.js')
3+
const tmock = require('../../fixtures/tmock.js')
4+
const validateEngines = require('../../../lib/cli/validate-engines.js')
55

66
const cliMock = async (t, opts) => {
77
let exitHandlerArgs = null
@@ -13,9 +13,9 @@ const cliMock = async (t, opts) => {
1313
exitHandlerMock.setNpm = _npm => npm = _npm
1414

1515
const { Npm, ...mock } = await loadMockNpm(t, { ...opts, init: false })
16-
const cli = tmock(t, '{LIB}/cli-entry.js', {
16+
const cli = tmock(t, '{LIB}/cli/entry.js', {
1717
'{LIB}/npm.js': Npm,
18-
'{LIB}/utils/exit-handler.js': exitHandlerMock,
18+
'{LIB}/cli/exit-handler.js': exitHandlerMock,
1919
})
2020

2121
return {
@@ -159,3 +159,16 @@ t.test('unsupported node version', async t => {
159159
/npm v.* does not support Node\.js 12\.6\.0\./
160160
)
161161
})
162+
163+
t.test('non-ascii dash', async t => {
164+
const { cli, logs } = await cliMock(t, {
165+
globals: {
166+
'process.argv': ['node', 'npm', 'scope', '\u2010not-a-dash'],
167+
},
168+
})
169+
await cli(process)
170+
t.equal(
171+
logs.error[0],
172+
'arg Argument starts with non-ascii dash, this is probably invalid: \u2010not-a-dash'
173+
)
174+
})

‎test/lib/utils/update-notifier.js ‎test/lib/cli/update-notifier.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const runUpdateNotifier = async (t, {
8383
prefixDir,
8484
argv,
8585
})
86-
const updateNotifier = tmock(t, '{LIB}/utils/update-notifier.js', mocks)
86+
const updateNotifier = tmock(t, '{LIB}/cli/update-notifier.js', mocks)
8787

8888
const result = await updateNotifier(mock.npm)
8989

‎test/lib/es6/validate-engines.js ‎test/lib/cli/validate-engines.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const mockGlobals = require('@npmcli/mock-globals')
33
const tmock = require('../../fixtures/tmock')
44

55
const mockValidateEngines = (t) => {
6-
const validateEngines = tmock(t, '{LIB}/es6/validate-engines.js', {
6+
const validateEngines = tmock(t, '{LIB}/cli/validate-engines.js', {
77
'{ROOT}/package.json': { version: '1.2.3', engines: { node: '>=0' } },
88
})
99
mockGlobals(t, { 'process.version': 'v4.5.6' })

‎test/lib/npm.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -178,17 +178,16 @@ t.test('npm.load', async t => {
178178

179179
outputs.length = 0
180180
logs.length = 0
181-
await npm.exec('get', ['scope', '\u2010not-a-dash'])
181+
await npm.exec('get', ['scope', 'usage'])
182182

183183
t.strictSame([npm.command, npm.flatOptions.npmCommand], ['ll', 'll'],
184184
'does not change npm.command when another command is called')
185185

186186
t.match(logs, [
187-
'error arg Argument starts with non-ascii dash, this is probably invalid: \u2010not-a-dash',
188187
/timing command:config Completed in [0-9.]+ms/,
189188
/timing command:get Completed in [0-9.]+ms/,
190189
])
191-
t.same(outputs, ['scope=@foo\n\u2010not-a-dash=undefined'])
190+
t.same(outputs, ['scope=@foo\nusage=false'])
192191
})
193192

194193
await t.test('--no-workspaces with --workspace', async t => {

‎test/lib/utils/exit-handler.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => {
7272
},
7373
})
7474

75-
const exitHandler = tmock(t, '{LIB}/utils/exit-handler.js', {
75+
const exitHandler = tmock(t, '{LIB}/cli/exit-handler.js', {
7676
'{LIB}/utils/error-message.js': (err) => ({
7777
summary: [['ERR SUMMARY', err.message]],
7878
detail: [['ERR DETAIL', err.message]],

‎test/lib/utils/installed-deep.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const t = require('tap')
22
const installedDeep = require('../../../lib/utils/installed-deep.js')
3-
const mockNpm = require('../../../fixtures/mock-npm')
3+
const mockNpm = require('../../fixtures/mock-npm')
44

55
const fixture = {
66
'package.json': JSON.stringify({

‎test/lib/utils/installed-shallow.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const t = require('tap')
22
const installed = require('../../../lib/utils/installed-shallow.js')
3-
const mockNpm = require('../../../fixtures/mock-npm')
3+
const mockNpm = require('../../fixtures/mock-npm')
44

55
const mockShallow = async (t, config) => {
66
const res = await mockNpm(t, {

0 commit comments

Comments
 (0)