Skip to content

fix(@angular/cli): robustly parse npm manifest from array#33147

Merged
clydin merged 2 commits intoangular:mainfrom
clydin:fix/cli/robust-npm-manifest-parsing
May 7, 2026
Merged

fix(@angular/cli): robustly parse npm manifest from array#33147
clydin merged 2 commits intoangular:mainfrom
clydin:fix/cli/robust-npm-manifest-parsing

Conversation

@clydin
Copy link
Copy Markdown
Member

@clydin clydin commented May 7, 2026

Update parseNpmLikeManifest to find the manifest with the highest version instead of assuming the last item in the array is the correct one. This makes the parsing robust against out-of-order results from npm view or similar commands without incurring performance penalties.

This change ensures that the latest relevant version is always selected when a range is queried and multiple versions are returned, improving reliability in edge cases where registry output might not be sorted.

Update `parseNpmLikeManifest` to find the manifest with the highest version instead of assuming the last item in the array is the correct one. This makes the parsing robust against out-of-order results from `npm view` or similar commands without incurring performance penalties.

This change ensures that the latest relevant version is always selected when a range is queried and multiple versions are returned, improving reliability in edge cases where registry output might not be sorted.
@clydin clydin added the target: patch This PR is targeted for the next patch release label May 7, 2026
@clydin clydin marked this pull request as ready for review May 7, 2026 14:07
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the parseNpmLikeManifest function to handle multiple manifests by selecting the highest versioned entry, supported by a new validation helper and expanded unit tests. Review feedback suggests further strengthening the validation logic to verify semantic version formats using the semver package, refining debug log messages for clarity, and adding test coverage for invalid version strings.

Comment thread packages/angular/cli/src/package-managers/parsers.ts Outdated
Comment thread packages/angular/cli/src/package-managers/parsers.ts Outdated
Comment thread packages/angular/cli/src/package-managers/parsers.ts Outdated
Comment thread packages/angular/cli/src/package-managers/parsers.ts Outdated
Comment thread packages/angular/cli/src/package-managers/parsers_spec.ts
@clydin clydin force-pushed the fix/cli/robust-npm-manifest-parsing branch 2 times, most recently from c6cf19b to 07ce1e4 Compare May 7, 2026 14:21
@clydin clydin added the action: review The PR is still awaiting reviews from at least one requested reviewer label May 7, 2026
@clydin clydin requested a review from alan-agius4 May 7, 2026 14:22
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, after the lint issue is fixed.

@alan-agius4 alan-agius4 added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 7, 2026
@clydin clydin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 7, 2026
@clydin clydin force-pushed the fix/cli/robust-npm-manifest-parsing branch from 07ce1e4 to 4f0385f Compare May 7, 2026 15:01
…rsing

Introduce a reusable `isValidManifest` type guard to ensure that parsed manifests contain both `name` and `version` strings. This applies to both single object responses and elements within an array response from the package manager.
@clydin clydin force-pushed the fix/cli/robust-npm-manifest-parsing branch from 4f0385f to 4a78ddc Compare May 7, 2026 15:07
@clydin clydin merged commit db3c5d8 into angular:main May 7, 2026
30 of 34 checks passed
@clydin
Copy link
Copy Markdown
Member Author

clydin commented May 7, 2026

This PR was merged into the repository. The changes were merged into the following branches:

@JohnArcher
Copy link
Copy Markdown

Hey there,

thanks for your quick response to issue #33146 and implementing a fix. I see that you only added validation steps to the final PR that was merged into main and 21.2.x.

But the key problem - sorting for the latest version - that you initially fixed in 5c1db4e (Line 241 to 257) was not picked for the final PR. Why is that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/cli target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ng update / ng add picks wrong CLI version: parseNpmLikeManifest assumes registry array is sorted by semver

3 participants