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: symlink a workspace pkg correctly, when it has a custom publish dir #5089

Merged
merged 5 commits into from
Jul 26, 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/chilled-penguins-hang.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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',
},
})
})