fix(npm): resolve cached package name from install dir for non-registry specs#24821
Open
sjawhar wants to merge 1 commit intoanomalyco:devfrom
Open
fix(npm): resolve cached package name from install dir for non-registry specs#24821sjawhar wants to merge 1 commit intoanomalyco:devfrom
sjawhar wants to merge 1 commit intoanomalyco:devfrom
Conversation
…ry specs
Npm.add cache fast-path used npa(pkg).name as the package name. For
non-registry specs (remote tarball URLs, git+https://, github: shorthand,
file: paths), npa returns name=undefined and the code fell back to the
spec string itself, then looked at <dir>/node_modules/<spec>/ which does
not exist. Plugins/packages installed from these sources fail to load on
the second and subsequent runs, even though the fresh-install path works
correctly.
The fresh-install path uses tree.edgesOut.values().next().value.to.{name,
path} from Arborist. For the cache fast-path, derive the actual installed
package name from <dir>/package.json's first dependency entry (the same
source Npm.install reads to detect dirty installs).
Fallback chain: package.json first dep -> npa(pkg).name -> spec string.
github-actions
Bot
added
needs:compliance
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
github-actions
Bot
removed
the
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue for this PR
Closes #24828
Type of change
What does this PR do?
Fixes the
Npm.addcache fast-path so it resolves non-registry specs to their actual installed directory instead of a path derived from the spec string.The cache fast-path was reading the package name from
npa(pkg).name. That works for registry specs like@scope/pkg@1.0.0, but forhttps://,git+https://,github:owner/repo, andfile:specsnpa(pkg).namereturnsundefined, sonamefalls back to the full spec string and the resulting<dir>/node_modules/<spec>/path doesn't exist.The fix: read the install-root
package.json(which Arborist writes during the fresh-install path) and recover the actual name from its firstdependenciesentry. This is the same sourceNpm.installalready reads on lines 247-275 to detect dirty installs.Fallback chain: install-root
package.jsonfirst dep →npa(pkg).name→ spec string. The third level preserves backward compat for the unlikely case wherepackage.jsonis missing or has no deps.The fresh-install path is unchanged — it's correct and continues to use
tree.edgesOut.How did you verify your code works?
Added 5 tests in
packages/core/test/npm.test.tscovering each spec type. Each test seeds a synthetic populated cache (mimicking a previous successful install) and assertsNpm.add(spec).directoryresolves through the install dir to the actual package, not the broken spec-string path.Without the fix, the four non-registry tests fail. With the fix, all five pass.
End-to-end tested against a production opencode setup that uses a private GitHub release tarball as a plugin: previously the cache fast-path returned a non-existent path on every run after the first, now it resolves correctly.
Screenshots / recordings
N/A — no UI changes.
Checklist