-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add npm cache npx
command and fix stale-version reuse in npx
#8100
base: latest
Are you sure you want to change the base?
Conversation
npm cache npx
command and fix stale-version reuse in npxnpm cache npx
command and fix stale-version reuse in npx
We are going to be rebase/merging this so I have renamed this PR to NOT be a conventional message, that way our linter will warn us about commits that don't pass conventional commit linting. |
@@ -53,7 +55,8 @@ const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree, shallow }) | |||
return { node } | |||
} | |||
// package requested by version range, only remaining registry type | |||
if (semver.satisfies(node.package.version, spec.rawSpec)) { | |||
// the npx tree shouldn't be ok w/ an outdated version | |||
if (!isNpxTree && semver.satisfies(node.package.version, spec.rawSpec)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wraithgar I'm going to need help getting to this codepath in the unit tests, haven't been able to pull the right strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a version of the package that is in the npx cache that is older than the latest version as specified by the requested range.
So: [email protected]
in the cache, npx pkg@1
is requested, [email protected]
exists. It should prompt/update to 1.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wraithgar Hmm. Do I have my setup wrong?
What I'm seeing is that this is called first in the exec (missingFromTree not called with npx)
I'm trying to use
const { fixtures, package } = createPkg({
localVersion: '1.0.0',
versions: ['1.0.0', '1.0.1'],
name: '@npmcli/create-index',
})
...
await exec({
args: ['@npmcli/create-index@^1.0.0'],
})
But the exec first calls the misingFromTree
function without isNpxTree
const { manifest, node } = await missingFromTree({ spec, tree: localTree, flatOptions })
if (manifest) {
// Package does not exist in the local tree
needInstall.push({ spec, manifest })
...
}
which means this is satisfied (the first run doesn't have isNpxTree set and the semver tecnically satisfies, so no manifest
returned)
if (!isNpxTree && semver.satisfies(node.package.version, spec.rawSpec)) {
return { node }
}
So the exec
command can never get in to the branch that calls missingFromTree
with the isNpxTree: true
because the needInstall
never gets the push
.
if (needInstall.length > 0) {
...
await Promise.all(needInstall.map(async ({ spec }) => {
const { manifest } = await missingFromTree({
spec,
tree: npxTree,
flatOptions,
isNpxTree: true,
})
...
}
a7f9fd6
to
ecf3c92
Compare
#7838
References