Skip to content

Commit

Permalink
fix: pnpm link --global <pkg> should not change the type of the depen…
Browse files Browse the repository at this point in the history
…dency
  • Loading branch information
lvqq committed Oct 18, 2022
1 parent e797072 commit 6fb896b
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 11 deletions.
8 changes: 8 additions & 0 deletions .changeset/odd-peas-fetch.md
@@ -0,0 +1,8 @@
---
"@pnpm/core": patch
"@pnpm/manifest-utils": patch
"@pnpm/plugin-commands-installation": patch
"@pnpm/types": patch
---

`pnpm link --global <pkg>` should not change the type of the dependency
14 changes: 11 additions & 3 deletions packages/core/src/link/index.ts
Expand Up @@ -37,6 +37,7 @@ import {
type LinkFunctionOptions = LinkOptions & {
linkToBin?: string
dir: string
targetDependenciesFieldMap?: Record<string, DependenciesField>
}

export { LinkFunctionOptions }
Expand Down Expand Up @@ -74,12 +75,19 @@ export async function link (
throw new PnpmError('INVALID_PACKAGE_NAME', `Package in ${linkFromPath} must have a name field to be linked`)
}

let targetDependencyType: string
if (opts.targetDependenciesFieldMap && Object.keys(opts.targetDependenciesFieldMap).length > 0 && typeof linkFrom === 'string') {
targetDependencyType = opts.targetDependenciesFieldMap?.[linkFrom]
} else if (opts.targetDependenciesField) {
targetDependencyType = 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 @@ -109,7 +117,7 @@ export async function link (
// TODO: cover with test that linking reports with correct dependency types
const stu = specsToUpsert.find((s) => s.alias === manifest.name)
await symlinkDirectRootDependency(path, destModules, alias, {
fromDependenciesField: stu?.saveType ?? opts.targetDependenciesField,
fromDependenciesField: stu?.saveType ?? opts.targetDependenciesFieldMap?.[path] ?? opts.targetDependenciesField,
linkedPackage: manifest,
prefix: opts.dir,
})
Expand All @@ -122,7 +130,7 @@ export async function link (
})

let newPkg!: ProjectManifest
if (opts.targetDependenciesField) {
if (opts.targetDependenciesField ?? Object.keys(opts.targetDependenciesFieldMap ?? {}).length > 0) {
newPkg = await updateProjectManifestObject(opts.dir, opts.manifest, specsToUpsert)
for (const { alias } of specsToUpsert) {
updatedWantedLockfile.importers[importerId].specifiers[alias] = getSpecFromPackageManifest(newPkg, alias)
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/link/options.ts
Expand Up @@ -22,6 +22,7 @@ interface StrictLinkOptions {
storeDir: string
reporter: ReporterFunction
targetDependenciesField?: DependenciesField
targetDependenciesFieldMap?: Record<string, DependenciesField>
dir: string
preferSymlinkedExecutables: boolean

Expand Down
33 changes: 32 additions & 1 deletion packages/core/test/link.ts
Expand Up @@ -6,7 +6,7 @@ import {
link,
} from '@pnpm/core'
import { fixtures } from '@pnpm/test-fixtures'
import { prepareEmpty } from '@pnpm/prepare'
import { prepare, prepareEmpty } from '@pnpm/prepare'
import { addDistTag } from '@pnpm/registry-mock'
import { RootLog } from '@pnpm/core-loggers'
import sinon from 'sinon'
Expand Down Expand Up @@ -210,6 +210,37 @@ test('throws error is package name is not defined', async () => {
}
})

test('link with option targetDependenciesFieldMap', async () => {
const project = prepare({
devDependencies: {
'@pnpm.e2e/hello-world-js-bin': '*',
},
})

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': '*',
},
},
targetDependenciesFieldMap: {
[`../${linkedPkgName}`]: 'devDependencies',
},
}))

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
4 changes: 2 additions & 2 deletions packages/manifest-utils/src/getAllDependenciesFromManifest.ts
@@ -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, DependenciesManifestField } from '@pnpm/types'

export const getDependencyTypeFromManifest = (
manifest: Pick<ProjectManifest, DependenciesManifestField>,
depName: string
): DependenciesManifestField | null => {
if (manifest.optionalDependencies?.[depName]) return 'optionalDependencies'
else if (manifest.peerDependencies?.[depName]) return 'peerDependencies'
else if (manifest.dependencies?.[depName]) return 'dependencies'
else if (manifest.devDependencies?.[depName]) return 'devDependencies'
else 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, DependenciesManifestField } from '@pnpm/types'

export function getSpecFromPackageManifest (
manifest: Pick<ProjectManifest, 'devDependencies' | 'dependencies' | 'optionalDependencies' | 'peerDependencies'>,
manifest: Pick<ProjectManifest, DependenciesManifestField>,
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
38 changes: 38 additions & 0 deletions packages/manifest-utils/test/getDependencyTypeFromManifest.test.ts
@@ -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)
})
15 changes: 12 additions & 3 deletions packages/plugin-commands-installation/src/link.ts
Expand Up @@ -21,6 +21,8 @@ import {
LinkFunctionOptions,
WorkspacePackages,
} from '@pnpm/core'
import { getDependencyTypeFromManifest } from '@pnpm/manifest-utils'
import { DependenciesManifestField } from '@pnpm/types'
import pLimit from 'p-limit'
import pathAbsolute from 'path-absolute'
import pick from 'ramda/src/pick'
Expand Down Expand Up @@ -160,6 +162,9 @@ export async function handler (
}))
)

const { manifest, writeProjectManifest } = await readProjectManifest(linkCwdDir, opts)
const targetDependenciesFieldMap: Record<string, DependenciesManifestField | undefined> = {}

if (pkgNames.length > 0) {
let globalPkgNames!: string[]
if (opts.workspaceDir) {
Expand All @@ -178,11 +183,14 @@ export async function handler (
globalPkgNames = pkgNames
}
const globalPkgPath = pathAbsolute(opts.dir)
globalPkgNames.forEach((pkgName) => pkgPaths.push(path.join(globalPkgPath, 'node_modules', pkgName)))
globalPkgNames.forEach((pkgName) => {
const pkgPath = path.join(globalPkgPath, 'node_modules', pkgName)
pkgPaths.push(pkgPath)
const dependencyType = getDependencyTypeFromManifest(manifest, pkgName)
targetDependenciesFieldMap[pkgPath] = dependencyType ?? linkOpts.targetDependenciesField
})
}

const { manifest, writeProjectManifest } = await readProjectManifest(linkCwdDir, opts)

const linkConfig = await getConfig(
{ ...opts.cliOptions, dir: cwd },
{
Expand All @@ -195,6 +203,7 @@ export async function handler (
const newManifest = await link(pkgPaths, path.join(linkCwdDir, 'node_modules'), {
...linkConfig,
targetDependenciesField: linkOpts.targetDependenciesField,
targetDependenciesFieldMap,
storeController: storeL.ctrl,
storeDir: storeL.dir,
manifest,
Expand Down
51 changes: 51 additions & 0 deletions packages/plugin-commands-installation/test/link.ts
Expand Up @@ -4,6 +4,7 @@ import readYamlFile from 'read-yaml-file'
import { install, link } from '@pnpm/plugin-commands-installation'
import { prepare, preparePackages } from '@pnpm/prepare'
import { assertProject, isExecutable } from '@pnpm/assert-project'
import { readProjectManifest } from '@pnpm/cli-utils'
import { fixtures } from '@pnpm/test-fixtures'
import PATH from 'path-name'
import writePkg from 'write-pkg'
Expand Down Expand Up @@ -145,6 +146,56 @@ test('link a global package to the specified directory', async function () {
await project.has('global-package-with-bin')
})

test('do not change the type of the dependency when linking a global package', async function () {
prepare({
devDependencies: {
'global-package-with-bin': '*',
},
})
process.chdir('..')

const globalDir = path.resolve('global')
const globalBin = path.join(globalDir, 'bin')
const oldPath = process.env[PATH]
process.env[PATH] = `${globalBin}${path.delimiter}${oldPath ?? ''}`
await fs.mkdir(globalBin, { recursive: true })

await writePkg('global-package-with-bin', { name: 'global-package-with-bin', version: '1.0.0', bin: 'bin.js' })
await fs.writeFile('global-package-with-bin/bin.js', '#!/usr/bin/env node\nconsole.log(/hi/)\n', 'utf8')

process.chdir('global-package-with-bin')

// link to global
await link.handler({
...DEFAULT_OPTS,
cliOptions: {
global: true,
},
bin: globalBin,
dir: globalDir,
})

process.chdir('../project')

// link from global
await link.handler({
...DEFAULT_OPTS,
cliOptions: {
global: true,
},
bin: globalBin,
dir: globalDir,
}, ['global-package-with-bin'])

const { manifest } = await readProjectManifest(process.cwd(), {})

process.env[PATH] = oldPath

expect(manifest.devDependencies).toStrictEqual({
'global-package-with-bin': '^1.0.0',
})
})

test('relative link', async () => {
const project = prepare({
dependencies: {
Expand Down
2 changes: 2 additions & 0 deletions packages/types/src/misc.ts
@@ -1,5 +1,7 @@
export type DependenciesField = 'optionalDependencies' | 'dependencies' | 'devDependencies'

export type DependenciesManifestField = DependenciesField | 'peerDependencies'

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

0 comments on commit 6fb896b

Please sign in to comment.