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: pass strings to JSON.stringify in --json mode #7540

Merged
merged 1 commit into from
May 17, 2024

Conversation

lukekarrys
Copy link
Contributor

In refactoring this behavior previously plain strings were no longer
being passed through JSON.stringify even in json mode. This commit
changes that to the previous behavior which fixes the bug in npm view.
This also required changing the behavior of npm pkg which relied on this.

Fixes #7537

@lukekarrys lukekarrys requested a review from a team as a code owner May 16, 2024 23:59
@lukekarrys lukekarrys force-pushed the lk/strings-in-json-mode branch from 053033c to ef96b6b Compare May 17, 2024 00:01
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented May 17, 2024

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 31.815 ±0.29 10.675 ±0.02 11.826 ±0.04 1.578 ±0.01 1.569 ±0.00 1.292 ±0.01 8.426 ±0.01 1.296 ±0.00 0.140 ±0.00 0.168 ±0.00 15.101 ±0.16 2.134 ±0.05
#7540 33.871 ±6.18 10.658 ±0.04 11.818 ±0.01 1.534 ±0.00 1.548 ±0.00 1.274 ±0.00 8.301 ±0.02 1.280 ±0.04 0.138 ±0.00 0.165 ±0.00 14.673 ±0.21 2.100 ±0.03
app-medium clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps
no-clean
audit
npm@latest 25.024 ±0.40 8.029 ±0.00 8.932 ±0.02 1.483 ±0.01 1.500 ±0.02 1.432 ±0.02 5.838 ±0.04 1.314 ±0.01 0.138 ±0.00 0.166 ±0.00 9.985 ±0.06 2.013 ±0.09
#7540 24.886 ±0.76 7.996 ±0.00 8.873 ±0.03 1.500 ±0.02 1.492 ±0.01 1.405 ±0.01 5.826 ±0.01 1.311 ±0.01 0.138 ±0.00 0.165 ±0.00 9.898 ±0.13 1.947 ±0.02

@lukekarrys lukekarrys marked this pull request as draft May 17, 2024 00:21
In refactoring this behavior previously plain strings were no longer
being passed through JSON.stringify even in json mode. This commit
changes that to the previous behavior which fixes the bug in `npm view`.
This also required changing the behavior of `npm pkg` which relied on this.

Fixes #7537
@lukekarrys lukekarrys force-pushed the lk/strings-in-json-mode branch from ef96b6b to 8b03990 Compare May 17, 2024 00:29
@lukekarrys lukekarrys marked this pull request as ready for review May 17, 2024 00:33
// 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)))

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items is an array of objects, so i dont think it can be spread within another object in the same way as arguments to a function

> Object.assign({}, ...[{a:1},{b:2}])
{ a: 1, b: 2 }
> { ...[{a:1},{b:2}] }
{ '0': { a: 1 }, '1': { b: 2 } }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh you're totally right

@lukekarrys lukekarrys merged commit 4dfc7d2 into latest May 17, 2024
23 checks passed
@lukekarrys lukekarrys deleted the lk/strings-in-json-mode branch May 17, 2024 04:37
@github-actions github-actions bot mentioned this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] npm view returning string without quotes with version 10.8.0
4 participants