feat!: dlx should use exact versions of packages in the cache key (#8811)

close #8722

.changeset/proud-zoos-grow.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-script-runners": major
+"pnpm": major
+---
+
+The `dlx` command should always resolve packages to their exact versions and use those exact versions to create a cache key. This way `dlx` will always install the newest versions of the directly requested packages.

exec/plugin-commands-script-runners/package.json

@@ -45,6 +45,7 @@
     "write-yaml-file": "catalog:"
   },
   "dependencies": {
+    "@pnpm/client": "workspace:*",
     "@pnpm/cli-utils": "workspace:*",
     "@pnpm/command": "workspace:*",
     "@pnpm/common-cli-options-help": "workspace:*",
@@ -57,6 +58,8 @@
     "@pnpm/lifecycle": "workspace:*",
     "@pnpm/log.group": "catalog:",
     "@pnpm/package-bins": "workspace:*",
+    "@pnpm/parse-wanted-dependency": "workspace:*",
+    "@pnpm/pick-registry-for-package": "workspace:*",
     "@pnpm/plugin-commands-env": "workspace:*",
     "@pnpm/plugin-commands-installation": "workspace:*",
     "@pnpm/read-package-json": "workspace:*",

exec/plugin-commands-script-runners/src/dlx.ts

@@ -2,6 +2,8 @@ import fs, { type Stats } from 'fs'
 import path from 'path'
 import util from 'util'
 import { docsUrl } from '@pnpm/cli-utils'
+import { createResolver } from '@pnpm/client'
+import { parseWantedDependency } from '@pnpm/parse-wanted-dependency'
 import { OUTPUT_OPTIONS } from '@pnpm/common-cli-options-help'
 import { type Config, types } from '@pnpm/config'
 import { createHexHash } from '@pnpm/crypto.hash'
@@ -9,6 +11,7 @@ import { PnpmError } from '@pnpm/error'
 import { add } from '@pnpm/plugin-commands-installation'
 import { readPackageJsonFromDir } from '@pnpm/read-package-json'
 import { getBinsFromPackageManifest } from '@pnpm/package-bins'
+import { pickRegistryForPackage } from '@pnpm/pick-registry-for-package'
 import execa from 'execa'
 import omit from 'ramda/src/omit'
 import pick from 'ramda/src/pick'
@@ -73,7 +76,22 @@ export async function handler (
   [command, ...args]: string[]
 ): Promise<{ exitCode: number }> {
   const pkgs = opts.package ?? [command]
-  const { cacheLink, cacheExists, cachedDir } = findCache(pkgs, {
+  const { resolve } = createResolver({
+    ...opts,
+    authConfig: opts.rawConfig,
+  })
+  const resolvedPkgs = await Promise.all(pkgs.map(async (pkg) => {
+    const { alias, pref } = parseWantedDependency(pkg) || {}
+    if (alias == null) return pkg
+    const resolved = await resolve({ alias, pref }, {
+      lockfileDir: opts.lockfileDir ?? opts.dir,
+      preferredVersions: {},
+      projectDir: opts.dir,
+      registry: pickRegistryForPackage(opts.registries, alias, pref),
+    })
+    return resolved.id
+  }))
+  const { cacheLink, cacheExists, cachedDir } = findCache(resolvedPkgs, {
     dlxCacheMaxAge: opts.dlxCacheMaxAge,
     cacheDir: opts.cacheDir,
     registries: opts.registries,
@@ -92,7 +110,7 @@ export async function handler (
       saveDev: false,
       saveOptional: false,
       savePeer: false,
-    }, pkgs)
+    }, resolvedPkgs)
     try {
       await symlinkDir(cachedDir, cacheLink, { overwrite: true })
     } catch (error) {

exec/plugin-commands-script-runners/test/dlx.e2e.ts

@@ -213,7 +213,7 @@ test('dlx with cache', async () => {
   }, ['shx', 'touch', 'foo'])
 
   expect(fs.existsSync('foo')).toBe(true)
-  verifyDlxCache(createCacheKey('shx'))
+  verifyDlxCache(createCacheKey('shx@0.3.4'))
   expect(spy).toHaveBeenCalled()
 
   spy.mockReset()
@@ -227,7 +227,7 @@ test('dlx with cache', async () => {
   }, ['shx', 'touch', 'bar'])
 
   expect(fs.existsSync('bar')).toBe(true)
-  verifyDlxCache(createCacheKey('shx'))
+  verifyDlxCache(createCacheKey('shx@0.3.4'))
   expect(spy).not.toHaveBeenCalled()
 
   spy.mockRestore()
@@ -246,11 +246,11 @@ test('dlx does not reuse expired cache', async () => {
     cacheDir: path.resolve('cache'),
     dlxCacheMaxAge: Infinity,
   }, ['shx', 'echo', 'hello world'])
-  verifyDlxCache(createCacheKey('shx'))
+  verifyDlxCache(createCacheKey('shx@0.3.4'))
 
   // change the date attributes of the cache to 30 minutes older than now
   const newDate = new Date(now.getTime() - 30 * 60_000)
-  fs.lutimesSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg'), newDate, newDate)
+  fs.lutimesSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg'), newDate, newDate)
 
   const spy = jest.spyOn(add, 'handler')
 
@@ -264,12 +264,12 @@ test('dlx does not reuse expired cache', async () => {
   }, ['shx', 'touch', 'BAR'])
 
   expect(fs.existsSync('BAR')).toBe(true)
-  expect(spy).toHaveBeenCalledWith(expect.anything(), ['shx'])
+  expect(spy).toHaveBeenCalledWith(expect.anything(), ['shx@0.3.4'])
 
   spy.mockRestore()
 
   expect(
-    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx')))
+    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4')))
       .map(sanitizeDlxCacheComponent)
       .sort()
   ).toStrictEqual([
@@ -277,7 +277,7 @@ test('dlx does not reuse expired cache', async () => {
     '***********-*****',
     '***********-*****',
   ].sort())
-  verifyDlxCacheLink(createCacheKey('shx'))
+  verifyDlxCacheLink(createCacheKey('shx@0.3.4'))
 })
 
 test('dlx still saves cache even if execution fails', async () => {
@@ -294,5 +294,5 @@ test('dlx still saves cache even if execution fails', async () => {
   }, ['shx', 'mkdir', path.resolve('not-a-dir')])
 
   expect(fs.readFileSync(path.resolve('not-a-dir'), 'utf-8')).toEqual(expect.anything())
-  verifyDlxCache(createCacheKey('shx'))
+  verifyDlxCache(createCacheKey('shx@0.3.4'))
 })

exec/plugin-commands-script-runners/tsconfig.json

@@ -27,6 +27,9 @@
     {
       "path": "../../config/config"
     },
+    {
+      "path": "../../config/pick-registry-for-package"
+    },
     {
       "path": "../../crypto/hash"
     },
@@ -48,9 +51,15 @@
     {
       "path": "../../packages/logger"
     },
+    {
+      "path": "../../packages/parse-wanted-dependency"
+    },
     {
       "path": "../../packages/types"
     },
+    {
+      "path": "../../pkg-manager/client"
+    },
     {
       "path": "../../pkg-manager/package-bins"
     },

pnpm-lock.yaml

@@ -2338,6 +2338,9 @@ importers:
       '@pnpm/cli-utils':
         specifier: workspace:*
         version: link:../../cli/cli-utils
+      '@pnpm/client':
+        specifier: workspace:*
+        version: link:../../pkg-manager/client
       '@pnpm/command':
         specifier: workspace:*
         version: link:../../cli/command
@@ -2371,6 +2374,12 @@ importers:
       '@pnpm/package-bins':
         specifier: workspace:*
         version: link:../../pkg-manager/package-bins
+      '@pnpm/parse-wanted-dependency':
+        specifier: workspace:*
+        version: link:../../packages/parse-wanted-dependency
+      '@pnpm/pick-registry-for-package':
+        specifier: workspace:*
+        version: link:../../config/pick-registry-for-package
       '@pnpm/plugin-commands-env':
         specifier: workspace:*
         version: link:../../env/plugin-commands-env

pnpm/test/dlx.ts

@@ -1,13 +1,22 @@
 import fs from 'fs'
 import path from 'path'
 import PATH_NAME from 'path-name'
+import { getConfig } from '@pnpm/config'
 import { prepare, prepareEmpty } from '@pnpm/prepare'
 import { readModulesManifest } from '@pnpm/modules-yaml'
 import { addUser, REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
 import { dlx } from '@pnpm/plugin-commands-script-runners'
-import { execPnpm, execPnpmSync, testDefaults } from './utils'
+import { execPnpm, execPnpmSync } from './utils'
 
-const createCacheKey = (...pkgs: string[]): string => dlx.createCacheKey(pkgs, { default: testDefaults({}).registry })
+let registries: Record<string, string>
+
+beforeAll(async () => {
+  const { config } = await getConfig({ cliOptions: {}, packageManager: { name: '', version: '' } })
+  registries = config.registries
+  registries.default = `http://localhost:${REGISTRY_MOCK_PORT}/`
+})
+
+const createCacheKey = (...pkgs: string[]): string => dlx.createCacheKey(pkgs, registries)
 
 test('silent dlx prints the output of the child process only', async () => {
   prepare({})
@@ -74,17 +83,17 @@ test('parallel dlx calls of the same package', async () => {
 
   expect(['foo', 'bar', 'baz'].filter(name => fs.existsSync(name))).toStrictEqual(['foo', 'bar', 'baz'])
   expect(
-    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg'))
+    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg'))
   ).toStrictEqual([
     'node_modules',
     'package.json',
     'pnpm-lock.yaml',
   ])
   expect(
-    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg')))
-  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx')))
+    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg')))
+  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4')))
 
-  const cacheContentAfterFirstRun = fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'))).sort()
+  const cacheContentAfterFirstRun = fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'))).sort()
 
   // parallel dlx calls with cache
   await Promise.all(['abc', 'def', 'ghi'].map(
@@ -92,17 +101,17 @@ test('parallel dlx calls of the same package', async () => {
   ))
 
   expect(['abc', 'def', 'ghi'].filter(name => fs.existsSync(name))).toStrictEqual(['abc', 'def', 'ghi'])
-  expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'))).sort()).toStrictEqual(cacheContentAfterFirstRun)
+  expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'))).sort()).toStrictEqual(cacheContentAfterFirstRun)
   expect(
-    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg'))
+    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg'))
   ).toStrictEqual([
     'node_modules',
     'package.json',
     'pnpm-lock.yaml',
   ])
   expect(
-    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg')))
-  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx')))
+    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg')))
+  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4')))
 
   // parallel dlx calls with expired cache
   await Promise.all(['a/b/c', 'd/e/f', 'g/h/i'].map(
@@ -114,17 +123,17 @@ test('parallel dlx calls of the same package', async () => {
   ))
 
   expect(['a/b/c', 'd/e/f', 'g/h/i'].filter(name => fs.existsSync(name))).toStrictEqual(['a/b/c', 'd/e/f', 'g/h/i'])
-  expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'))).length).toBeGreaterThan(cacheContentAfterFirstRun.length)
+  expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'))).length).toBeGreaterThan(cacheContentAfterFirstRun.length)
   expect(
-    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg'))
+    fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg'))
   ).toStrictEqual([
     'node_modules',
     'package.json',
     'pnpm-lock.yaml',
   ])
   expect(
-    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx'), 'pkg')))
-  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx')))
+    path.dirname(fs.realpathSync(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4'), 'pkg')))
+  ).toBe(path.resolve('cache', 'dlx', createCacheKey('shx@0.3.4')))
 })
 
 test('dlx creates cache and store prune cleans cache', async () => {
@@ -146,45 +155,36 @@ test('dlx creates cache and store prune cleans cache', async () => {
   await Promise.all(Object.entries(commands).map(([cmd, args]) => execPnpm([...settings, 'dlx', cmd, ...args])))
 
   // ensure that the dlx cache has certain structure
-  expect(
-    fs.readdirSync(path.resolve('cache', 'dlx'))
-      .sort()
-  ).toStrictEqual(
-    Object.keys(commands)
-      .map(cmd => createCacheKey(cmd))
-      .sort()
-  )
-  for (const cmd of Object.keys(commands)) {
-    expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey(cmd))).length).toBe(2)
+  const dlxBaseDir = path.resolve('cache', 'dlx')
+  const dlxDirs = fs.readdirSync(dlxBaseDir)
+  expect(dlxDirs.length).toEqual(Object.keys(commands).length)
+  for (const dlxDir of dlxDirs) {
+    expect(fs.readdirSync(path.resolve(dlxBaseDir, dlxDir)).length).toBe(2)
   }
 
   // modify the dates of the cache items
   const ageTable = {
-    shx: 20,
-    'shelljs/shx#61aca968cd7afc712ca61a4fc4ec3201e3770dc7': 75,
-    '@pnpm.e2e/touch-file-good-bin-name': 33,
-    '@pnpm.e2e/touch-file-one-bin': 123,
-  } satisfies Record<keyof typeof commands, number>
+    [dlxDirs[0]]: 20,
+    [dlxDirs[1]]: 75,
+    [dlxDirs[2]]: 33,
+    [dlxDirs[3]]: 123,
+  } satisfies Record<string, number>
   const now = new Date()
-  await Promise.all(Object.entries(ageTable).map(async ([cmd, age]) => {
+  await Promise.all(Object.entries(ageTable).map(async ([dlxDir, age]) => {
     const newDate = new Date(now.getTime() - age * 60_000)
-    const dlxCacheLink = path.resolve('cache', 'dlx', createCacheKey(cmd), 'pkg')
+    const dlxCacheLink = path.resolve('cache', 'dlx', dlxDir, 'pkg')
     await fs.promises.lutimes(dlxCacheLink, newDate, newDate)
   }))
 
   await execPnpm([...settings, 'store', 'prune'])
 
   // test to see if dlx cache items are deleted or kept as expected
+  const keptDirs = [dlxDirs[0], dlxDirs[2]].sort()
   expect(
-    fs.readdirSync(path.resolve('cache', 'dlx'))
-      .sort()
-  ).toStrictEqual(
-    ['shx', '@pnpm.e2e/touch-file-good-bin-name']
-      .map(cmd => createCacheKey(cmd))
-      .sort()
-  )
-  for (const cmd of ['shx', '@pnpm.e2e/touch-file-good-bin-name']) {
-    expect(fs.readdirSync(path.resolve('cache', 'dlx', createCacheKey(cmd))).length).toBe(2)
+    fs.readdirSync(path.resolve('cache', 'dlx')).sort()
+  ).toStrictEqual(keptDirs)
+  for (const keptDir of keptDirs) {
+    expect(fs.readdirSync(path.resolve('cache', 'dlx', keptDir)).length).toBe(2)
   }
 
   await execPnpm([
@@ -210,7 +210,7 @@ test('dlx should ignore non-auth info from .npmrc in the current directory', asy
     `--config.cache-dir=${cacheDir}`,
     'dlx', 'shx', 'echo', 'hi'])
 
-  const modulesManifest = await readModulesManifest(path.join(cacheDir, 'dlx', createCacheKey('shx'), 'pkg/node_modules'))
+  const modulesManifest = await readModulesManifest(path.join(cacheDir, 'dlx', createCacheKey('shx@0.3.4'), 'pkg/node_modules'))
   expect(modulesManifest?.hoistPattern).toStrictEqual(['*'])
 })