refactor: create pkg-requires-build

pnpm · zkochan · Feb 27, 2024 · Feb 24, 2024 · Feb 26, 2024 · Feb 26, 2024
commit db539f7069f6b395f8fbcc5832fa1486b8846d2d
@@ -0,0 +1,15 @@
+# @pnpm/exec.pkg-requires-build
+
+> Checks if a package requires to be built
+
+[![npm version](https://img.shields.io/npm/v/@pnpm/exec.pkg-requires-build.svg)](https://www.npmjs.com/package/@pnpm/exec.pkg-requires-build)
+
+## Installation
+
+```sh
+pnpm add @pnpm/exec.pkg-requires-build
+```
+
+## License
+
+MIT
@@ -1,7 +1,7 @@
 {
-  "name": "@pnpm/exec.files-include-install-scripts",
-  "version": "1.0.0",
-  "description": "Checks if a package has files indicating that it needs to be built during installation",
+  "name": "@pnpm/exec.pkg-requires-build",
+  "version": "0.0.0",
+  "description": "Checks if a package requires to be built",
   "main": "lib/index.js",
   "types": "lib/index.d.ts",
   "files": [
@@ -18,7 +18,7 @@
     "fix": "tslint -c tslint.json src/**/*.ts test/**/*.ts --fix",
     "compile": "tsc --build && pnpm run lint --fix"
   },
-  "repository": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts",
+  "repository": "https://github.com/pnpm/pnpm/blob/main/exec/pkg-requires-build",
   "keywords": [
     "pnpm9",
     "pnpm"
@@ -27,9 +27,12 @@
   "bugs": {
     "url": "https://github.com/pnpm/pnpm/issues"
   },
-  "homepage": "https://github.com/pnpm/pnpm/blob/main/exec/files-include-install-scripts#readme",
+  "homepage": "https://github.com/pnpm/pnpm/blob/main/exec/pkg-requires-build#readme",
+  "dependencies": {
+    "@pnpm/types": "workspace:*"
+  },
   "devDependencies": {
-    "@pnpm/exec.files-include-install-scripts": "workspace:*"
+    "@pnpm/exec.pkg-requires-build": "workspace:*"
   },
   "funding": "https://opencollective.com/pnpm",
   "exports": {

@@ -0,0 +1,17 @@
+import { type DependencyManifest } from '@pnpm/types'
+
+export function pkgRequiresBuild (manifest: Partial<DependencyManifest> | undefined, filesIndex: Record<string, unknown>) {
+  return Boolean(
+    manifest?.scripts != null && (
+      Boolean(manifest.scripts.preinstall) ||
+      Boolean(manifest.scripts.install) ||
+      Boolean(manifest.scripts.postinstall)
+    ) ||
+    filesIncludeInstallScripts(filesIndex)
+  )
+}
+
+function filesIncludeInstallScripts (filesIndex: Record<string, unknown>): boolean {
+  return filesIndex['binding.gyp'] != null ||
+    Object.keys(filesIndex).some((filename) => !(filename.match(/^[.]hooks[\\/]/) == null)) // TODO: optimize this
+}
@@ -8,6 +8,10 @@
     "src/**/*.ts",
     "../../__typings__/**/*.d.ts"
   ],
-  "references": [],
+  "references": [
+    {
+      "path": "../../packages/types"
+    }
+  ],
   "composite": true
 }
@@ -34,7 +34,7 @@
     "@pnpm/logger": "^5.0.0"
   },
   "dependencies": {
-    "@pnpm/exec.files-include-install-scripts": "1.0.0",
+    "@pnpm/exec.pkg-requires-build": "workspace:*",
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/fs.packlist": "workspace:*",
     "@pnpm/read-project-manifest": "workspace:*",

@@ -1,6 +1,6 @@
 import { promises as fs, type Stats } from 'fs'
 import path from 'path'
-import { filesIncludeInstallScripts } from '@pnpm/exec.files-include-install-scripts'
+import { pkgRequiresBuild } from '@pnpm/exec.pkg-requires-build'
 import type { DirectoryFetcher, DirectoryFetcherOptions } from '@pnpm/fetcher-base'
 import { logger } from '@pnpm/logger'
 import { packlist } from '@pnpm/fs.packlist'
@@ -22,7 +22,7 @@ export function createDirectoryFetcher (
 
   const directoryFetcher: DirectoryFetcher = (cafs, resolution, opts) => {
     const dir = path.join(opts.lockfileDir, resolution.directory)
-    return fetchFromDir(dir, opts)
+    return fetchFromDir(dir)
   }
 
   return {
@@ -37,33 +37,22 @@ export async function fetchFromDir (
   opts: FetchFromDirOpts & CreateDirectoryFetcherOptions
 ) {
   if (opts.includeOnlyPackageFiles) {
-    return fetchPackageFilesFromDir(dir, opts)
+    return fetchPackageFilesFromDir(dir)
   }
   const readFileStat: ReadFileStat = opts?.resolveSymlinks === true ? realFileStat : fileStat
-  return fetchAllFilesFromDir(readFileStat, dir, opts)
+  return fetchAllFilesFromDir(readFileStat, dir)
 }
 
 async function fetchAllFilesFromDir (
   readFileStat: ReadFileStat,
-  dir: string,
-  opts: FetchFromDirOpts
+  dir: string
 ) {
   const filesIndex = await _fetchAllFilesFromDir(readFileStat, dir)
-  let manifest: DependencyManifest | undefined
-  if (opts.readManifest) {
-    // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
-    // Safe read was added to support the Bit workspace in which the components have no package.json files.
-    // Related PR in Bit: https://github.com/teambit/bit/pull/5251
-    manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
-  }
-  const requiresBuild = Boolean(
-    manifest?.scripts != null && (
-      Boolean(manifest.scripts.preinstall) ||
-      Boolean(manifest.scripts.install) ||
-      Boolean(manifest.scripts.postinstall)
-    ) ||
-    filesIncludeInstallScripts(filesIndex)
-  )
+  // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
+  // Safe read was added to support the Bit workspace in which the components have no package.json files.
+  // Related PR in Bit: https://github.com/teambit/bit/pull/5251
+  const manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
+  const requiresBuild = pkgRequiresBuild(manifest, filesIndex)
   return {
     local: true as const,
     filesIndex,
@@ -134,27 +123,14 @@ async function fileStat (filePath: string): Promise<{ filePath: string, stat: St
   }
 }
 
-async function fetchPackageFilesFromDir (
-  dir: string,
-  opts: FetchFromDirOpts
-) {
+async function fetchPackageFilesFromDir (dir: string) {
   const files = await packlist(dir)
   const filesIndex: Record<string, string> = Object.fromEntries(files.map((file) => [file, path.join(dir, file)]))
-  let manifest: DependencyManifest | undefined
-  if (opts.readManifest) {
-    // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
-    // Safe read was added to support the Bit workspace in which the components have no package.json files.
-    // Related PR in Bit: https://github.com/teambit/bit/pull/5251
-    manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
-  }
-  const requiresBuild = Boolean(
-    manifest?.scripts != null && (
-      Boolean(manifest.scripts.preinstall) ||
-      Boolean(manifest.scripts.install) ||
-      Boolean(manifest.scripts.postinstall)
-    ) ||
-    filesIncludeInstallScripts(filesIndex)
-  )
+  // In a regular pnpm workspace it will probably never happen that a dependency has no package.json file.
+  // Safe read was added to support the Bit workspace in which the components have no package.json files.
+  // Related PR in Bit: https://github.com/teambit/bit/pull/5251
+  const manifest = await safeReadProjectManifestOnly(dir) as DependencyManifest ?? undefined
+  const requiresBuild = pkgRequiresBuild(manifest, filesIndex)
   return {
     local: true as const,
     filesIndex,

@@ -12,6 +12,9 @@
     {
       "path": "../../__utils__/test-fixtures"
     },
+    {
+      "path": "../../exec/pkg-requires-build"
+    },
     {
       "path": "../../fs/packlist"
     },

@@ -15,7 +15,7 @@
     "@pnpm/logger": "^5.0.0"
   },
   "dependencies": {
-    "@pnpm/exec.files-include-install-scripts": "workspace:*",
+    "@pnpm/exec.pkg-requires-build": "workspace:*",
     "@pnpm/fetcher-base": "workspace:*",
     "@pnpm/fs.indexed-pkg-importer": "workspace:*",
     "@pnpm/store-controller-types": "workspace:*",

@@ -1,6 +1,5 @@
-import { promises as fs, readFileSync } from 'fs'
+import { promises as fs } from 'fs'
 import path from 'path'
-import { filesIncludeInstallScripts } from '@pnpm/exec.files-include-install-scripts'
 import {
   type CafsLocker,
   createCafs,
@@ -35,7 +34,7 @@ export function createPackageImporterAsync (
   const gfm = getFlatMap.bind(null, opts.cafsDir)
   return async (to, opts) => {
     const { filesMap, isBuilt } = gfm(opts.filesResponse, opts.sideEffectsCacheKey)
-    const willBeBuilt = !isBuilt && (opts.requiresBuild ?? pkgRequiresBuild(filesMap))
+    const willBeBuilt = !isBuilt && opts.requiresBuild
     const pkgImportMethod = willBeBuilt
       ? 'clone-or-copy'
       : (opts.filesResponse.packageImportMethod ?? packageImportMethod)
@@ -65,7 +64,7 @@ function createPackageImporter (
   const gfm = getFlatMap.bind(null, opts.cafsDir)
   return (to, opts) => {
     const { filesMap, isBuilt } = gfm(opts.filesResponse, opts.sideEffectsCacheKey)
-    const willBeBuilt = !isBuilt && (opts.requiresBuild ?? pkgRequiresBuild(filesMap))
+    const willBeBuilt = !isBuilt && opts.requiresBuild
     const pkgImportMethod = willBeBuilt
       ? 'clone-or-copy'
       : (opts.filesResponse.packageImportMethod ?? packageImportMethod)
@@ -131,16 +130,3 @@ export function createCafsStore (
     },
   }
 }
-
-function pkgRequiresBuild (filesMap: Record<string, string>) {
-  return filesIncludeInstallScripts(filesMap) ||
-    filesMap['package.json'] && pkgJsonHasInstallScripts(filesMap['package.json'])
-}
-
-function pkgJsonHasInstallScripts (file: string): boolean {
-  const pkgJson = JSON.parse(readFileSync(file, 'utf8'))
-  if (!pkgJson.scripts) return false
-  return Boolean(pkgJson.scripts.preinstall) ||
-    Boolean(pkgJson.scripts.install) ||
-    Boolean(pkgJson.scripts.postinstall)
-}
@@ -13,7 +13,7 @@
       "path": "../../__utils__/prepare"
     },
     {
-      "path": "../../exec/files-include-install-scripts"
+      "path": "../../exec/pkg-requires-build"
     },
     {
       "path": "../../fetching/fetcher-base"

@@ -35,7 +35,7 @@
     "@pnpm/cafs-types": "workspace:*",
     "@pnpm/create-cafs-store": "workspace:*",
     "@pnpm/error": "workspace:*",
-    "@pnpm/exec.files-include-install-scripts": "1.0.0",
+    "@pnpm/exec.pkg-requires-build": "workspace:*",
     "@pnpm/fs.hard-link-dir": "workspace:*",
     "@pnpm/graceful-fs": "workspace:*",
     "@pnpm/store.cafs": "workspace:*",
-Original file line number
+Diff line change
@@ Expand Up / @@ -12,6 +12,9 @@ @@
         {
           "path": "../../__utils__/test-fixtures"
         },
+        {
+          "path": "../../exec/pkg-requires-build"
+        },
         {
           "path": "../../fs/packlist"
         },
@@ Expand Down @@