Skip to content

Commit

Permalink
feat: the file protocol should always inject the dependency (#4408)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zkochan
zkochan committed Mar 5, 2022
1 parent ef84f47 commit 9c22c06
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 11 deletions.
7 changes: 7 additions & 0 deletions .changeset/green-trainers-sit.md
View file
@@ -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.
298 changes: 298 additions & 0 deletions packages/core/test/install/injectLocalPackages.ts
View file
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/install/local.ts
View file
Expand Up @@ -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)
Expand All @@ -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')

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/install/multipleImporters.ts
View file
Expand Up @@ -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 = {
Expand Down
2 changes: 1 addition & 1 deletion packages/local-resolver/src/index.ts
View file
Expand Up @@ -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.`)
}
Expand Down
10 changes: 9 additions & 1 deletion packages/local-resolver/src/parsePref.ts
View file
Expand Up @@ -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)) {
Expand All @@ -78,6 +85,7 @@ function fromLocal (
}
}

injected = protocol === 'file:'
const dependencyPath = injected
? normalize(path.relative(lockfileDir, fetchSpec))
: normalize(path.resolve(fetchSpec))
Expand Down

0 comments on commit 9c22c06

Please sign in to comment.