Skip to content

Commit 4b86e08

Browse files
committed
chore(refactor): finish passing npm context
No more requiring npm as a singleton. This will now allow us to move forward with the other refactoring we need to always use the npm object itself in tests, not a mocked one.
1 parent a49b8d7 commit 4b86e08

File tree

12 files changed

+119
-287
lines changed

12 files changed

+119
-287
lines changed

lib/cli.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ module.exports = (process) => {
2020

2121
const npm = require('../lib/npm.js')
2222
const errorHandler = require('../lib/utils/error-handler.js')
23+
errorHandler.setNpm(npm)
2324

2425
// if npm is called as "npmg" or "npm_g", then
2526
// run in global mode.

lib/utils/cache-file.js

Lines changed: 0 additions & 66 deletions
This file was deleted.

lib/utils/error-handler.js

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,38 @@
1+
let npm // set by the cli
12
let cbCalled = false
23
const log = require('npmlog')
3-
const npm = require('../npm.js')
44
let itWorked = false
55
const path = require('path')
6+
const writeFileAtomic = require('write-file-atomic')
7+
const mkdirp = require('mkdirp')
8+
const fs = require('graceful-fs')
69
let wroteLogFile = false
710
let exitCode = 0
811
const errorMessage = require('./error-message.js')
912
const replaceInfo = require('./replace-info.js')
1013

11-
const cacheFile = require('./cache-file.js')
14+
const append = (file, data) => {
15+
const dir = path.dirname(file)
16+
mkdirp.sync(dir)
17+
18+
if (typeof process.getuid === 'function') {
19+
const st = fs.lstatSync(dir)
20+
fs.chownSync(dir, st.uid, st.gid)
21+
fs.chownSync(file, st.uid, st.gid)
22+
}
23+
}
24+
25+
const write = (file, data) => {
26+
const dir = path.dirname(file)
27+
mkdirp.sync(dir)
28+
29+
writeFileAtomic.sync(file, data)
30+
if (typeof process.getuid === 'function') {
31+
const st = fs.lstatSync(dir)
32+
fs.chownSync(dir, st.uid, st.gid)
33+
fs.chownSync(file, st.uid, st.gid)
34+
}
35+
}
1236

1337
let logFileName
1438
const getLogFile = () => {
@@ -19,7 +43,6 @@ const getLogFile = () => {
1943
}
2044

2145
const timings = {
22-
version: npm.version,
2346
command: process.argv.slice(2),
2447
logfile: null,
2548
}
@@ -36,7 +59,7 @@ process.on('exit', code => {
3659
if (npm.config && npm.config.loaded && npm.config.get('timing')) {
3760
try {
3861
timings.logfile = getLogFile()
39-
cacheFile.append('_timing.json', JSON.stringify(timings) + '\n')
62+
append(path.resolve(npm.config.get('cache'), '_timing.json'), JSON.stringify(timings) + '\n')
4063
} catch (_) {
4164
// ignore
4265
}
@@ -174,7 +197,7 @@ const errorHandler = (er) => {
174197
log.error(k, v)
175198
}
176199

177-
const msg = errorMessage(er)
200+
const msg = errorMessage(er, npm)
178201
for (const errline of [...msg.summary, ...msg.detail])
179202
log.error(...errline)
180203

@@ -214,7 +237,7 @@ const writeLogFile = () => {
214237
logOutput += line + os.EOL
215238
})
216239
})
217-
cacheFile.write(getLogFile(), logOutput)
240+
write(getLogFile(), logOutput)
218241

219242
// truncate once it's been written.
220243
log.record.length = 0
@@ -226,3 +249,7 @@ const writeLogFile = () => {
226249

227250
module.exports = errorHandler
228251
module.exports.exit = exit
252+
module.exports.setNpm = (n) => {
253+
npm = n
254+
timings.version = npm.version
255+
}

lib/utils/error-message.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
const npm = require('../npm.js')
21
const { format } = require('util')
32
const { resolve } = require('path')
43
const nameValidator = require('validate-npm-package-name')
54
const npmlog = require('npmlog')
65
const replaceInfo = require('./replace-info.js')
7-
const { report: explainEresolve } = require('./explain-eresolve.js')
6+
const { report } = require('./explain-eresolve.js')
87

9-
module.exports = (er) => {
8+
module.exports = (er, npm) => {
109
const short = []
1110
const detail = []
1211

@@ -19,7 +18,7 @@ module.exports = (er) => {
1918
case 'ERESOLVE':
2019
short.push(['ERESOLVE', er.message])
2120
detail.push(['', ''])
22-
detail.push(['', explainEresolve(er)])
21+
detail.push(['', report(er, npm.color, resolve(npm.cache, 'eresolve-report.txt'))])
2322
break
2423

2524
case 'ENOLOCK': {

lib/utils/explain-eresolve.js

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
11
// this is called when an ERESOLVE error is caught in the error-handler,
22
// or when there's a log.warn('eresolve', msg, explanation), to turn it
33
// into a human-intelligible explanation of what's wrong and how to fix.
4-
//
5-
// TODO: abstract out the explainNode methods into a separate util for
6-
// use by a future `npm explain <path || spec>` command.
7-
8-
const npm = require('../npm.js')
94
const { writeFileSync } = require('fs')
10-
const { resolve } = require('path')
115
const { explainEdge, explainNode, printNode } = require('./explain-dep.js')
126

137
// expl is an explanation object that comes from Arborist. It looks like:
148
// Depth is how far we want to want to descend into the object making a report.
159
// The full report (ie, depth=Infinity) is always written to the cache folder
1610
// at ${cache}/eresolve-report.txt along with full json.
17-
const explainEresolve = (expl, color, depth) => {
11+
const explain = (expl, color, depth) => {
1812
const { edge, current, peerConflict, currentEdge } = expl
1913

2014
const out = []
@@ -42,9 +36,7 @@ const explainEresolve = (expl, color, depth) => {
4236
}
4337

4438
// generate a full verbose report and tell the user how to fix it
45-
const report = (expl, depth = 4) => {
46-
const fullReport = resolve(npm.cache, 'eresolve-report.txt')
47-
39+
const report = (expl, color, fullReport) => {
4840
const orNoStrict = expl.strictPeerDeps ? '--no-strict-peer-deps, ' : ''
4941
const fix = `Fix the upstream dependency conflict, or retry
5042
this command with ${orNoStrict}--force, or --legacy-peer-deps
@@ -54,7 +46,7 @@ to accept an incorrect (and potentially broken) dependency resolution.`
5446
5547
${new Date().toISOString()}
5648
57-
${explainEresolve(expl, false, Infinity)}
49+
${explain(expl, false, Infinity)}
5850
5951
${fix}
6052
@@ -63,13 +55,10 @@ Raw JSON explanation object:
6355
${JSON.stringify(expl, null, 2)}
6456
`, 'utf8')
6557

66-
return explainEresolve(expl, npm.color, depth) +
58+
return explain(expl, color, 4) +
6759
`\n\n${fix}\n\nSee ${fullReport} for a full report.`
6860
}
6961

70-
// the terser explain method for the warning when using --force
71-
const explain = (expl, depth = 2) => explainEresolve(expl, npm.color, depth)
72-
7362
module.exports = {
7463
explain,
7564
report,

lib/utils/setup-log.js

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,22 @@ module.exports = (config) => {
1414

1515
const { warn } = log
1616

17+
const stdoutTTY = process.stdout.isTTY
18+
const stderrTTY = process.stderr.isTTY
19+
const dumbTerm = process.env.TERM === 'dumb'
20+
const stderrNotDumb = stderrTTY && !dumbTerm
21+
const enableColorStderr = color === 'always' ? true
22+
: color === false ? false
23+
: stderrTTY
24+
25+
const enableColorStdout = color === 'always' ? true
26+
: color === false ? false
27+
: stdoutTTY
28+
1729
log.warn = (heading, ...args) => {
1830
if (heading === 'ERESOLVE' && args[1] && typeof args[1] === 'object') {
1931
warn(heading, args[0])
20-
return warn('', explain(args[1]))
32+
return warn('', explain(args[1], enableColorStdout, 2))
2133
}
2234
return warn(heading, ...args)
2335
}
@@ -29,19 +41,6 @@ module.exports = (config) => {
2941

3042
log.heading = config.get('heading') || 'npm'
3143

32-
const stdoutTTY = process.stdout.isTTY
33-
const stderrTTY = process.stderr.isTTY
34-
const dumbTerm = process.env.TERM === 'dumb'
35-
const stderrNotDumb = stderrTTY && !dumbTerm
36-
37-
const enableColorStderr = color === 'always' ? true
38-
: color === false ? false
39-
: stderrTTY
40-
41-
const enableColorStdout = color === 'always' ? true
42-
: color === false ? false
43-
: stdoutTTY
44-
4544
if (enableColorStderr)
4645
log.enableColor()
4746
else

tap-snapshots/test/lib/utils/error-handler.js.test.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = `
99
0 verbose code 1
1010
1 error foo A complete log of this run can be found in:
11-
1 error foo {CWD}/cachefolder/_logs/expecteddate-debug.log
11+
1 error foo {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log
1212
2 verbose stack Error: ERROR
1313
3 verbose cwd {CWD}
1414
4 verbose Foo 1.0.0

0 commit comments

Comments
 (0)