Skip to content

Commit e85aea2

Browse files
bteazkochan
andauthored
perf: lazily load README when embedding publish manifest (#12278)
--------- Co-authored-by: Zoltan Kochan <z@kochan.io>
1 parent 564619f commit e85aea2

4 files changed

Lines changed: 83 additions & 29 deletions

File tree

.changeset/lazy-readme-embed.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@pnpm/releasing.exportable-manifest": patch
3+
"@pnpm/releasing.commands": patch
4+
"pnpm": patch
5+
---
6+
7+
Avoid reading `README.md` from disk when publishing if the publish manifest already provides a `readme` field. The README is now only read lazily, inside `createExportableManifest`, when it is actually needed.

releasing/commands/src/publish/pack.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,6 @@ function preventBundledDependenciesWithoutHoistedNodeLinker (nodeLinker: Config[
352352
}
353353
}
354354

355-
async function readReadmeFile (projectDir: string): Promise<string | undefined> {
356-
const files = await fs.promises.readdir(projectDir)
357-
const readmePath = files.find(name => /readme\.md$/i.test(name))
358-
const readmeFile = readmePath ? await fs.promises.readFile(path.join(projectDir, readmePath), 'utf8') : undefined
359-
360-
return readmeFile
361-
}
362-
363355
async function packPkg (opts: {
364356
destFile: string
365357
filesMap: Record<string, string>
@@ -405,11 +397,10 @@ async function createPublishManifest (opts: {
405397
skipManifestObfuscation?: boolean
406398
}): Promise<ExportedManifest> {
407399
const { projectDir, embedReadme, modulesDir, manifest, catalogs, hooks, skipManifestObfuscation } = opts
408-
const readmeFile = embedReadme ? await readReadmeFile(projectDir) : undefined
409400
return createExportableManifest(projectDir, manifest, {
410401
catalogs,
411402
hooks,
412-
readmeFile,
403+
embedReadme,
413404
modulesDir,
414405
skipManifestObfuscation,
415406
})

releasing/exportable-manifest/src/index.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import fs from 'node:fs'
12
import path from 'node:path'
23

34
import { type CatalogResolver, resolveFromCatalog } from '@pnpm/catalogs.resolver'
@@ -29,7 +30,16 @@ export interface MakePublishManifestOptions {
2930
hooks?: Hooks
3031
modulesDir?: string
3132
skipManifestObfuscation?: boolean
32-
readmeFile?: string
33+
embedReadme?: boolean
34+
}
35+
36+
async function readReadmeFile (projectDir: string): Promise<string | undefined> {
37+
const entries = await fs.promises.readdir(projectDir, { withFileTypes: true })
38+
// Only embed a regular README.md file. A symlink could point outside the
39+
// project and leak its target's contents into the published manifest.
40+
const readmeEntry = entries.find((entry) => entry.isFile() && /^readme\.md$/i.test(entry.name))
41+
if (readmeEntry == null) return undefined
42+
return fs.promises.readFile(path.join(projectDir, readmeEntry.name), 'utf8')
3343
}
3444

3545
export async function createExportableManifest (
@@ -72,8 +82,11 @@ export async function createExportableManifest (
7282

7383
overridePublishConfig(publishManifest)
7484

75-
if (opts?.readmeFile) {
76-
publishManifest.readme ??= opts.readmeFile
85+
if (publishManifest.readme == null && opts?.embedReadme) {
86+
const readme = await readReadmeFile(dir)
87+
if (readme != null) {
88+
publishManifest.readme = readme
89+
}
7790
}
7891

7992
for (const hook of opts?.hooks?.beforePacking ?? []) {

releasing/exportable-manifest/test/index.test.ts

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
/// <reference path="../../../__typings__/index.d.ts"/>
2+
import fs from 'node:fs'
3+
import os from 'node:os'
24
import path from 'node:path'
35

46
import { expect, test } from '@jest/globals'
@@ -119,15 +121,17 @@ test('skipManifestObfuscation does not mutate the original manifest', async () =
119121
},
120122
}
121123

122-
expect(await createExportableManifest(process.cwd(), manifest, {
123-
...defaultOpts,
124-
skipManifestObfuscation: true,
125-
readmeFile: 'readme content',
126-
})).toStrictEqual({
127-
name: 'foo',
128-
version: '1.0.0',
129-
main: './dist/index.js',
130-
readme: 'readme content',
124+
await withTempProjectReadme('readme content', async (projectDir) => {
125+
expect(await createExportableManifest(projectDir, manifest, {
126+
...defaultOpts,
127+
skipManifestObfuscation: true,
128+
embedReadme: true,
129+
})).toStrictEqual({
130+
name: 'foo',
131+
version: '1.0.0',
132+
main: './dist/index.js',
133+
readme: 'readme content',
134+
})
131135
})
132136

133137
expect(manifest).toStrictEqual({
@@ -140,16 +144,55 @@ test('skipManifestObfuscation does not mutate the original manifest', async () =
140144
})
141145

142146
test('readme added to published manifest', async () => {
143-
expect(await createExportableManifest(process.cwd(), {
144-
name: 'foo',
145-
version: '1.0.0',
146-
}, { ...defaultOpts, readmeFile: 'readme content' })).toStrictEqual({
147-
name: 'foo',
148-
version: '1.0.0',
149-
readme: 'readme content',
147+
await withTempProjectReadme('readme content', async (projectDir) => {
148+
expect(await createExportableManifest(projectDir, {
149+
name: 'foo',
150+
version: '1.0.0',
151+
}, {
152+
...defaultOpts,
153+
embedReadme: true,
154+
})).toStrictEqual({
155+
name: 'foo',
156+
version: '1.0.0',
157+
readme: 'readme content',
158+
})
150159
})
151160
})
152161

162+
;(process.platform === 'win32' ? test.skip : test)('readme is not embedded when README.md is a symlink pointing outside the project', async () => {
163+
const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-readme-'))
164+
try {
165+
const secretFile = path.join(tmpDir, 'secret.txt')
166+
await fs.promises.writeFile(secretFile, 'secret content', 'utf8')
167+
const projectDir = path.join(tmpDir, 'project')
168+
await fs.promises.mkdir(projectDir)
169+
await fs.promises.symlink(secretFile, path.join(projectDir, 'README.md'))
170+
171+
expect(await createExportableManifest(projectDir, {
172+
name: 'foo',
173+
version: '1.0.0',
174+
}, {
175+
...defaultOpts,
176+
embedReadme: true,
177+
})).toStrictEqual({
178+
name: 'foo',
179+
version: '1.0.0',
180+
})
181+
} finally {
182+
await fs.promises.rm(tmpDir, { recursive: true, force: true })
183+
}
184+
})
185+
186+
async function withTempProjectReadme<T> (readmeContent: string, fn: (projectDir: string) => Promise<T>): Promise<T> {
187+
const projectDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-readme-'))
188+
try {
189+
await fs.promises.writeFile(path.join(projectDir, 'README.md'), readmeContent, 'utf8')
190+
return await fn(projectDir)
191+
} finally {
192+
await fs.promises.rm(projectDir, { recursive: true, force: true })
193+
}
194+
}
195+
153196
test('workspace deps are replaced', async () => {
154197
const manifest: ProjectManifest = {
155198
name: 'workspace-protocol-package',

0 commit comments

Comments
 (0)