Resolve installed dependency versions from lockfiles instead of node_modules#14703
Resolve installed dependency versions from lockfiles instead of node_modules#14703dario-piotrowicz wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 9e5beec The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✅ All changesets look good |
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
|
@dario-piotrowicz Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
24edfc0 to
2f88b64
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
2f88b64 to
1d033b4
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
1d033b4 to
18f5c12
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
18f5c12 to
23ebae8
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…modules `getInstalledPackageVersion` now consults the nearest lockfile (pnpm-lock.yaml, package-lock.json, yarn.lock, or bun.lock) before falling back to resolving from node_modules. The lockfile is parsed once and memoized, making repeated version lookups significantly faster. This also improves the accuracy of the dependency metadata collected during deploys. All four major package managers (npm, pnpm, yarn, and bun) are supported across their lockfile format versions
50849a3 to
9e5beec
Compare
| const found = findLockfile(projectPath, { | ||
| last: opts.stopAtProjectPath === true ? projectPath : undefined, | ||
| }); |
There was a problem hiding this comment.
🟡 Dependency version lookups re-scan the folder tree on disk for every package
The nearest lockfile is located by scanning folders on disk (findLockfile at packages/workers-utils/src/package-resolution/lockfiles-resolution/resolve.ts:55) on every single package lookup before the memoization cache is even consulted, so the intended speed-up is undercut because the same directory scan is repeated once per dependency.
Impact: Collecting dependency metadata during a deploy does far more redundant filesystem work than intended, so projects with many dependencies see little of the promised performance benefit.
Discovery runs before the cache check
In getInstalledVersionsFromLockfile (packages/workers-utils/src/package-resolution/lockfiles-resolution/resolve.ts:55-77), findLockfile is invoked unconditionally at the top of the function. Only the parse result (parseLockfile) is memoized in cacheStore keyed by lockfilePath + "\0" + projectPath. findLockfile (discovery.ts:36-58) walks up the directory tree with walk.up and performs up to LOCKFILE_NAMES.length (4) statSync calls per directory level.
When called from the loop in collectPackageDependencies (packages/deploy-helpers/src/deploy/helpers/package-dependencies.ts:83-92), this runs once per dependency — e.g. ~130 full directory-walk + stat sweeps for ~130 deps — even though the discovered lockfile path is identical every time. The cache should also memoize the discovery step (e.g. keyed by projectPath + stopAtProjectPath) so the walk happens once per collection pass. This matches the PR author's own note that the observed speed-up was minimal.
Prompt for agents
In getInstalledVersionsFromLockfile (packages/workers-utils/src/package-resolution/lockfiles-resolution/resolve.ts), findLockfile is called unconditionally at the start of the function on every invocation, before the memoization cache is consulted. Only the parseLockfile result is cached. When called in a loop (once per dependency in collectPackageDependencies), this repeats the full directory-tree walk plus multiple statSync calls per dependency, even though the discovered lockfile is identical each time. This undercuts the stated performance goal of the PR. Consider also memoizing the discovery result — e.g. cache the findLockfile output keyed by projectPath (and the stopAtProjectPath flag), or short-circuit discovery when a cache entry already exists for the (projectPath) — so the directory walk runs once per collection pass rather than once per dependency.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
getInstalledPackageVersionnow consults the nearest lockfile (pnpm-lock.yaml, package-lock.json, yarn.lock, or bun.lock) before falling back to resolving from node_modules. The lockfile is parsed once and memoized, making repeated version lookups significantly faster. This also improves the accuracy of the dependency metadata collected during deploys. All four major package managers (npm, pnpm, yarn, and bun) are supported across their lockfile format versionsNote
I run some benchmarks on this PR's prerelease and I must say that the perf benefit seems to be quite minimal even when dealing with a good number of dependencies (~130). That however might be because of performance fs operations on my system, maybe the different could be more significative on older less powerful machines (also this definitely scaled batter, so the more the dependencies the more this approach speeds things up).
A picture of a cute animal (not mandatory, but encouraged)