Skip to content

Commit

Permalink
fix: symlink a workspace pkg correctly, when it has a custom publish …
Browse files Browse the repository at this point in the history
…dir (#5089)

close #3901
  • Loading branch information
zkochan committed Jul 26, 2022
1 parent 01c5834 commit eb2426c
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-penguins-hang.md
@@ -0,0 +1,5 @@
---
"@pnpm/exportable-manifest": minor
---

Accept the module directory path where the dependency's manifest should be read.
6 changes: 6 additions & 0 deletions .changeset/fifty-otters-perform.md
@@ -0,0 +1,6 @@
---
"@pnpm/plugin-commands-publishing": patch
"pnpm": patch
---

It should be possible to publish a package with local dependencies from a custom publish directory (set via `publishConfig.directory`) [#3901](https://github.com/pnpm/pnpm/issues/3901#issuecomment-1194156886).
6 changes: 6 additions & 0 deletions .changeset/tasty-elephants-battle.md
@@ -0,0 +1,6 @@
---
"@pnpm/npm-resolver": patch
"pnpm": patch
---

When a project in a workspace has a `publishConfig.directory` set, dependent projects should install the project from that directory [#3901](https://github.com/pnpm/pnpm/issues/3901)
66 changes: 66 additions & 0 deletions packages/core/test/install/multipleImporters.ts
Expand Up @@ -11,6 +11,7 @@ import {
mutateModules,
} from '@pnpm/core'
import rimraf from '@zkochan/rimraf'
import loadJsonFile from 'load-json-file'
import exists from 'path-exists'
import pick from 'ramda/src/pick.js'
import sinon from 'sinon'
Expand Down Expand Up @@ -1530,3 +1531,68 @@ test('do not update dependency that has the same name as a dependency in the wor
'/is-negative/2.1.0',
])
})

test('symlink local package from the location described in its publishConfig.directory', async () => {
preparePackages([
{
location: 'project-1',
package: { name: 'project-1' },
},
{
location: 'project-1/dist',
package: { name: 'project-1-dist' },
},
{
location: 'project-2',
package: { name: 'project-2' },
},
])

const project1Manifest = {
name: 'project-1',
version: '1.0.0',
publishConfig: {
directory: 'dist',
},
}
const project2Manifest = {
name: 'project-2',
version: '1.0.0',

dependencies: {
'project-1': 'workspace:*',
},
}
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'),
},
]
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,
},
},
}
await mutateModules(importers, await testDefaults({ workspacePackages }))

const linkedManifest = await loadJsonFile<{ name: string }>('project-2/node_modules/project-1/package.json')
expect(linkedManifest.name).toBe('project-1-dist')
})
22 changes: 16 additions & 6 deletions packages/exportable-manifest/src/index.ts
Expand Up @@ -15,13 +15,22 @@ const PREPUBLISH_SCRIPTS = [
'postpublish',
]

export default async function makePublishManifest (dir: string, originalManifest: ProjectManifest, opts?: { readmeFile?: string }) {
export interface MakePublishManifestOptions {
modulesDir?: string
readmeFile?: string
}

export default async function makePublishManifest (
dir: string,
originalManifest: ProjectManifest,
opts?: MakePublishManifestOptions
) {
const publishManifest: ProjectManifest = omit(['pnpm', 'scripts'], originalManifest)
if (originalManifest.scripts != null) {
publishManifest.scripts = omit(PREPUBLISH_SCRIPTS, originalManifest.scripts)
}
for (const depsField of ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies']) {
const deps = await makePublishDependencies(dir, originalManifest[depsField])
const deps = await makePublishDependencies(dir, originalManifest[depsField], opts?.modulesDir)
if (deps != null) {
publishManifest[depsField] = deps
}
Expand All @@ -36,29 +45,30 @@ export default async function makePublishManifest (dir: string, originalManifest
return publishManifest
}

async function makePublishDependencies (dir: string, dependencies: Dependencies | undefined) {
async function makePublishDependencies (dir: string, dependencies: Dependencies | undefined, modulesDir?: string) {
if (dependencies == null) return dependencies
const publishDependencies: Dependencies = fromPairs(
await Promise.all(
Object.entries(dependencies)
.map(async ([depName, depSpec]) => [
depName,
await makePublishDependency(depName, depSpec, dir),
await makePublishDependency(depName, depSpec, dir, modulesDir),
])
) as any, // eslint-disable-line
)
return publishDependencies
}

async function makePublishDependency (depName: string, depSpec: string, dir: string) {
async function makePublishDependency (depName: string, depSpec: string, dir: string, modulesDir?: string) {
if (!depSpec.startsWith('workspace:')) {
return depSpec
}

// Dependencies with bare "*", "^" and "~" versions
const versionAliasSpecParts = /^workspace:([^@]+@)?([\^~*])$/.exec(depSpec)
if (versionAliasSpecParts != null) {
const { manifest } = await tryReadProjectManifest(path.join(dir, 'node_modules', depName))
modulesDir = modulesDir ?? path.join(dir, 'node_modules')
const { manifest } = await tryReadProjectManifest(path.join(modulesDir, depName))
if ((manifest == null) || !manifest.version) {
throw new PnpmError(
'CANNOT_RESOLVE_WORKSPACE_PROTOCOL',
Expand Down
22 changes: 15 additions & 7 deletions packages/npm-resolver/src/index.ts
Expand Up @@ -291,11 +291,13 @@ function pickMatchingLocalVersionOrNull (
}
}

interface LocalPackage {
dir: string
manifest: DependencyManifest
}

function resolveFromLocalPackage (
localPackage: {
dir: string
manifest: DependencyManifest
},
localPackage: LocalPackage,
normalizedPref: string | undefined,
opts: {
hardLinkLocalPackages?: boolean
Expand All @@ -305,12 +307,13 @@ function resolveFromLocalPackage (
) {
let id!: string
let directory!: string
const localPackageDir = resolveLocalPackageDir(localPackage)
if (opts.hardLinkLocalPackages) {
directory = normalize(path.relative(opts.lockfileDir!, localPackage.dir))
directory = normalize(path.relative(opts.lockfileDir!, localPackageDir))
id = `file:${directory}`
} else {
directory = localPackage.dir
id = `link:${normalize(path.relative(opts.projectDir, localPackage.dir))}`
directory = localPackageDir
id = `link:${normalize(path.relative(opts.projectDir, localPackageDir))}`
}
return {
id,
Expand All @@ -324,6 +327,11 @@ function resolveFromLocalPackage (
}
}

function resolveLocalPackageDir (localPackage: LocalPackage) {
if (localPackage.manifest.publishConfig?.directory == null) return localPackage.dir
return path.join(localPackage.dir, localPackage.manifest.publishConfig.directory)
}

function defaultTagForAlias (alias: string, defaultTag: string): RegistryPackageSpec {
return {
fetchSpec: defaultTag,
Expand Down
4 changes: 3 additions & 1 deletion packages/plugin-commands-publishing/src/pack.ts
Expand Up @@ -110,6 +110,7 @@ export async function handler (
filesMap,
projectDir: dir,
embedReadme: opts.embedReadme,
modulesDir: path.join(opts.dir, 'node_modules'),
})
if (!opts.ignoreScripts) {
await _runScriptsIfPresent(['postpack'], entryManifest)
Expand All @@ -132,6 +133,7 @@ async function packPkg (opts: {
filesMap: Record<string, string>
projectDir: string
embedReadme?: boolean
modulesDir: string
}): Promise<void> {
const {
destFile,
Expand All @@ -152,7 +154,7 @@ async function packPkg (opts: {
const mode = isExecutable ? 0o755 : 0o644
if (/^package\/package\.(json|json5|yaml)/.test(name)) {
const readmeFile = embedReadme ? await readReadmeFile(filesMap) : undefined
const publishManifest = await exportableManifest(projectDir, manifest, { readmeFile })
const publishManifest = await exportableManifest(projectDir, manifest, { readmeFile, modulesDir: opts.modulesDir })
pack.entry({ mode, mtime, name: 'package/package.json' }, JSON.stringify(publishManifest, null, 2))
continue
}
Expand Down
39 changes: 39 additions & 0 deletions packages/plugin-commands-publishing/test/pack.ts
Expand Up @@ -223,3 +223,42 @@ test('pack: remove publishConfig', async () => {
types: 'index.d.ts',
})
})

test('pack should read from the correct node_modules when publishing from a custom directory', async () => {
prepare({
name: 'custom-publish-dir',
version: '0.0.0',
publishConfig: {
directory: 'dist',
},
dependencies: {
local: 'workspace:*',
},
})

fs.mkdirSync('dist')
fs.copyFileSync('package.json', 'dist/package.json')
fs.mkdirSync('node_modules/local', { recursive: true })
fs.writeFileSync('node_modules/local/package.json', JSON.stringify({ name: 'local', version: '1.0.0' }), 'utf8')

await pack.handler({
...DEFAULT_OPTS,
argv: { original: [] },
dir: process.cwd(),
extraBinPaths: [],
packDestination: process.cwd(),
})

await tar.x({ file: 'custom-publish-dir-0.0.0.tgz' })

expect((await import(path.resolve('package/package.json'))).default).toStrictEqual({
name: 'custom-publish-dir',
version: '0.0.0',
dependencies: {
local: '1.0.0',
},
publishConfig: {
directory: 'dist',
},
})
})

0 comments on commit eb2426c

Please sign in to comment.