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

fix: pnpm link --global <pkg> should not change the type of the dependency #5512

Merged
merged 5 commits into from Oct 19, 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
5 changes: 5 additions & 0 deletions .changeset/lazy-days-run.md
@@ -0,0 +1,5 @@
---
"@pnpm/types": minor
---

New type exported: DependenciesOrPeersField
6 changes: 6 additions & 0 deletions .changeset/odd-peas-fetch.md
@@ -0,0 +1,6 @@
---
"pnpm": patch
"@pnpm/core": patch
---

`pnpm link --global <pkg>` should not change the type of the dependency [#5478](https://github.com/pnpm/pnpm/issues/5478).
5 changes: 5 additions & 0 deletions .changeset/ten-dancers-happen.md
@@ -0,0 +1,5 @@
---
"@pnpm/manifest-utils": minor
---

New function exported: `getDependencyTypeFromManifest()`.
8 changes: 6 additions & 2 deletions packages/core/src/link/index.ts
Expand Up @@ -15,6 +15,7 @@ import { logger, streamParser } from '@pnpm/logger'
import {
getPref,
getSpecFromPackageManifest,
getDependencyTypeFromManifest,
guessDependencyType,
PackageSpecObject,
updateProjectManifestObject,
Expand Down Expand Up @@ -74,12 +75,14 @@ export async function link (
throw new PnpmError('INVALID_PACKAGE_NAME', `Package in ${linkFromPath} must have a name field to be linked`)
}

const targetDependencyType = getDependencyTypeFromManifest(opts.manifest, manifest.name) ?? opts.targetDependenciesField

specsToUpsert.push({
alias: manifest.name,
pref: getPref(manifest.name, manifest.name, manifest.version, {
pinnedVersion: opts.pinnedVersion,
}),
saveType: (opts.targetDependenciesField ?? (ctx.manifest && guessDependencyType(manifest.name, ctx.manifest))) as DependenciesField,
saveType: (targetDependencyType ?? (ctx.manifest && guessDependencyType(manifest.name, ctx.manifest))) as DependenciesField,
})

const packagePath = normalize(path.relative(opts.dir, linkFromPath))
Expand Down Expand Up @@ -108,8 +111,9 @@ export async function link (
for (const { alias, manifest, path } of linkedPkgs) {
// TODO: cover with test that linking reports with correct dependency types
const stu = specsToUpsert.find((s) => s.alias === manifest.name)
const targetDependencyType = getDependencyTypeFromManifest(opts.manifest, manifest.name) ?? opts.targetDependenciesField
await symlinkDirectRootDependency(path, destModules, alias, {
fromDependenciesField: stu?.saveType ?? opts.targetDependenciesField,
fromDependenciesField: stu?.saveType ?? (targetDependencyType as DependenciesField),
linkedPackage: manifest,
prefix: opts.dir,
})
Expand Down
24 changes: 24 additions & 0 deletions packages/core/test/link.ts
Expand Up @@ -210,6 +210,30 @@ test('throws error is package name is not defined', async () => {
}
})

test('link should not change the type of the dependency', async () => {
const project = prepareEmpty()

const linkedPkgName = 'hello-world-js-bin'
const linkedPkgPath = path.resolve('..', linkedPkgName)

f.copy(linkedPkgName, linkedPkgPath)
await link([`../${linkedPkgName}`], path.join(process.cwd(), 'node_modules'), await testDefaults({
dir: process.cwd(),
manifest: {
devDependencies: {
'@pnpm.e2e/hello-world-js-bin': '*',
},
},
}))

await project.isExecutable('.bin/hello-world-js-bin')

const wantedLockfile = await project.readLockfile()
expect(wantedLockfile.devDependencies).toStrictEqual({
'@pnpm.e2e/hello-world-js-bin': 'link:../hello-world-js-bin',
})
})

// test.skip('relative link when an external lockfile is used', async () => {
// const projects = prepare(t, [
// {
Expand Down
@@ -1,7 +1,7 @@
import { Dependencies, ProjectManifest } from '@pnpm/types'
import { Dependencies, DependenciesField, ProjectManifest } from '@pnpm/types'

export function getAllDependenciesFromManifest (
pkg: Pick<ProjectManifest, 'devDependencies' | 'dependencies' | 'optionalDependencies'>
pkg: Pick<ProjectManifest, DependenciesField>
): Dependencies {
return {
...pkg.devDependencies,
Expand Down
12 changes: 12 additions & 0 deletions packages/manifest-utils/src/getDependencyTypeFromManifest.ts
@@ -0,0 +1,12 @@
import { ProjectManifest, DependenciesOrPeersField } from '@pnpm/types'

export function getDependencyTypeFromManifest (
manifest: Pick<ProjectManifest, DependenciesOrPeersField>,
depName: string
): DependenciesOrPeersField | null {
if (manifest.optionalDependencies?.[depName]) return 'optionalDependencies'
if (manifest.dependencies?.[depName]) return 'dependencies'
if (manifest.devDependencies?.[depName]) return 'devDependencies'
if (manifest.peerDependencies?.[depName]) return 'peerDependencies'
return null
}
4 changes: 2 additions & 2 deletions packages/manifest-utils/src/getSpecFromPackageManifest.ts
@@ -1,7 +1,7 @@
import { ProjectManifest } from '@pnpm/types'
import { ProjectManifest, DependenciesOrPeersField } from '@pnpm/types'

export function getSpecFromPackageManifest (
manifest: Pick<ProjectManifest, 'devDependencies' | 'dependencies' | 'optionalDependencies' | 'peerDependencies'>,
manifest: Pick<ProjectManifest, DependenciesOrPeersField>,
depName: string
) {
return manifest.optionalDependencies?.[depName] ??
Expand Down
1 change: 1 addition & 0 deletions packages/manifest-utils/src/index.ts
Expand Up @@ -7,6 +7,7 @@ import { getSpecFromPackageManifest } from './getSpecFromPackageManifest'

export * from './getPref'
export * from './updateProjectManifestObject'
export * from './getDependencyTypeFromManifest'

export { getSpecFromPackageManifest }

Expand Down
@@ -0,0 +1,38 @@
import { getDependencyTypeFromManifest } from '@pnpm/manifest-utils'

test('getDependencyTypeFromManifest()', () => {
expect(
getDependencyTypeFromManifest({
dependencies: {
foo: '1.0.0',
},
}, 'foo')).toEqual('dependencies')

expect(
getDependencyTypeFromManifest({
devDependencies: {
foo: '1.0.0',
},
}, 'foo')).toEqual('devDependencies')

expect(
getDependencyTypeFromManifest({
optionalDependencies: {
foo: '1.0.0',
},
}, 'foo')).toEqual('optionalDependencies')

expect(
getDependencyTypeFromManifest({
peerDependencies: {
foo: '1.0.0',
},
}, 'foo')).toEqual('peerDependencies')

expect(
getDependencyTypeFromManifest({
peerDependencies: {
foo: '1.0.0',
},
}, 'bar')).toEqual(null)
})
2 changes: 2 additions & 0 deletions packages/types/src/misc.ts
@@ -1,5 +1,7 @@
export type DependenciesField = 'optionalDependencies' | 'dependencies' | 'devDependencies'

export type DependenciesOrPeersField = DependenciesField | 'peerDependencies'

// NOTE: The order in this array is important.
export const DEPENDENCIES_FIELDS: DependenciesField[] = [
'optionalDependencies',
Expand Down