diff --git a/lib/commands/pkg.js b/lib/commands/pkg.js index a108d79c51517..a011fc10be107 100644 --- a/lib/commands/pkg.js +++ b/lib/commands/pkg.js @@ -59,28 +59,24 @@ class Pkg extends BaseCommand { this.npm.config.set('json', true) const pkgJson = await PackageJson.load(path) - let unwrap = false let result = pkgJson.content if (args.length) { result = new Queryable(result).query(args) // in case there's only a single result from the query // just prints that one element to stdout + // TODO(BREAKING_CHANGE): much like other places where we unwrap single + // item arrays this should go away. it makes the behavior unknown for users + // who don't already know the shape of the data. if (Object.keys(result).length === 1) { - unwrap = true result = result[args] } } - if (workspace) { - // workspaces are always json - output.buffer({ [workspace]: result }) - } else { - // if the result was unwrapped, stringify as json which will add quotes around strings - // TODO: https://github.com/npm/cli/issues/5508 a raw mode has been requested similar - // to jq -r. If that was added then it would conditionally not call JSON.stringify here - output.buffer(unwrap ? JSON.stringify(result) : result) - } + // The display layer is responsible for calling JSON.stringify on the result + // TODO: https://github.com/npm/cli/issues/5508 a raw mode has been requested similar + // to jq -r. If that was added then this method should no longer set `json:true` all the time + output.buffer(workspace ? { [workspace]: result } : result) } async set (args, { path }) { diff --git a/lib/utils/display.js b/lib/utils/display.js index 3716630dbd9b1..67a3b98c0417a 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -82,26 +82,31 @@ const ERROR_KEY = 'error' // is a json error that should be merged into the finished output const JSON_ERROR_KEY = 'jsonError' +const isPlainObject = (v) => v && typeof v === 'object' && !Array.isArray(v) + const getArrayOrObject = (items) => { - // Arrays cant be merged, if the first item is an array return that - if (Array.isArray(items[0])) { - return items[0] - } - // We use objects with 0,1,2,etc keys to merge array - if (items.length && items.every((o, i) => Object.hasOwn(o, i))) { - return Object.assign([], ...items) + if (items.length) { + const foundNonObject = items.find(o => !isPlainObject(o)) + // Non-objects and arrays cant be merged, so just return the first item + if (foundNonObject) { + return foundNonObject + } + // We use objects with 0,1,2,etc keys to merge array + if (items.every((o, i) => Object.hasOwn(o, i))) { + return Object.assign([], ...items) + } } - // Otherwise its an object with all items merged together - return Object.assign({}, ...items) + // Otherwise its an object with all object items merged together + return Object.assign({}, ...items.filter(o => isPlainObject(o))) } -const mergeJson = ({ [JSON_ERROR_KEY]: metaError }, buffer) => { +const getJsonBuffer = ({ [JSON_ERROR_KEY]: metaError }, buffer) => { const items = [] // meta also contains the meta object passed to flush const errors = metaError ? [metaError] : [] // index 1 is the meta, 2 is the logged argument for (const [, { [JSON_ERROR_KEY]: error }, obj] of buffer) { - if (obj && typeof obj === 'object') { + if (obj) { items.push(obj) } if (error) { @@ -113,13 +118,12 @@ const mergeJson = ({ [JSON_ERROR_KEY]: metaError }, buffer) => { return null } - // If all items are keyed with array indexes, then we return the - // array. This skips any error checking since we cant really set - // an error property on an array in a way that can be stringified - // XXX(BREAKING_CHANGE): remove this in favor of always returning an object const res = getArrayOrObject(items) - if (!Array.isArray(res) && errors.length) { + // This skips any error checking since we can only set an error property + // on an object that can be stringified + // XXX(BREAKING_CHANGE): remove this in favor of always returning an object with result and error keys + if (isPlainObject(res) && errors.length) { // This is not ideal. JSON output has always been keyed at the root with an `error` // key, so we cant change that without it being a breaking change. At the same time // some commands output arbitrary keys at the top level of the output, such as package @@ -292,9 +296,11 @@ class Display { switch (level) { case output.KEYS.flush: { this.#outputState.buffering = false - const json = this.#json ? mergeJson(meta, this.#outputState.buffer) : null - if (json) { - this.#writeOutput(output.KEYS.standard, meta, JSON.stringify(json, null, 2)) + if (this.#json) { + const json = getJsonBuffer(meta, this.#outputState.buffer) + if (json) { + this.#writeOutput(output.KEYS.standard, meta, JSON.stringify(json, null, 2)) + } } else { this.#outputState.buffer.forEach((item) => this.#writeOutput(...item)) } diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js index da050bbb0708e..90d130a399265 100644 --- a/test/lib/cli/exit-handler.js +++ b/test/lib/cli/exit-handler.js @@ -277,7 +277,6 @@ t.test('merges output buffers errors with --json', async (t) => { output.buffer({ output_data: 1 }) output.buffer({ more_data: 2 }) - output.buffer('not json, will be ignored') await exitHandler() diff --git a/test/lib/commands/view.js b/test/lib/commands/view.js index 2480a39a4a373..5da9182ddd55e 100644 --- a/test/lib/commands/view.js +++ b/test/lib/commands/view.js @@ -406,6 +406,12 @@ t.test('package with --json and no versions', async t => { t.equal(joinedOutput(), '', 'no info to display') }) +t.test('package with --json and single string arg', async t => { + const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) + await view.exec(['blue', 'dist-tags.latest']) + t.equal(JSON.parse(joinedOutput()), '1.0.0', 'no info to display') +}) + t.test('package with single version', async t => { t.test('full json', async t => { const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } })