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(view): dont unwrap arrays in json mode #7506

Merged
merged 1 commit into from
May 11, 2024
Merged

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented May 10, 2024

The view command alters the data by default to unwrap single item arrays
when in human readable mode (the default). This change makes it so those
arrays are not altered when the --json flag is set.

Fixes #3611

# Before
❯ npm view tiny-tarball versions
1.0.0
❯ npm view tiny-tarball versions --json
"1.0.0"
# After
❯ npmlocal view tiny-tarball versions
1.0.0
❯ npmlocal view tiny-tarball versions --json
[
  "1.0.0"
]

@lukekarrys lukekarrys requested a review from a team as a code owner May 10, 2024 19:46
@lukekarrys lukekarrys changed the title fix(view): dont unwrap arrays in json modified fix(view): dont unwrap arrays in json mode May 10, 2024
@lukekarrys lukekarrys force-pushed the lk/view-versions-json branch 2 times, most recently from 986ac2a to 0be70c4 Compare May 10, 2024 19:50
The view command alters the data by default to unwrap single item arrays
when in human readable mode (the default). This change makes it so those
arrays are not altered when the --json flag is set.

Fixes #3611
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented May 10, 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 35.938 ±1.02 10.448 ±0.06 11.486 ±0.04 1.552 ±0.00 1.519 ±0.00 1.266 ±0.01 8.016 ±0.03 1.290 ±0.02 0.138 ±0.00 0.165 ±0.01 13.049 ±0.13 4.171 ±2.96
#7506 39.301 ±0.37 10.557 ±0.05 11.589 ±0.07 1.538 ±0.04 1.524 ±0.01 1.260 ±0.00 8.147 ±0.07 1.277 ±0.01 0.137 ±0.00 0.163 ±0.00 14.469 ±0.14 2.112 ±0.02
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 30.000 ±0.97 7.860 ±0.02 8.758 ±0.00 1.488 ±0.02 1.502 ±0.00 1.396 ±0.00 5.801 ±0.01 1.326 ±0.01 0.139 ±0.00 0.169 ±0.00 9.375 ±0.20 2.988 ±1.46
#7506 29.355 ±0.96 7.845 ±0.04 8.810 ±0.04 1.492 ±0.02 1.487 ±0.00 1.395 ±0.02 5.777 ±0.01 1.288 ±0.03 0.135 ±0.00 0.164 ±0.00 9.640 ±0.05 1.947 ±0.00

@wraithgar
Copy link
Member

This feels like one of those incorrect behaviors folks have undoubtedly come to rely on, and this would be a breaking change.

@lukekarrys
Copy link
Contributor Author

lukekarrys commented May 11, 2024

I'm not so sure, because currently this behavior is impossible to predict unless you know the data being returned. In the case of versions it will only unwrap the array if the package has only one version. It will properly keep it as an array for packages with multiple versions. So I think it's unlikely people are relying on this as they would still need to handle when versions returns an array.

# before
❯ npm view abbrev versions --json
[
  "1.0.3",
  "1.0.4",
  "1.0.5",
  "1.0.6",
  "1.0.7",
  "1.0.9",
  "1.1.0",
  "1.1.1",
  "2.0.0"
]
# after
❯ npmlocal view abbrev versions --json
[
  "1.0.3",
  "1.0.4",
  "1.0.5",
  "1.0.6",
  "1.0.7",
  "1.0.9",
  "1.1.0",
  "1.1.1",
  "2.0.0"
]

@ljharb
Copy link
Collaborator

ljharb commented May 11, 2024

If they did the smart thing and always wrapped the field in [].concat() then this wouldn’t break them, at least :-)

@lukekarrys lukekarrys merged commit e40454c into latest May 11, 2024
23 checks passed
@lukekarrys lukekarrys deleted the lk/view-versions-json branch May 11, 2024 07:38
@github-actions github-actions bot mentioned this pull request May 11, 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 $package versions --json returns non-JSON string for packages with only one version
4 participants