Skip to content

Commit

Permalink
fix: only build git-hosted dependency if it has no main file (#5868)
Browse files Browse the repository at this point in the history
close #5845
  • Loading branch information
zkochan committed Jan 3, 2023
1 parent a02f1fe commit 40a4818
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 9 deletions.
6 changes: 6 additions & 0 deletions .changeset/hip-beers-smile.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@pnpm/prepare-package": patch
"pnpm": patch
---

Only run prepublish scripts of git-hosted dependencies, if the dependency doesn't have a main file. In this case we can assume that the dependencies has to be built.
18 changes: 11 additions & 7 deletions exec/prepare-package/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import fs from 'fs'
import path from 'path'
import { runLifecycleHook, RunLifecycleHookOptions } from '@pnpm/lifecycle'
import { safeReadPackageJsonFromDir } from '@pnpm/read-package-json'
import { PackageScripts } from '@pnpm/types'
import { PackageManifest } from '@pnpm/types'
import rimraf from '@zkochan/rimraf'
import preferredPM from 'preferred-pm'
import omit from 'ramda/src/omit'
Expand All @@ -21,7 +22,7 @@ export interface PreparePackageOptions {

export async function preparePackage (opts: PreparePackageOptions, pkgDir: string) {
const manifest = await safeReadPackageJsonFromDir(pkgDir)
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest.scripts)) return
if (manifest?.scripts == null || !packageShouldBeBuilt(manifest, pkgDir)) return
const pm = (await preferredPM(pkgDir))?.name ?? 'npm'
const execOpts: RunLifecycleHookOptions = {
depPath: `${manifest.name}@${manifest.version}`,
Expand All @@ -47,9 +48,12 @@ export async function preparePackage (opts: PreparePackageOptions, pkgDir: strin
await rimraf(path.join(pkgDir, 'node_modules'))
}

function packageShouldBeBuilt (packageScripts: PackageScripts): boolean {
return [
...PREPUBLISH_SCRIPTS,
'prepare',
].some((scriptName) => packageScripts[scriptName] != null && packageScripts[scriptName] !== '')
function packageShouldBeBuilt (manifest: PackageManifest, pkgDir: string): boolean {
if (manifest.scripts == null) return false
const scripts = manifest.scripts
if (scripts.prepare != null && scripts.prepare !== '') return true
const hasPrepublishScript = PREPUBLISH_SCRIPTS.some((scriptName) => scripts[scriptName] != null && scripts[scriptName] !== '')
if (!hasPrepublishScript) return false
const mainFile = manifest.main ?? 'index.js'
return !fs.existsSync(path.join(pkgDir, mainFile))
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "has-prepublish-script",
"version": "1.0.0",
"file": "main.js",
"scripts": {
"prepublish": "node -e \"process.stdout.write('prepublish')\" | json-append output.json"
},
"devDependencies": {
"json-append": "1.1.1"
}
}
Empty file.
11 changes: 10 additions & 1 deletion exec/prepare-package/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,20 @@ import { sync as loadJsonFile } from 'load-json-file'

const f = fixtures(__dirname)

test('prepare package runs the prepbublish script', async () => {
test('prepare package runs the prepublish script', async () => {
const tmp = tempDir()
f.copy('has-prepublish-script', tmp)
await preparePackage({ rawConfig: {} }, tmp)
expect(loadJsonFile(path.join(tmp, 'output.json'))).toStrictEqual([
'prepublish',
])
})

test('prepare package does not run the prebublish script if the main file is present', async () => {
const tmp = tempDir()
f.copy('has-prepublish-script-and-main-file', tmp)
await preparePackage({ rawConfig: {} }, tmp)
expect(loadJsonFile(path.join(tmp, 'output.json'))).toStrictEqual([
'prepublish',
])
})
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = {
testEnvironment: "node",
collectCoverage: true,
coveragePathIgnorePatterns: ["/node_modules/"],
testPathIgnorePatterns: ["/fixtures/", "<rootDir>/test/utils/.+"],
testPathIgnorePatterns: ["/fixtures/", "/__fixtures__/", "<rootDir>/test/utils/.+"],
testTimeout: 4 * 60 * 1000, // 4 minutes
setupFilesAfterEnv: [path.join(__dirname, "jest.setup.js")],
cacheDirectory: path.join(__dirname, ".jest-cache", packageName),
Expand Down

0 comments on commit 40a4818

Please sign in to comment.