feat: the file protocol should always inject the dependency (#4408)

.changeset/green-trainers-sit.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/local-resolver": major
+"@pnpm/package-requester": major
+"pnpm": major
+---
+
+Local dependencies referenced through the `file:` protocol are hard linked (not symlinked) [#4408](https://github.com/pnpm/pnpm/pull/4408). If you need to symlink a dependency, use the `link:` protocol instead.

packages/core/test/install/injectLocalPackages.ts

@@ -443,6 +443,204 @@ test('inject local packages declared via file protocol', async () => {
   }
 })
 
+test('inject local packages when the file protocol is used', async () => {
+  const project1Manifest = {
+    name: 'project-1',
+    version: '1.0.0',
+    dependencies: {
+      'is-negative': '1.0.0',
+    },
+    devDependencies: {
+      'dep-of-pkg-with-1-dep': '100.0.0',
+    },
+    peerDependencies: {
+      'is-positive': '>=1.0.0',
+    },
+  }
+  const project2Manifest = {
+    name: 'project-2',
+    version: '1.0.0',
+    dependencies: {
+      'project-1': 'file:../project-1',
+    },
+    devDependencies: {
+      'is-positive': '1.0.0',
+    },
+  }
+  const project3Manifest = {
+    name: 'project-3',
+    version: '1.0.0',
+    dependencies: {
+      'project-2': 'file:../project-2',
+    },
+    devDependencies: {
+      'is-positive': '2.0.0',
+    },
+  }
+  const projects = preparePackages([
+    {
+      location: 'project-1',
+      package: project1Manifest,
+    },
+    {
+      location: 'project-2',
+      package: project2Manifest,
+    },
+    {
+      location: 'project-3',
+      package: project3Manifest,
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      buildIndex: 0,
+      manifest: project1Manifest,
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: project2Manifest,
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+    {
+      buildIndex: 0,
+      manifest: project3Manifest,
+      mutation: 'install',
+      rootDir: path.resolve('project-3'),
+    },
+  ]
+  const workspacePackages = {
+    'project-1': {
+      '1.0.0': {
+        dir: path.resolve('project-1'),
+        manifest: project1Manifest,
+      },
+    },
+    'project-2': {
+      '1.0.0': {
+        dir: path.resolve('project-2'),
+        manifest: project2Manifest,
+      },
+    },
+    'project-3': {
+      '1.0.0': {
+        dir: path.resolve('project-3'),
+        manifest: project2Manifest,
+      },
+    },
+  }
+  await mutateModules(importers, await testDefaults({
+    workspacePackages,
+  }))
+
+  await projects['project-1'].has('is-negative')
+  await projects['project-1'].has('dep-of-pkg-with-1-dep')
+  await projects['project-1'].hasNot('is-positive')
+
+  await projects['project-2'].has('is-positive')
+  await projects['project-2'].has('project-1')
+
+  await projects['project-3'].has('is-positive')
+  await projects['project-3'].has('project-2')
+
+  expect(fs.readdirSync('node_modules/.pnpm').length).toBe(8)
+
+  const rootModules = assertProject(process.cwd())
+  {
+    const lockfile = await rootModules.readLockfile()
+    expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({
+      resolution: {
+        directory: 'project-1',
+        type: 'directory',
+      },
+      id: 'file:project-1',
+      name: 'project-1',
+      version: '1.0.0',
+      peerDependencies: {
+        'is-positive': '>=1.0.0',
+      },
+      dependencies: {
+        'is-negative': '1.0.0',
+        'is-positive': '1.0.0',
+      },
+      dev: false,
+    })
+    expect(lockfile.packages['file:project-2_is-positive@2.0.0']).toEqual({
+      resolution: {
+        directory: 'project-2',
+        type: 'directory',
+      },
+      id: 'file:project-2',
+      name: 'project-2',
+      version: '1.0.0',
+      dependencies: {
+        'project-1': 'file:project-1_is-positive@2.0.0',
+      },
+      transitivePeerDependencies: ['is-positive'],
+      dev: false,
+    })
+
+    const modulesState = await rootModules.readModulesManifest()
+    expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2)
+    expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`)
+    expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`)
+  }
+
+  await rimraf('node_modules')
+  await rimraf('project-1/node_modules')
+  await rimraf('project-2/node_modules')
+  await rimraf('project-3/node_modules')
+
+  await mutateModules(importers, await testDefaults({
+    frozenLockfile: true,
+    workspacePackages,
+  }))
+
+  await projects['project-1'].has('is-negative')
+  await projects['project-1'].has('dep-of-pkg-with-1-dep')
+  await projects['project-1'].hasNot('is-positive')
+
+  await projects['project-2'].has('is-positive')
+  await projects['project-2'].has('project-1')
+
+  await projects['project-3'].has('is-positive')
+  await projects['project-3'].has('project-2')
+
+  expect(fs.readdirSync('node_modules/.pnpm').length).toBe(8)
+
+  // The injected project is updated when one of its dependencies needs to be updated
+  importers[0].manifest.dependencies!['is-negative'] = '2.0.0'
+  writeJsonFile('project-1/package.json', importers[0].manifest)
+  await mutateModules(importers, await testDefaults({ workspacePackages }))
+  {
+    const lockfile = await rootModules.readLockfile()
+    expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({
+      resolution: {
+        directory: 'project-1',
+        type: 'directory',
+      },
+      id: 'file:project-1',
+      name: 'project-1',
+      version: '1.0.0',
+      peerDependencies: {
+        'is-positive': '>=1.0.0',
+      },
+      dependencies: {
+        'is-negative': '2.0.0',
+        'is-positive': '1.0.0',
+      },
+      dev: false,
+    })
+    const modulesState = await rootModules.readModulesManifest()
+    expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2)
+    expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`)
+    expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`)
+  }
+})
+
 test('inject local packages and relink them after build', async () => {
   const project1Manifest = {
     name: 'project-1',
@@ -570,6 +768,106 @@ test('inject local packages and relink them after build', async () => {
   expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy()
 })
 
+test('inject local packages and relink them after build (file protocol is used)', async () => {
+  const project1Manifest = {
+    name: 'project-1',
+    version: '1.0.0',
+    dependencies: {
+      'is-negative': '1.0.0',
+    },
+    devDependencies: {
+      'dep-of-pkg-with-1-dep': '100.0.0',
+    },
+    peerDependencies: {
+      'is-positive': '1.0.0',
+    },
+    scripts: {
+      prepublishOnly: 'touch main.js',
+    },
+  }
+  const project2Manifest = {
+    name: 'project-2',
+    version: '1.0.0',
+    dependencies: {
+      'is-positive': '1.0.0',
+      'project-1': 'file:../project-1',
+    },
+  }
+  const projects = preparePackages([
+    {
+      location: 'project-1',
+      package: project1Manifest,
+    },
+    {
+      location: 'project-2',
+      package: project2Manifest,
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      buildIndex: 0,
+      manifest: project1Manifest,
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: project2Manifest,
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+  await mutateModules(importers, await testDefaults())
+
+  await projects['project-1'].has('is-negative')
+  await projects['project-1'].has('dep-of-pkg-with-1-dep')
+  await projects['project-1'].hasNot('is-positive')
+
+  await projects['project-2'].has('is-positive')
+  await projects['project-2'].has('project-1')
+
+  expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy()
+
+  const rootModules = assertProject(process.cwd())
+  const lockfile = await rootModules.readLockfile()
+  expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({
+    resolution: {
+      directory: 'project-1',
+      type: 'directory',
+    },
+    id: 'file:project-1',
+    name: 'project-1',
+    version: '1.0.0',
+    peerDependencies: {
+      'is-positive': '1.0.0',
+    },
+    dependencies: {
+      'is-negative': '1.0.0',
+      'is-positive': '1.0.0',
+    },
+    dev: false,
+  })
+
+  await rimraf('node_modules')
+  await rimraf('project-1/main.js')
+  await rimraf('project-1/node_modules')
+  await rimraf('project-2/node_modules')
+
+  await mutateModules(importers, await testDefaults({
+    frozenLockfile: true,
+  }))
+
+  await projects['project-1'].has('is-negative')
+  await projects['project-1'].has('dep-of-pkg-with-1-dep')
+  await projects['project-1'].hasNot('is-positive')
+
+  await projects['project-2'].has('is-positive')
+  await projects['project-2'].has('project-1')
+
+  expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy()
+})
+
 test('inject local packages when node-linker is hoisted', async () => {
   const project1Manifest = {
     name: 'project-1',

packages/core/test/install/local.ts

@@ -29,7 +29,7 @@ test('local file', async () => {
   const project = prepareEmpty()
   f.copy('local-pkg', path.resolve('..', 'local-pkg'))
 
-  const manifest = await addDependenciesToPackage({}, ['file:../local-pkg'], await testDefaults())
+  const manifest = await addDependenciesToPackage({}, ['link:../local-pkg'], await testDefaults())
 
   const expectedSpecs = { 'local-pkg': `link:..${path.sep}local-pkg` }
   expect(manifest.dependencies).toStrictEqual(expectedSpecs)
@@ -56,7 +56,7 @@ test('local directory with no package.json', async () => {
 
   const manifest = await addDependenciesToPackage({}, ['file:./pkg'], await testDefaults())
 
-  const expectedSpecs = { pkg: 'link:pkg' }
+  const expectedSpecs = { pkg: 'file:pkg' }
   expect(manifest.dependencies).toStrictEqual(expectedSpecs)
   await project.has('pkg')
 
@@ -96,7 +96,7 @@ test('local file with symlinked node_modules', async () => {
   await fs.mkdir(path.join('..', 'node_modules'))
   await symlinkDir(path.join('..', 'node_modules'), 'node_modules')
 
-  const manifest = await addDependenciesToPackage({}, ['file:../local-pkg'], await testDefaults())
+  const manifest = await addDependenciesToPackage({}, ['link:../local-pkg'], await testDefaults())
 
   const expectedSpecs = { 'local-pkg': `link:..${path.sep}local-pkg` }
   expect(manifest.dependencies).toStrictEqual(expectedSpecs)

packages/core/test/install/multipleImporters.ts

@@ -359,7 +359,7 @@ test('headless install is used when package linked to another package in the wor
 
     dependencies: {
       'is-positive': '1.0.0',
-      'project-2': 'file:../project-2',
+      'project-2': 'link:../project-2',
     },
   }
   const pkg2 = {

packages/local-resolver/src/index.ts

@@ -55,7 +55,7 @@ export default async function resolveLocal (
     localDependencyManifest = await readProjectManifestOnly(spec.fetchSpec) as DependencyManifest
   } catch (internalErr: any) { // eslint-disable-line
     if (!existsSync(spec.fetchSpec)) {
-      if (wantedDependency.injected) {
+      if (spec.id.startsWith('file:')) {
         throw new PnpmError('LINKED_PKG_DIR_NOT_FOUND',
           `Could not install from "${spec.fetchSpec}" as it does not exist.`)
       }

packages/local-resolver/src/parsePref.ts

@@ -62,7 +62,14 @@ function fromLocal (
     .replace(/^(file|link|workspace):[/]*([A-Za-z]:)/, '$2') // drive name paths on windows
     .replace(/^(file|link|workspace):(?:[/]*([~./]))?/, '$2')
 
-  const protocol = type === 'directory' && !injected ? 'link:' : 'file:'
+  let protocol!: string
+  if (pref.startsWith('file:')) {
+    protocol = 'file:'
+  } else if (pref.startsWith('link:')) {
+    protocol = 'link:'
+  } else {
+    protocol = type === 'directory' && !injected ? 'link:' : 'file:'
+  }
   let fetchSpec!: string
   let normalizedPref!: string
   if (/^~[/]/.test(spec)) {
@@ -78,6 +85,7 @@ function fromLocal (
     }
   }
 
+  injected = protocol === 'file:'
   const dependencyPath = injected
     ? normalize(path.relative(lockfileDir, fetchSpec))
     : normalize(path.resolve(fetchSpec))

packages/local-resolver/test/index.ts

@@ -32,10 +32,10 @@ test('resolve workspace directory', async () => {
 
 test('resolve directory specified using the file: protocol', async () => {
   const resolveResult = await resolveFromLocal({ pref: 'file:..' }, { projectDir: __dirname })
-  expect(resolveResult!.id).toEqual('link:..')
-  expect(resolveResult!.normalizedPref).toEqual('link:..')
+  expect(resolveResult!.id).toEqual('file:..')
+  expect(resolveResult!.normalizedPref).toEqual('file:..')
   expect(resolveResult!['manifest']!.name).toEqual('@pnpm/local-resolver')
-  expect(resolveResult!.resolution['directory']).toEqual(normalize(path.join(__dirname, '..')))
+  expect(resolveResult!.resolution['directory']).toEqual('..')
   expect(resolveResult!.resolution['type']).toEqual('directory')
 })
 
@@ -108,7 +108,7 @@ test('fail when resolving tarball specified with the link: protocol', async () =
 })
 
 test('fail when resolving from not existing directory an injected dependency', async () => {
-  const wantedDependency = { injected: true, pref: 'link:./dir-does-not-exist' }
+  const wantedDependency = { pref: 'file:./dir-does-not-exist' }
   const projectDir = __dirname
   await expect(
     resolveFromLocal(wantedDependency, { projectDir })

packages/package-requester/src/packageRequester.ts

@@ -195,7 +195,7 @@ async function resolveAndFetch (
 
   const id = pkgId as string
 
-  if (resolution.type === 'directory' && !wantedDependency.injected) {
+  if (resolution.type === 'directory' && !id.startsWith('file:')) {
     if (manifest == null) {
       throw new Error(`Couldn't read package.json of local dependency ${wantedDependency.alias ? wantedDependency.alias + '@' : ''}${wantedDependency.pref ?? ''}`)
     }