Skip to content
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

feat: strict-peer-dependencies is true by default #4427

Merged
merged 5 commits into from Mar 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/lazy-falcons-tap.md
@@ -0,0 +1,6 @@
---
"@pnpm/config": major
"@pnpm/core": major
---

`strict-peer-dependencies` is `true` by default.
2 changes: 1 addition & 1 deletion packages/config/src/index.ts
Expand Up @@ -208,7 +208,7 @@ export default async (
shrinkwrap: npmDefaults.shrinkwrap,
reverse: false,
sort: true,
'strict-peer-dependencies': false,
'strict-peer-dependencies': true,
'unsafe-perm': npmDefaults['unsafe-perm'],
'use-beta-cli': false,
userconfig: npmDefaults.userconfig,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/install/extendInstallOptions.ts
Expand Up @@ -154,7 +154,7 @@ const defaults = async (opts: InstallOptions) => {
symlink: true,
storeController: opts.storeController,
storeDir: opts.storeDir,
strictPeerDependencies: false,
strictPeerDependencies: true,
tag: 'latest',
unsafePerm: process.platform === 'win32' ||
process.platform === 'cygwin' ||
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/install/misc.ts
Expand Up @@ -1177,7 +1177,7 @@ test('memory consumption is under control on huge package with many peer depende
version: '0.0.0',
},
['@teambit/bit@0.0.30'],
await testDefaults({ fastUnpack: true, lockfileOnly: true })
await testDefaults({ fastUnpack: true, lockfileOnly: true, strictPeerDependencies: false })
)

expect(await exists('pnpm-lock.yaml')).toBeTruthy()
Expand All @@ -1193,7 +1193,7 @@ test('memory consumption is under control on huge package with many peer depende
version: '0.0.0',
},
['@teambit/react@0.0.30'],
await testDefaults({ fastUnpack: true, lockfileOnly: true })
await testDefaults({ fastUnpack: true, lockfileOnly: true, strictPeerDependencies: false })
)

expect(await exists('pnpm-lock.yaml')).toBeTruthy()
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/install/multipleImporters.ts
Expand Up @@ -1150,7 +1150,7 @@ test('resolve a subdependency from the workspace and use it as a peer', async ()
},
},
}
await mutateModules(importers, await testDefaults({ linkWorkspacePackagesDepth: Infinity, workspacePackages }))
await mutateModules(importers, await testDefaults({ linkWorkspacePackagesDepth: Infinity, strictPeerDependencies: false, workspacePackages }))

const project = assertProject(process.cwd())

Expand Down
46 changes: 24 additions & 22 deletions packages/core/test/install/peerDependencies.ts
Expand Up @@ -56,6 +56,7 @@ test('nothing is needlessly removed from node_modules', async () => {
prepareEmpty()
const opts = await testDefaults({
modulesCacheMaxAge: 0,
strictPeerDependencies: false,
})
const manifest = await addDependenciesToPackage({}, ['using-ajv', 'ajv-keywords@1.5.0'], opts)

Expand Down Expand Up @@ -143,7 +144,7 @@ test('the right peer dependency is used in every workspace package', async () =>
rootDir: path.resolve('project-2'),
},
]
await mutateModules(importers, await testDefaults({ lockfileOnly: true }))
await mutateModules(importers, await testDefaults({ lockfileOnly: true, strictPeerDependencies: false }))

const lockfile = await readYamlFile<Lockfile>(path.resolve(WANTED_LOCKFILE))

Expand All @@ -161,7 +162,7 @@ test('warning is reported when cannot resolve peer dependency for top-level depe

const reporter = jest.fn()

await addDependenciesToPackage({}, ['ajv-keywords@1.5.0'], await testDefaults({ reporter }))
await addDependenciesToPackage({}, ['ajv-keywords@1.5.0'], await testDefaults({ reporter, strictPeerDependencies: false }))

expect(reporter).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -315,7 +316,7 @@ test('warning is reported when cannot resolve peer dependency for non-top-level

const reporter = jest.fn()

await addDependenciesToPackage({}, ['abc-grand-parent-without-c'], await testDefaults({ reporter }))
await addDependenciesToPackage({}, ['abc-grand-parent-without-c'], await testDefaults({ reporter, strictPeerDependencies: false }))

expect(reporter).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -360,7 +361,7 @@ test('warning is reported when bad version of resolved peer dependency for non-t

const reporter = jest.fn()

await addDependenciesToPackage({}, ['abc-grand-parent-without-c', 'peer-c@2'], await testDefaults({ reporter }))
await addDependenciesToPackage({}, ['abc-grand-parent-without-c', 'peer-c@2'], await testDefaults({ reporter, strictPeerDependencies: false }))

expect(reporter).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -457,9 +458,9 @@ test('top peer dependency is linked on subsequent install', async () => {
test('top peer dependency is linked on subsequent install, through transitive peer', async () => {
prepareEmpty()

const manifest = await addDependenciesToPackage({}, ['abc-grand-parent@1.0.0'], await testDefaults())
const manifest = await addDependenciesToPackage({}, ['abc-grand-parent@1.0.0'], await testDefaults({ strictPeerDependencies: false }))

await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults())
await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults({ strictPeerDependencies: false }))

expect(await exists(path.resolve('node_modules/.pnpm/abc-grand-parent@1.0.0_peer-c@1.0.0/node_modules/abc-grand-parent'))).toBeTruthy()
})
Expand Down Expand Up @@ -498,9 +499,9 @@ test('the list of transitive peer dependencies is kept up to date', async () =>
test('top peer dependency is linked on subsequent install. Reverse order', async () => {
prepareEmpty()

const manifest = await addDependenciesToPackage({}, ['abc-parent-with-ab@1.0.0'], await testDefaults())
const manifest = await addDependenciesToPackage({}, ['abc-parent-with-ab@1.0.0'], await testDefaults({ strictPeerDependencies: false }))

await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults({ modulesCacheMaxAge: 0 }))
await addDependenciesToPackage(manifest, ['peer-c@1.0.0'], await testDefaults({ modulesCacheMaxAge: 0, strictPeerDependencies: false }))

expect(await exists(path.resolve('node_modules/.pnpm/abc-parent-with-ab@1.0.0/node_modules/abc-parent-with-ab'))).toBeFalsy()
expect(await exists(path.resolve('node_modules/.pnpm/abc-parent-with-ab@1.0.0_peer-c@1.0.0/node_modules/abc-parent-with-ab'))).toBeTruthy()
Expand All @@ -519,7 +520,7 @@ test('peer dependencies are linked when running one named installation', async (

prepareEmpty()

const manifest = await addDependenciesToPackage({}, ['abc-grand-parent-with-c', 'abc-parent-with-ab', 'peer-c@2.0.0'], await testDefaults())
const manifest = await addDependenciesToPackage({}, ['abc-grand-parent-with-c', 'abc-parent-with-ab', 'peer-c@2.0.0'], await testDefaults({ strictPeerDependencies: false }))

const pkgVariationsDir = path.resolve('node_modules/.pnpm/abc@1.0.0')

Expand All @@ -542,16 +543,16 @@ test('peer dependencies are linked when running one named installation', async (

// this part was failing. See issue: https://github.com/pnpm/pnpm/issues/1201
await addDistTag({ package: 'peer-a', version: '1.0.1', distTag: 'latest' })
await install(manifest, await testDefaults({ update: true, depth: 100 }))
await install(manifest, await testDefaults({ update: true, depth: 100, strictPeerDependencies: false }))
})

test('peer dependencies are linked when running two separate named installations', async () => {
await addDistTag({ package: 'peer-a', version: '1.0.0', distTag: 'latest' })
await addDistTag({ package: 'peer-c', version: '1.0.0', distTag: 'latest' })
prepareEmpty()

const manifest = await addDependenciesToPackage({}, ['abc-grand-parent-with-c', 'peer-c@2.0.0'], await testDefaults())
await addDependenciesToPackage(manifest, ['abc-parent-with-ab'], await testDefaults())
const manifest = await addDependenciesToPackage({}, ['abc-grand-parent-with-c', 'peer-c@2.0.0'], await testDefaults({ strictPeerDependencies: false }))
await addDependenciesToPackage(manifest, ['abc-parent-with-ab'], await testDefaults({ strictPeerDependencies: false }))

const pkgVariationsDir = path.resolve('node_modules/.pnpm/abc@1.0.0')

Expand Down Expand Up @@ -685,7 +686,7 @@ test('peer dependency is grouped with dependent when the peer is a top dependenc

const reporter = sinon.spy()

await addDependenciesToPackage({}, ['ajv@4.10.4', 'ajv-keywords@1.5.0'], await testDefaults({ reporter, lockfileDir: path.resolve('..') }))
await addDependenciesToPackage({}, ['ajv@4.10.4', 'ajv-keywords@1.5.0'], await testDefaults({ reporter, lockfileDir: path.resolve('..'), strictPeerDependencies: false }))

expect(reporter.calledWithMatch({
message: `localhost+${REGISTRY_MOCK_PORT}/ajv-keywords@1.5.0 requires a peer of ajv@>=4.10.0 but none was installed.`,
Expand Down Expand Up @@ -718,8 +719,8 @@ test('peer dependency is grouped correctly with peer installed via separate inst
dependencies: {
abc: '1.0.0',
},
}, await testDefaults({ reporter, lockfileDir }))
await addDependenciesToPackage(manifest, ['peer-c@2.0.0'], await testDefaults({ reporter, lockfileDir }))
}, await testDefaults({ reporter, lockfileDir, strictPeerDependencies: false }))
await addDependenciesToPackage(manifest, ['peer-c@2.0.0'], await testDefaults({ reporter, lockfileDir, strictPeerDependencies: false }))

expect(await exists(path.join('../node_modules/.pnpm/abc@1.0.0_peer-c@2.0.0/node_modules/dep-of-pkg-with-1-dep'))).toBeTruthy()
})
Expand All @@ -730,7 +731,7 @@ test('peer dependency is grouped with dependent when the peer is a top dependenc
process.chdir('_')
const lockfileDir = path.resolve('..')

let manifest = await addDependenciesToPackage({}, ['ajv@4.10.4', 'ajv-keywords@1.5.0'], await testDefaults({ lockfileDir }))
let manifest = await addDependenciesToPackage({}, ['ajv@4.10.4', 'ajv-keywords@1.5.0'], await testDefaults({ lockfileDir, strictPeerDependencies: false }))

{
const lockfile = await readYamlFile<Lockfile>(path.resolve('..', WANTED_LOCKFILE))
Expand All @@ -746,7 +747,7 @@ test('peer dependency is grouped with dependent when the peer is a top dependenc
})
}

manifest = await install(manifest, await testDefaults({ lockfileDir }))
manifest = await install(manifest, await testDefaults({ lockfileDir, strictPeerDependencies: false }))

{
const lockfile = await readYamlFile<Lockfile>(path.resolve('..', WANTED_LOCKFILE))
Expand Down Expand Up @@ -774,6 +775,7 @@ test('peer dependency is grouped with dependent when the peer is a top dependenc
],
await testDefaults({
lockfileDir,
strictPeerDependencies: false,
})
)

Expand Down Expand Up @@ -835,7 +837,7 @@ test('external lockfile: peer dependency is grouped with dependent even after a
process.chdir('_')
const lockfileDir = path.resolve('..')

const manifest = await addDependenciesToPackage({}, ['peer-c@1.0.0', 'abc-parent-with-ab@1.0.0'], await testDefaults({ lockfileDir }))
const manifest = await addDependenciesToPackage({}, ['peer-c@1.0.0', 'abc-parent-with-ab@1.0.0'], await testDefaults({ lockfileDir, strictPeerDependencies: false }))

{
const lockfile = await readYamlFile<Lockfile>(path.resolve('..', WANTED_LOCKFILE))
Expand All @@ -851,7 +853,7 @@ test('external lockfile: peer dependency is grouped with dependent even after a
})
}

await addDependenciesToPackage(manifest, ['peer-c@2.0.0'], await testDefaults({ lockfileDir }))
await addDependenciesToPackage(manifest, ['peer-c@2.0.0'], await testDefaults({ lockfileDir, strictPeerDependencies: false }))

{
const lockfile = await readYamlFile<Lockfile>(path.resolve('..', WANTED_LOCKFILE))
Expand Down Expand Up @@ -1004,7 +1006,7 @@ test('warning is not reported when cannot resolve optional peer dependency', asy

const reporter = jest.fn()

await addDependenciesToPackage({}, ['abc-optional-peers@1.0.0', 'peer-c@2.0.0'], await testDefaults({ reporter }))
await addDependenciesToPackage({}, ['abc-optional-peers@1.0.0', 'peer-c@2.0.0'], await testDefaults({ reporter, strictPeerDependencies: false }))

expect(reporter).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -1076,7 +1078,7 @@ test('warning is not reported when cannot resolve optional peer dependency (spec

const reporter = jest.fn()

await addDependenciesToPackage({}, ['abc-optional-peers-meta-only@1.0.0', 'peer-c@2.0.0'], await testDefaults({ reporter }))
await addDependenciesToPackage({}, ['abc-optional-peers-meta-only@1.0.0', 'peer-c@2.0.0'], await testDefaults({ reporter, strictPeerDependencies: false }))

expect(reporter).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -1187,7 +1189,7 @@ test('peer dependency that is resolved by a dev dependency', async () => {
mutation: 'install',
rootDir: process.cwd(),
},
], await testDefaults({ fastUnpack: false, lockfileOnly: true }))
], await testDefaults({ fastUnpack: false, lockfileOnly: true, strictPeerDependencies: false }))

const lockfile = await project.readLockfile()
expect(lockfile.packages['/@types/mongoose/5.7.32'].dev).toBeTruthy()
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/lockfile.ts
Expand Up @@ -119,10 +119,10 @@ test("lockfile doesn't lock subdependencies that don't satisfy the new specs", a
const project = prepareEmpty()

// dependends on react-onclickoutside@5.9.0
const manifest = await addDependenciesToPackage({}, ['react-datetime@2.8.8'], await testDefaults({ fastUnpack: false, save: true }))
const manifest = await addDependenciesToPackage({}, ['react-datetime@2.8.8'], await testDefaults({ fastUnpack: false, save: true, strictPeerDependencies: false }))

// dependends on react-onclickoutside@0.3.4
await addDependenciesToPackage(manifest, ['react-datetime@1.3.0'], await testDefaults({ save: true }))
await addDependenciesToPackage(manifest, ['react-datetime@1.3.0'], await testDefaults({ save: true, strictPeerDependencies: false }))

expect(
project.requireModule('.pnpm/react-datetime@1.3.0/node_modules/react-onclickoutside/package.json').version
Expand Down
5 changes: 4 additions & 1 deletion packages/plugin-commands-installation/src/installDeps.ts
Expand Up @@ -233,7 +233,10 @@ when running add/update with the --workspace option')
rootDir: opts.dir,
targetDependenciesField: getSaveType(opts),
}
let [updatedImporter] = await mutateModules([mutatedProject], installOpts)
let [updatedImporter] = await mutateModules([mutatedProject], {
...installOpts,
strictPeerDependencies: opts.autoInstallPeers ? false : installOpts.strictPeerDependencies,
})
if (opts.save !== false) {
if (opts.autoInstallPeers && !isEmpty(updatedImporter.peerDependencyIssues?.intersections ?? {})) {
logger.info({
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-commands-installation/src/remove.ts
Expand Up @@ -65,6 +65,7 @@ export function rcOptionsTypes () {
'shared-workspace-lockfile',
'store',
'store-dir',
'strict-peer-dependencies',
'virtual-store-dir',
], allTypes)
}
Expand Down
4 changes: 2 additions & 2 deletions packages/pnpm/test/hooks.ts
Expand Up @@ -92,10 +92,10 @@ test('filterLog hook filters peer dependency warning', async () => {
}
`
await fs.writeFile('.pnpmfile.cjs', pnpmfile, 'utf8')
const result = execPnpmSync(['add', '@rollup/pluginutils@3.1.0'])
const result = execPnpmSync(['add', '@rollup/pluginutils@3.1.0', '--no-strict-peer-dependencies'])

expect(result.status).toBe(0)
expect(result.stdout.toString()).toEqual(
expect.not.stringContaining('requires a peer of rollup')
)
})
})
4 changes: 2 additions & 2 deletions packages/pnpm/test/install/hooks.ts
Expand Up @@ -607,8 +607,8 @@ test('readPackage hook is used during removal inside a workspace', async () => {
`, 'utf8')

process.chdir('project')
await execPnpm(['install'])
await execPnpm(['uninstall', 'is-positive'])
await execPnpm(['install', '--no-strict-peer-dependencies'])
await execPnpm(['uninstall', 'is-positive', '--no-strict-peer-dependencies'])

process.chdir('..')
const lockfile = await readYamlFile<Lockfile>('pnpm-lock.yaml')
Expand Down
2 changes: 1 addition & 1 deletion packages/pnpm/test/monorepo/index.ts
Expand Up @@ -1189,7 +1189,7 @@ test('peer dependency is grouped with dependent when the peer is a top dependenc
})
}

await execPnpm(['uninstall', 'ajv'])
await execPnpm(['uninstall', 'ajv', '--no-strict-peer-dependencies'])

{
const lockfile = await readYamlFile<Lockfile>(path.resolve('..', WANTED_LOCKFILE))
Expand Down