Skip to content

Replace lru-cache with internal implementation - #343

Open
tommy-mitchell wants to merge 2 commits into
npm:mainfrom
tommy-mitchell:feat/cache
Open

tommy-mitchell wants to merge 2 commits into
npm:mainfrom
tommy-mitchell:feat/cache

Conversation

@tommy-mitchell

@tommy-mitchell tommy-mitchell commented Oct 4, 2026 •

Copy link
Copy Markdown

Replaces the lru-cache dependency with a minimal internal implementation using Map based on node-semver's implementation (see npm/node-semver#704). Only the functions used in the cache have been implemented.

Tests have been added that reflect actual usage (see npm/node-semver#697 (comment)).

Motivation

hosted-git-info has an install size of over 3MB due to the lru-cache dependency:

$ npx pkg-size install hosted-git-info

 3.4 MB  2 Packages

 3.3 MB  lru-cache       v11.5.3
28.4 kB  hosted-git-info v10.1.1

@tommy-mitchell
tommy-mitchell requested a review from a team as a code owner October 4, 2026 05:15
@tommy-mitchell tommy-mitchell changed the title deps: remove lru-cache Replace lru-cache with internal implementation Oct 4, 2026
@jeffwidman

Copy link
Copy Markdown
Member

I opened #347 as an independent draft for an existing cache behavior worth preserving here.

fromUrl() tries to cache unsuccessful parses as undefined, but neither the current lru-cache nor this PR's proposed set() retains that value. Ordinary registry version specifications can therefore repeatedly go through hosted-Git parsing during npm operations. This is a pre-existing issue, not a regression introduced by this PR.

The draft uses an internal null sentinel while preserving the public undefined return and existing 1,000-entry bound. It also invalidates cached results when addHost() changes which URLs are recognized.

  • A public reproduction with 20,000 lookups of 100 distinct non-Git inputs reduces parser executions from 20,000 to 100.
  • With only that fix applied to npm 11.17.0, local Dependabot-shaped commands showed 6.7% lower writer CPU and 4.9% lower checker CPU, with byte-identical manifests/lockfiles. These are workload-specific native-command measurements, not complete Dependabot-job savings.

If this PR moves forward, it would be useful to incorporate that behavior too. The proposed Map implementation can retain the null sentinel; it would also need clear() or equivalent invalidation for host registration. The regression tests cover negative-hit recency, bounded eviction/reparsing alongside successful results, the unchanged return value, and newly registered hosts.

The fix is independent of whether the external cache dependency is retained or replaced, so the two changes should be straightforward to coordinate.

@tommy-mitchell

Copy link
Copy Markdown
Author

If this PR moves forward, it would be useful to incorporate that behavior too. The proposed Map implementation can retain the null sentinel; it would also need clear() or equivalent invalidation for host registration.

I'd be happy to either modify this PR or put up a follow-up PR depending on which goes in first :)

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.

2 participants