Skip to content

fix(view): dont unwrap arrays in json mode#7506

Merged
lukekarrys merged 1 commit intolatestfrom
lk/view-versions-json
May 11, 2024
Merged

fix(view): dont unwrap arrays in json mode#7506
lukekarrys merged 1 commit intolatestfrom
lk/view-versions-json

Conversation

@lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented May 10, 2024 •
edited
Loading

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
@lukekarrys
fix(view): dont unwrap arrays in json mode
d3ec5b5 
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
@lukekarrys lukekarrys force-pushed the lk/view-versions-json branch from 0be70c4 to d3ec5b5 Compare May 10, 2024 19:52
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented May 10, 2024 •
edited
Loading

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

wraithgar commented May 11, 2024

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 •
edited
Loading

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
Contributor

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
@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
Merged
@npm-cli-bot npm-cli-bot mentioned this pull request May 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wraithgar wraithgar wraithgar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

@lukekarrys @npm-cli-bot @wraithgar @ljharb