Skip to content

Commit 61d5771

Browse files
lukekarryslukekarrys
authoredMay 17, 2024··
fix: remove json.stringify from all commands (#7541)
This converts all remaining commands/utils to use the display layer for formatting their json output
·
v11.2.0arborist-v7.5.3
1 parent 4dfc7d2 commit 61d5771

22 files changed

+95
-106
lines changed
 

‎lib/commands/access.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class Access extends BaseCommand {
208208
outputs[item] = lookup[val] || val
209209
}
210210
if (this.npm.config.get('json')) {
211-
output.standard(JSON.stringify(outputs, null, 2))
211+
output.buffer(outputs)
212212
} else {
213213
for (const item of Object.keys(outputs).sort(localeCompare)) {
214214
if (!limiter || limiter === item) {

‎lib/commands/config.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ ${defData}
403403

404404
publicConf[key] = value
405405
}
406-
output.standard(JSON.stringify(publicConf, null, 2))
406+
output.buffer(publicConf)
407407
}
408408
}
409409

‎lib/commands/explain.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class Explain extends ArboristWorkspaceCmd {
7676
}
7777

7878
if (this.npm.flatOptions.json) {
79-
output.standard(JSON.stringify(expls, null, 2))
79+
output.buffer(expls)
8080
} else {
8181
output.standard(expls.map(expl => {
8282
return explainNode(expl, Infinity, this.npm.chalk)

‎lib/commands/fund.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,12 @@ class Fund extends ArboristWorkspaceCmd {
8585
})
8686

8787
if (this.npm.config.get('json')) {
88-
output.standard(this.printJSON(fundingInfo))
88+
output.buffer(fundingInfo)
8989
} else {
9090
output.standard(this.printHuman(fundingInfo))
9191
}
9292
}
9393

94-
printJSON (fundingInfo) {
95-
return JSON.stringify(fundingInfo, null, 2)
96-
}
97-
9894
printHuman (fundingInfo) {
9995
const unicode = this.npm.config.get('unicode')
10096
const seenUrls = new Map()

‎lib/commands/hook.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class Hook extends BaseCommand {
4040
async add (pkg, uri, secret, opts) {
4141
const hook = await hookApi.add(pkg, uri, secret, opts)
4242
if (opts.json) {
43-
output.standard(JSON.stringify(hook, null, 2))
43+
output.buffer(hook)
4444
} else if (opts.parseable) {
4545
output.standard(Object.keys(hook).join('\t'))
4646
output.standard(Object.keys(hook).map(k => hook[k]).join('\t'))
@@ -53,7 +53,7 @@ class Hook extends BaseCommand {
5353
const hooks = await hookApi.ls({ ...opts, package: pkg })
5454

5555
if (opts.json) {
56-
output.standard(JSON.stringify(hooks, null, 2))
56+
output.buffer(hooks)
5757
} else if (opts.parseable) {
5858
output.standard(Object.keys(hooks[0]).join('\t'))
5959
hooks.forEach(hook => {
@@ -80,7 +80,7 @@ class Hook extends BaseCommand {
8080
async rm (id, opts) {
8181
const hook = await hookApi.rm(id, opts)
8282
if (opts.json) {
83-
output.standard(JSON.stringify(hook, null, 2))
83+
output.buffer(hook)
8484
} else if (opts.parseable) {
8585
output.standard(Object.keys(hook).join('\t'))
8686
output.standard(Object.keys(hook).map(k => hook[k]).join('\t'))
@@ -92,7 +92,7 @@ class Hook extends BaseCommand {
9292
async update (id, uri, secret, opts) {
9393
const hook = await hookApi.update(id, uri, secret, opts)
9494
if (opts.json) {
95-
output.standard(JSON.stringify(hook, null, 2))
95+
output.buffer(hook)
9696
} else if (opts.parseable) {
9797
output.standard(Object.keys(hook).join('\t'))
9898
output.standard(Object.keys(hook).map(k => hook[k]).join('\t'))

‎lib/commands/org.js

+7-9
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,12 @@ class Org extends BaseCommand {
100100
org = org.replace(/^[~@]?/, '')
101101
const userCount = Object.keys(roster).length
102102
if (opts.json) {
103-
output.standard(
104-
JSON.stringify({
105-
user,
106-
org,
107-
userCount,
108-
deleted: true,
109-
})
110-
)
103+
output.buffer({
104+
user,
105+
org,
106+
userCount,
107+
deleted: true,
108+
})
111109
} else if (opts.parseable) {
112110
output.standard(['user', 'org', 'userCount', 'deleted'].join('\t'))
113111
output.standard([user, org, userCount, true].join('\t'))
@@ -135,7 +133,7 @@ class Org extends BaseCommand {
135133
roster = newRoster
136134
}
137135
if (opts.json) {
138-
output.standard(JSON.stringify(roster, null, 2))
136+
output.buffer(roster)
139137
} else if (opts.parseable) {
140138
output.standard(['user', 'role'].join('\t'))
141139
Object.keys(roster).forEach(u => {

‎lib/commands/ping.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ class Ping extends BaseCommand {
1616
const time = Date.now() - start
1717
log.notice('PONG', `${time}ms`)
1818
if (this.npm.config.get('json')) {
19-
output.standard(JSON.stringify({
19+
output.buffer({
2020
registry: cleanRegistry,
2121
time,
2222
details,
23-
}, null, 2))
23+
})
2424
} else if (Object.keys(details).length) {
2525
log.notice('PONG', JSON.stringify(details, null, 2))
2626
}

‎lib/commands/profile.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class Profile extends BaseCommand {
108108
}
109109

110110
if (this.npm.config.get('json')) {
111-
output.standard(JSON.stringify(info, null, 2))
111+
output.buffer(info)
112112
return
113113
}
114114

@@ -211,7 +211,7 @@ class Profile extends BaseCommand {
211211
const result = await otplease(this.npm, conf, c => set(newUser, c))
212212

213213
if (this.npm.config.get('json')) {
214-
output.standard(JSON.stringify({ [prop]: result[prop] }, null, 2))
214+
output.buffer({ [prop]: result[prop] })
215215
} else if (this.npm.config.get('parseable')) {
216216
output.standard(prop + '\t' + result[prop])
217217
} else if (result[prop] != null) {
@@ -378,7 +378,7 @@ class Profile extends BaseCommand {
378378
await set({ tfa: { password: password, mode: 'disable' } }, conf)
379379

380380
if (this.npm.config.get('json')) {
381-
output.standard(JSON.stringify({ tfa: false }, null, 2))
381+
output.buffer({ tfa: false })
382382
} else if (this.npm.config.get('parseable')) {
383383
output.standard('tfa\tfalse')
384384
} else {

‎lib/commands/sbom.js

+22-40
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ class SBOM extends BaseCommand {
2222
'workspaces',
2323
]
2424

25-
get #parsedResponse () {
26-
return JSON.stringify(this.#response, null, 2)
27-
}
28-
2925
async exec () {
3026
const sbomFormat = this.npm.config.get('sbom-format')
3127
const packageLockOnly = this.npm.config.get('package-lock-only')
@@ -35,56 +31,43 @@ class SBOM extends BaseCommand {
3531
throw this.usageError(`Must specify --sbom-format flag with one of: ${SBOM_FORMATS.join(', ')}.`)
3632
}
3733

38-
const Arborist = require('@npmcli/arborist')
39-
4034
const opts = {
4135
...this.npm.flatOptions,
4236
path: this.npm.prefix,
4337
forceActual: true,
4438
}
39+
const Arborist = require('@npmcli/arborist')
4540
const arb = new Arborist(opts)
4641

47-
let tree
48-
if (packageLockOnly) {
49-
try {
50-
tree = await arb.loadVirtual(opts)
51-
} catch (err) {
52-
/* eslint-disable-next-line max-len */
53-
throw this.usageError('A package lock or shrinkwrap file is required in package-lock-only mode')
54-
}
55-
} else {
56-
tree = await arb.loadActual(opts)
57-
}
42+
const tree = packageLockOnly ? await arb.loadVirtual(opts).catch(() => {
43+
/* eslint-disable-next-line max-len */
44+
throw this.usageError('A package lock or shrinkwrap file is required in package-lock-only mode')
45+
}) : await arb.loadActual(opts)
5846

5947
// Collect the list of selected workspaces in the project
60-
let wsNodes
61-
if (this.workspaceNames && this.workspaceNames.length) {
62-
wsNodes = arb.workspaceNodes(tree, this.workspaceNames)
63-
}
48+
const wsNodes = this.workspaceNames?.length
49+
? arb.workspaceNodes(tree, this.workspaceNames)
50+
: null
6451

6552
// Build the selector and query the tree for the list of nodes
6653
const selector = this.#buildSelector({ wsNodes })
6754
log.info('sbom', `Using dependency selector: ${selector}`)
6855
const items = await tree.querySelectorAll(selector)
6956

70-
const errors = new Set()
71-
for (const node of items) {
72-
detectErrors(node).forEach(error => errors.add(error))
73-
}
74-
75-
if (errors.size > 0) {
76-
throw Object.assign(
77-
new Error([...errors].join('\n')),
78-
{ code: 'ESBOMPROBLEMS' }
79-
)
57+
const errors = items.flatMap(node => detectErrors(node))
58+
if (errors.length) {
59+
throw Object.assign(new Error([...new Set(errors)].join('\n')), {
60+
code: 'ESBOMPROBLEMS',
61+
})
8062
}
8163

8264
// Populate the response with the list of unique nodes (sorted by location)
83-
this.#buildResponse(
84-
items
85-
.sort((a, b) => localeCompare(a.location, b.location))
86-
)
87-
output.standard(this.#parsedResponse)
65+
this.#buildResponse(items.sort((a, b) => localeCompare(a.location, b.location)))
66+
67+
// TODO(BREAKING_CHANGE): all sbom output is in json mode but setting it before
68+
// any of the errors will cause those to be thrown in json mode.
69+
this.npm.config.set('json', true)
70+
output.buffer(this.#response)
8871
}
8972

9073
async execWorkspaces (args) {
@@ -122,10 +105,9 @@ class SBOM extends BaseCommand {
122105
const packageType = this.npm.config.get('sbom-type')
123106
const packageLockOnly = this.npm.config.get('package-lock-only')
124107

125-
this.#response =
126-
sbomFormat === 'cyclonedx'
127-
? cyclonedxOutput({ npm: this.npm, nodes: items, packageType, packageLockOnly })
128-
: spdxOutput({ npm: this.npm, nodes: items, packageType })
108+
this.#response = sbomFormat === 'cyclonedx'
109+
? cyclonedxOutput({ npm: this.npm, nodes: items, packageType, packageLockOnly })
110+
: spdxOutput({ npm: this.npm, nodes: items, packageType })
129111
}
130112
}
131113

‎lib/commands/team.js

+10-10
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ class Team extends BaseCommand {
6868
async create (entity, opts) {
6969
await libteam.create(entity, opts)
7070
if (opts.json) {
71-
output.standard(JSON.stringify({
71+
output.buffer({
7272
created: true,
7373
team: entity,
74-
}))
74+
})
7575
} else if (opts.parseable) {
7676
output.standard(`${entity}\tcreated`)
7777
} else if (!this.npm.silent) {
@@ -82,10 +82,10 @@ class Team extends BaseCommand {
8282
async destroy (entity, opts) {
8383
await libteam.destroy(entity, opts)
8484
if (opts.json) {
85-
output.standard(JSON.stringify({
85+
output.buffer({
8686
deleted: true,
8787
team: entity,
88-
}))
88+
})
8989
} else if (opts.parseable) {
9090
output.standard(`${entity}\tdeleted`)
9191
} else if (!this.npm.silent) {
@@ -96,11 +96,11 @@ class Team extends BaseCommand {
9696
async add (entity, user, opts) {
9797
await libteam.add(user, entity, opts)
9898
if (opts.json) {
99-
output.standard(JSON.stringify({
99+
output.buffer({
100100
added: true,
101101
team: entity,
102102
user,
103-
}))
103+
})
104104
} else if (opts.parseable) {
105105
output.standard(`${user}\t${entity}\tadded`)
106106
} else if (!this.npm.silent) {
@@ -111,11 +111,11 @@ class Team extends BaseCommand {
111111
async rm (entity, user, opts) {
112112
await libteam.rm(user, entity, opts)
113113
if (opts.json) {
114-
output.standard(JSON.stringify({
114+
output.buffer({
115115
removed: true,
116116
team: entity,
117117
user,
118-
}))
118+
})
119119
} else if (opts.parseable) {
120120
output.standard(`${user}\t${entity}\tremoved`)
121121
} else if (!this.npm.silent) {
@@ -126,7 +126,7 @@ class Team extends BaseCommand {
126126
async listUsers (entity, opts) {
127127
const users = (await libteam.lsUsers(entity, opts)).sort()
128128
if (opts.json) {
129-
output.standard(JSON.stringify(users, null, 2))
129+
output.buffer(users)
130130
} else if (opts.parseable) {
131131
output.standard(users.join('\n'))
132132
} else if (!this.npm.silent) {
@@ -140,7 +140,7 @@ class Team extends BaseCommand {
140140
async listTeams (entity, opts) {
141141
const teams = (await libteam.lsTeams(entity, opts)).sort()
142142
if (opts.json) {
143-
output.standard(JSON.stringify(teams, null, 2))
143+
output.buffer(teams)
144144
} else if (opts.parseable) {
145145
output.standard(teams.join('\n'))
146146
} else if (!this.npm.silent) {

‎lib/commands/token.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class Token extends BaseCommand {
5050
log.info('token', 'getting list')
5151
const tokens = await listTokens(this.npm.flatOptions)
5252
if (json) {
53-
output.standard(JSON.stringify(tokens, null, 2))
53+
output.buffer(tokens)
5454
return
5555
}
5656
if (parseable) {
@@ -117,7 +117,7 @@ class Token extends BaseCommand {
117117
})
118118
)
119119
if (json) {
120-
output.standard(JSON.stringify(toRemove))
120+
output.buffer(toRemove)
121121
} else if (parseable) {
122122
output.standard(toRemove.join('\t'))
123123
} else {
@@ -142,7 +142,7 @@ class Token extends BaseCommand {
142142
delete result.key
143143
delete result.updated
144144
if (json) {
145-
output.standard(JSON.stringify(result))
145+
output.buffer(result)
146146
} else if (parseable) {
147147
Object.keys(result).forEach(k => output.standard(k + '\t' + result[k]))
148148
} else {

‎lib/commands/version.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ class Version extends BaseCommand {
126126
}
127127

128128
if (this.npm.config.get('json')) {
129-
output.standard(JSON.stringify(results, null, 2))
129+
output.buffer(results)
130130
} else {
131131
output.standard(results)
132132
}

‎lib/commands/whoami.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ class Whoami extends BaseCommand {
99

1010
async exec () {
1111
const username = await getIdentity(this.npm, { ...this.npm.flatOptions })
12-
output.standard(
13-
this.npm.config.get('json') ? JSON.stringify(username) : username
14-
)
12+
if (this.npm.config.get('json')) {
13+
output.buffer(username)
14+
} else {
15+
output.standard(username)
16+
}
1517
}
1618
}
1719

‎lib/npm.js

+10-11
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,22 @@ class Npm {
260260
this.#logFile.off()
261261
}
262262

263-
finish () {
263+
finish (err) {
264264
// Finish all our timer work, this will write the file if requested, end timers, etc
265265
this.#timers.finish({
266266
id: this.#runId,
267267
command: this.#argvClean,
268268
logfiles: this.logFiles,
269269
version: this.version,
270270
})
271+
272+
output.flush({
273+
[META]: true,
274+
// json can be set during a command so we send the
275+
// final value of it to the display layer here
276+
json: this.loaded && this.config.get('json'),
277+
jsonError: jsonError(err, this),
278+
})
271279
}
272280

273281
exitErrorMessage () {
@@ -296,16 +304,7 @@ class Npm {
296304
Object.assign(err, this.#getError(err, { pkg: localPkg }))
297305
}
298306

299-
// TODO: make this not need to be public
300-
this.finish()
301-
302-
output.flush({
303-
[META]: true,
304-
// json can be set during a command so we send the
305-
// final value of it to the display layer here
306-
json: this.loaded && this.config.get('json'),
307-
jsonError: jsonError(err, this),
308-
})
307+
this.finish(err)
309308

310309
if (err) {
311310
throw err

‎lib/utils/audit-error.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ const auditError = (npm, report) => {
2222
const { body: errBody } = error
2323
const body = Buffer.isBuffer(errBody) ? errBody.toString() : errBody
2424
if (npm.flatOptions.json) {
25-
output.standard(JSON.stringify({
25+
output.buffer({
2626
message: error.message,
2727
method: error.method,
2828
uri: replaceInfo(error.uri),
2929
headers: error.headers,
3030
statusCode: error.statusCode,
3131
body,
32-
}, null, 2))
32+
})
3333
} else {
3434
output.standard(body)
3535
}

‎lib/utils/open-url.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,11 @@ const assertValidUrl = (url) => {
1515
}
1616

1717
const outputMsg = (json, title, url) => {
18-
const msg = json ? JSON.stringify({ title, url }) : `${title}:\n${url}`
19-
output.standard(msg)
18+
if (json) {
19+
output.buffer({ title, url })
20+
} else {
21+
output.standard(`${title}:\n${url}`)
22+
}
2023
}
2124

2225
// attempt to open URL in web-browser, print address otherwise:

‎lib/utils/reify-output.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ const reifyOutput = (npm, arb) => {
9090
summary.audit = npm.command === 'audit' ? auditReport
9191
: auditReport.toJSON().metadata
9292
}
93-
output.standard(JSON.stringify(summary, null, 2))
93+
output.buffer(summary)
9494
} else {
9595
packagesChangedMessage(npm, summary)
9696
packagesFundingMessage(npm, summary)

‎lib/utils/verify-signatures.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,7 @@ class VerifySignatures {
6060
}
6161

6262
if (this.npm.config.get('json')) {
63-
output.standard(JSON.stringify({
64-
invalid,
65-
missing,
66-
}, null, 2))
63+
output.buffer({ invalid, missing })
6764
return
6865
}
6966
const end = process.hrtime.bigint()

‎tap-snapshots/test/lib/utils/open-url.js.test.cjs

+8-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ https://www.npmjs.com
1111
`
1212

1313
exports[`test/lib/utils/open-url.js TAP open url prints where to go when browser is disabled and json is enabled > printed expected message 1`] = `
14-
{"title":"npm home","url":"https://www.npmjs.com"}
14+
{
15+
"title": "npm home",
16+
"url": "https://www.npmjs.com"
17+
}
1518
`
1619

1720
exports[`test/lib/utils/open-url.js TAP open url prints where to go when given browser does not exist > printed expected message 1`] = `
@@ -33,5 +36,8 @@ https://www.npmjs.com
3336
`
3437

3538
exports[`test/lib/utils/open-url.js TAP open url prompt prints json output > must match snapshot 1`] = `
36-
{"title":"npm home","url":"https://www.npmjs.com"}
39+
{
40+
"title": "npm home",
41+
"url": "https://www.npmjs.com"
42+
}
3743
`

‎test/lib/utils/audit-error.js

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ const auditError = async (t, { command, error, ...config } = {}) => {
1919
res.error = err
2020
}
2121

22+
mock.npm.finish()
23+
2224
return {
2325
...res,
2426
logs: mock.logs.warn.byTitle('audit'),

‎test/lib/utils/open-url.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => {
2727
await openWithNpm(...args)
2828
}
2929

30+
mock.npm.finish()
31+
3032
return {
3133
...mock,
3234
openUrl: openWithNpm,
@@ -101,6 +103,8 @@ const mockOpenUrlPrompt = async (t, {
101103
await openUrlPrompt(...args).catch((er) => error = er)
102104
}
103105

106+
mock.npm.finish()
107+
104108
return {
105109
...mock,
106110
openerUrl,
@@ -178,7 +182,6 @@ t.test('open url prompt', async t => {
178182

179183
t.test('does not error when opener can not find command', async t => {
180184
const { OUTPUT, error, openerUrl } = await mockOpenUrlPrompt(t, {
181-
// openerResult: new Error('Opener failed'),
182185
openerResult: Object.assign(new Error('Opener failed'), { code: 127 }),
183186
})
184187

‎test/lib/utils/reify-output.js

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const mockReify = async (t, reify, { command, ...config } = {}) => {
2323
})
2424

2525
reifyOutput(mock.npm, reify)
26+
mock.npm.finish()
2627

2728
return mock.joinedOutput()
2829
}

0 commit comments

Comments
 (0)
Please sign in to comment.