[npm-check-fork] - Replace package-json and throat dependencies #5565
+868
−320
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add INpmRegistryPackageResponse and INpmRegistryVersionMetadata interfaces to support the upcoming replacement of the package-json dependency with a local implementation using WebClient. - INpmRegistryVersionMetadata extends INpmCheckPackageVersion for backward compatibility - INpmRegistryPackageResponse models the full npm registry API response - Added JSDoc with links to npm registry API documentation
…data Implement NpmRegistryClient class to replace external package-json dependency with a self-contained HTTP client using Node.js built-in modules. Features: - INpmRegistryClientOptions for configuring registry URL, user agent, timeout - INpmRegistryClientResult for consistent error handling - Automatic scoped package URL encoding (@scope/name -> @scope%2Fname) - Support for gzip/deflate response decompression - Proper error handling for 404, HTTP errors, network errors, and timeouts
Replace package-json and throat dependencies with NpmRegistryClient for fetching npm registry metadata. Preserves existing version sorting and homepage extraction logic. Changes: - Remove package-json and throat imports - Add lazy-initialized module-level NpmRegistryClient instance - Update getNpmInfo to use fetchPackageMetadataAsync - Preserve lodash/semver version sorting - Preserve bestGuessHomepage extraction
…hing Add batch fetching function to retrieve metadata for multiple packages concurrently with configurable concurrency limit. Features: - getNpmInfoBatch(packageNames, concurrency) returns Map<string, INpmRegistryInfo> - Default concurrency matches CPU count (like original throat behavior) - Processes packages in batches using Promise.all
Remove external dependencies that have been replaced by NpmRegistryClient: - package-json: replaced by NpmRegistryClient - throat: replaced by Promise.all batch processing Update tests to mock NpmRegistryClient instead of package-json.
Add basic unit tests for NpmRegistryClient constructor options. Update GetLatestFromRegistry tests with improved module mocking.
Document the removal of package-json and throat dependencies in favor of internal NpmRegistryClient implementation.
TheLarkInn
commented
Jan 24, 2026
TheLarkInn
commented
Jan 24, 2026
TheLarkInn
requested review from
D4N14L,
apostolisms,
dmichon-msft,
iclanton,
octogonz and
patmill
as code owners
TheLarkInn
requested review from
Copilot
and removed request for
D4N14L,
apostolisms,
dmichon-msft,
iclanton,
octogonz and
patmill
There was a problem hiding this comment.
Pull request overview
Removes the external package-json and throat dependencies from @rushstack/npm-check-fork by introducing a lightweight npm registry client and updating registry lookup logic accordingly.
Changes:
- Added
NpmRegistryClientimplemented with Node.jshttp/https(plus a basic test file). - Refactored
GetLatestFromRegistryto useNpmRegistryClientand introducedgetNpmInfoBatchwith configurable concurrency. - Updated types/exports and removed the external dependencies from
package.jsonand lockfiles.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| libraries/npm-check-fork/src/NpmRegistryClient.ts | New minimal registry client implemented via Node http/https. |
| libraries/npm-check-fork/src/GetLatestFromRegistry.ts | Swaps registry fetch to NpmRegistryClient and adds batched lookup helper. |
| libraries/npm-check-fork/src/interfaces/INpmCheckRegistry.ts | Adds registry response types used by the new client/refactor. |
| libraries/npm-check-fork/src/index.ts | Exports the new client/types and getNpmInfoBatch. |
| libraries/npm-check-fork/src/tests/GetLatestFromRegistry.test.ts | Updates tests to mock NpmRegistryClient instead of package-json. |
| libraries/npm-check-fork/src/tests/NpmRegistryClient.test.ts | Adds initial (currently minimal) test scaffolding for the new client. |
| libraries/npm-check-fork/package.json | Removes package-json/throat, adds @rushstack/node-core-library. |
| libraries/npm-check-fork/CHANGELOG.md | Adds a new version entry describing the dependency replacement. |
| common/changes/@rushstack/npm-check-fork/atomic-style-claude_2026-01-24-18-36.json | Changeset for the patch release. |
| common/config/subspaces/**/pnpm-lock.yaml, repo-state.json | Lockfile/hash updates reflecting dependency changes. |
Files not reviewed (2)
- common/config/subspaces/build-tests-subspace/pnpm-lock.yaml: Language not supported
- common/config/subspaces/default/pnpm-lock.yaml: Language not supported
Contributor
|
@TheLarkInn I've opened a new pull request, #5566, to work on those changes. Once the pull request is ready, I'll request review from you. |
Contributor
|
@TheLarkInn I've opened a new pull request, #5567, to work on those changes. Once the pull request is ready, I'll request review from you. |
This was referenced Jan 24, 2026
Contributor
|
@TheLarkInn I've opened a new pull request, #5568, to work on those changes. Once the pull request is ready, I'll request review from you. |
…esn't (#5567) * Initial plan * docs: reword JSDoc for structural compatibility instead of extends Co-authored-by: TheLarkInn <3408176+TheLarkInn@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TheLarkInn <3408176+TheLarkInn@users.noreply.github.com>
…5568) * Initial plan * test(npm-check-fork): add comprehensive unit tests for NpmRegistryClient with http mocking Co-authored-by: TheLarkInn <3408176+TheLarkInn@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TheLarkInn <3408176+TheLarkInn@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Removes external dependencies
package-json(ESM-only, ~500KB dependency tree) andthroat(redundant concurrency limiter) from@rushstack/npm-check-fork.Replaced with:
NpmRegistryClient- minimal registry client using Node.js built-inhttpsmodulePromise.allbatch processing for concurrent requestsDetails
New files:
src/NpmRegistryClient.ts- registry client withfetchPackageMetadataAsyncsrc/tests/NpmRegistryClient.test.tsModified files:
src/GetLatestFromRegistry.ts- uses NpmRegistryClient instead of package-jsonsrc/interfaces/INpmCheckRegistry.ts- added registry response typessrc/index.ts- exports new client andgetNpmInfoBatchfunctionInternal refactoring only.
How it was tested
rush build --to @rushstack/npm-check-fork- passesrush test --only @rushstack/npm-check-fork- passesrush upgrade-interactiveImpacted documentation
None - internal implementation change only.