• .changeset/config.json: Language not supported
• .prettierignore: Language not supported
• package.json: Language not supported

src/target.ts (1)

65-78: Potential issue with destructuring assignment.

The destructuring assignment in line 77 assumes that the target triple always includes a vendor field, but there may be cases where it's omitted.

#!/bin/bash
# Look for target triple examples in the codebase or documentation
rg --type=ts 'triple|target(-|_)triple' -g '!node_modules/' -g '!*.json' -g '!*.lock'

src/types.ts (1)

1-17: Well-structured Napi interface with optional properties.

The Napi interface is designed with many optional properties, providing flexibility for different configuration scenarios. However, consider documenting each property with JSDoc comments to improve developer understanding.

 export interface Napi {
+  /** Optional name for the binary file */
   binaryName?: string
+  /** Optional package name for resolution */
   packageName?: string
   package?: {
     name: string
   }
+  /** List of supported target platforms */
   targets?: string[]
   triples?: {
+    /** Whether to include default triples */
     defaults?: boolean
+    /** Additional custom triples to support */
     additional?: string[]
   }
   wasm?: {
     browser?: {
+      /** Whether to enable filesystem in browser WASM */
       fs?: boolean
     }
   }
 }

src/cli.ts (1)

1-5: CLI implementation looks good, but consider adding error handling

The CLI implementation follows Node.js best practices with the shebang line and proper module imports. However, there's no error handling for the Promise returned by checkAndPreparePackage.

-void checkAndPreparePackage(process.argv[2])
+checkAndPreparePackage(process.argv[2]).catch(err => {
+  console.error('Error during package preparation:', err)
+  process.exit(1)
+})

README.md (1)

16-16: Minor punctuation glitch after the project list.

Consider inserting a full stop ( . ) before “for” to avoid a run‑on sentence and satisfy spell‑/grammar‑checks.

- ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver]
+ ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

src/helpers.ts (2)

20-36: Use built‑in fs.rmSync for safer, atomic directory removal.

fs.rmdirSync is deprecated and fails on non‑empty dirs in newer Node versions. You can simplify and make the function more robust (and Windows‑friendly) with:

-export function removeRecursive(dir: string) {
-  for (const entry of fs.readdirSync(dir)) {
-    const entryPath = path.join(dir, entry)
-    let stats: Stats
-    try {
-      stats = fs.lstatSync(entryPath)
-    } catch {
-      continue // Guard against https://github.com/nodejs/node/issues/4760
-    }
-    if (stats.isDirectory()) {
-      removeRecursive(entryPath)
-    } else {
-      fs.unlinkSync(entryPath)
-    }
-  }
-  fs.rmdirSync(dir)
-}
+export function removeRecursive(dir: string) {
+  // Node ≥ 14.14
+  fs.rmSync(dir, { recursive: true, force: true })
+}

Keeps Node 12 support? Detect version and fall back if necessary.

---

155-239: Return type can be simplified to always string[]

getNapiNativeTarget currently returns string | string[] | [], forcing callers to branch.
Since callers often normalise to an array anyway (getNapiNativeTargets does), consider:

-export function getNapiNativeTarget(): string[] | string {
+export function getNapiNativeTarget(): string[] {
...
-        return 'android-arm64'
+        return ['android-arm64']
...
-  return []
+  return []

This reduces API friction and type assertions.

src/index.ts (2)

23-43: fetch() lacks robust redirect & error handling

The helper handles only 301/302 redirects and assumes an absolute Location header. Consider:
• Supporting 303, 307, 308 status codes.
• Resolving relative Location headers via new URL(location, url).
• Aborting the initial request when following a redirect to free sockets.
• Imposing a maximum redirect depth to avoid loops.

These changes improve resilience when registry mirrors are configured.

---

45-70: Minor: tar extraction assumes small archives and loads entire buffer

extractFileFromTarGzip unzips the whole archive into memory and iterates synchronously. For large native bundles (≥ tens of MB) this can blow RAM in constrained environments.

If performance becomes a concern, consider a streaming approach (zlib.createGunzip() + incremental tar parser such as tar-stream), aborting once the desired entry is found. Not urgent but worth tracking.

src/constants.ts (1)

1-11: Good centralization of constants

Creating a dedicated constants file improves maintainability by providing a single source of truth for configuration values.

Consider using a consistent module system approach. The file mixes ESM-style imports with CommonJS-style require(). For better ESM compatibility, you could use:

-export const { name, version } = require(
-  path.resolve(__dirname, '../package.json'),
-) as PackageJson
+import { fileURLToPath } from 'node:url'
+import { dirname } from 'node:path'
+
+const __filename = fileURLToPath(import.meta.url)
+const __dirname = dirname(__filename)
+
+export const { name, version } = JSON.parse(
+  readFileSync(path.resolve(__dirname, '../package.json'), 'utf8')
+) as PackageJson

Don't forget to add the import for readFileSync from the fs module.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/index.ts (2)

80-110: ⚠️ Potential issue

Fix potential command injection vulnerability in installUsingNPM

The current implementation uses execSync with template literals for pkg and version values, which could be vulnerable to command injection if these values are malicious. Although inputs typically come from package.json, it's better to implement defense-in-depth security measures.

Replace with spawnSync to avoid shell interpolation, improving security and Windows compatibility:

-import { execSync } from 'node:child_process'
+import { execSync, spawnSync } from 'node:child_process'
...
-    execSync(
-      `npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`,
-      { cwd: installDir, stdio: 'pipe', env },
-    )
+    const { status, error } = spawnSync(
+      'npm',
+      [
+        'install',
+        '--loglevel=error',
+        '--prefer-offline',
+        '--no-audit',
+        '--progress=false',
+        `${pkg}@${version}`,
+      ],
+      { cwd: installDir, stdio: 'pipe', env },
+    )
+    if (status !== 0) throw error ?? new Error('npm install failed')

---

238-252: ⚠️ Potential issue

Incorrect version is used when fetching/installing the binary package

Inside the loop you derive pkg at L199, yet the subsequent calls to both installUsingNPM() (L238-L243) and downloadDirectlyFromNPM() (L247-L252) forward the host package version (version) instead of the version declared in optionalDependencies![pkg].

When checkVersion is false (which you explicitly support), this silently fetches the wrong tarball or npm install target, leading to checksum mismatches or runtime ABI errors.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(name, pkg, optionalDependencies![pkg]!, subpath, nodePath)
...
-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(pkg, optionalDependencies![pkg]!, subpath, nodePath)

src/index.ts (4)

198-208: Use consistent object naming scheme in checkAndPreparePackage

The code derives pkg based on the target and packageName at L199, but the naming is inconsistent with how the native package naming works internally from what I can see from the rest of the codebase.

For better readability and consistency, ensure that the package name construction follows a consistent pattern throughout the codebase. Consider extracting this into a helper function if it's used in multiple places.

---

202-204: Consider explicit null check instead of non-null assertion

While the code includes a comment indicating that optionalDependencies is ensured to exist by getNapiInfoFromPackageJson, defensive programming would benefit from an explicit check rather than relying on non-null assertions.

-    /** @see {optionalDependencies} has been ensured to exist with @see {getNapiInfoFromPackageJson} */
-    if (!optionalDependencies![pkg]) {
+    // optionalDependencies is ensured to exist by getNapiInfoFromPackageJson
+    if (!optionalDependencies?.[pkg]) {
       continue
     }

---

216-222: Improve readability of multi-line error message

The multi-line error message could be improved for better readability using a template literal.

-      console.error(`${LOG_PREFIX}Failed to find package "${pkg}" on the file system
-
-This can happen if you use the "--no-optional" flag. The "optionalDependencies"
-package.json feature is used by ${name} to install the correct napi binary
-for your current platform. This install script will now attempt to work around
-this. If that fails, you need to remove the "--no-optional" flag to use ${name}.
-`)
+      console.error(`${LOG_PREFIX}Failed to find package "${pkg}" on the file system
+
+This can happen if you use the "--no-optional" flag. The "optionalDependencies"
+package.json feature is used by ${name} to install the correct napi binary
+for your current platform. This install script will now attempt to work around
+this. If that fails, you need to remove the "--no-optional" flag to use ${name}.`)

---

173-177: Mark exported function as stable API

The main exported function checkAndPreparePackage is a public API and should have a JSDoc comment describing its purpose, parameters, and return value, especially since it's likely to be used by external consumers.

// eslint-disable-next-line sonarjs/cognitive-complexity
+/**
+ * Checks and prepares native packages for a given package
+ * @param packageNameOrPackageJson - The package name or package.json object
+ * @param checkVersion - Whether to check version consistency
+ */
export async function checkAndPreparePackage(
  packageNameOrPackageJson: PackageJson | string,
  checkVersion?: boolean,
) {

README.md (2)

16-16: Add a period at the end of the description

The description sentence is missing a period at the end.

-The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver]
+The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

---

95-95: Add more detailed example code

The current example is very minimal. Consider adding a more comprehensive example showing both the CLI and API usage scenarios, including handling errors and working with optional arguments.

-checkAndPreparePackage('unrs-resolver' /* <napi-package-name> */)
+// Basic usage
+try {
+  await checkAndPreparePackage('unrs-resolver')
+  console.log('Native dependencies prepared successfully')
+} catch (error) {
+  console.error('Failed to prepare native dependencies:', error)
+}
+
+// With version checking
+try {
+  await checkAndPreparePackage('unrs-resolver', true)
+  console.log('Native dependencies prepared successfully with version checking')
+} catch (error) {
+  console.error('Failed to prepare native dependencies:', error)
+}
+
+// With package.json object
+import fs from 'node:fs'
+
+const packageJson = JSON.parse(fs.readFileSync('path/to/package.json', 'utf8'))
+try {
+  await checkAndPreparePackage(packageJson)
+  console.log('Native dependencies prepared successfully')
+} catch (error) {
+  console.error('Failed to prepare native dependencies:', error)
+}

package.json (1)

2-4: Consider adding a description that includes the package renaming in comments

Adding a comment about the package renaming history could be helpful for users familiar with the old name.

{
  "name": "napi-postinstall",
  "version": "0.0.0",
+  // Renamed from native-postinstall
  "type": "commonjs",

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

README.md (1)

76-83: PackageJson interface still incomplete

Previous reviews already flagged that optionalDependencies and napi fields are missing. The example type should reflect the actual structure or newcomers will copy an invalid shape.

src/helpers.ts (1)

38-41: Path truncation bug when resolving downloaded binary sub‑paths

Using path.basename(subpath) drops intermediate directories (darwin/x64/native.node → native.node) and can cause name clashes between different targets.

-return path.join(pkgLibDir, `${path.basename(subpath)}`)
+// Preserve the relative structure to avoid collisions
+return path.join(pkgLibDir, subpath)

src/index.ts (3)

106-109: ⚠️ Potential issue

Fix command injection vulnerability in npm install execution

The current implementation uses string concatenation with execSync, which makes it vulnerable to command injection if pkg or version contains shell metacharacters.

Replace with spawnSync to avoid shell interpretation:

-    execSync(
-      `npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`,
-      { cwd: installDir, stdio: 'pipe', env },
-    )
+    const { status, error } = spawnSync(
+      'npm',
+      [
+        'install',
+        '--loglevel=error',
+        '--prefer-offline',
+        '--no-audit',
+        '--progress=false',
+        `${pkg}@${version}`,
+      ],
+      { cwd: installDir, stdio: 'pipe', env },
+    )
+    if (status !== 0) throw error ?? new Error('npm install failed')

---

238-240: ⚠️ Potential issue

Use correct package version from optionalDependencies

You're using the host package version (version) instead of the version specified in optionalDependencies for the specific package.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(name, pkg, optionalDependencies![pkg]!, subpath, nodePath)

---

249-252: ⚠️ Potential issue

Use correct package version from optionalDependencies

You're using the host package version (version) instead of the version specified in optionalDependencies for the specific package.

-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(pkg, optionalDependencies![pkg]!, subpath, nodePath)

README.md (1)

14-17: Add a missing comma for readability

The sentence runs together after the inline link list; inserting a comma improves clarity:

-ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].
+ecosystem packages, like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

src/helpers.ts (1)

20-36: Use modern, atomic removal API

fs.rmdirSync(dir) is deprecated and removed in recent Node versions.
fs.rmSync(dir, { recursive: true, force: true }) is the recommended replacement and eliminates the manual recursion.

-export function removeRecursive(dir: string) {
-  for (const entry of fs.readdirSync(dir)) {
-    const entryPath = path.join(dir, entry)
-    let stats: Stats
-    try {
-      stats = fs.lstatSync(entryPath)
-    } catch {
-      continue // Guard against https://github.com/nodejs/node/issues/4760
-    }
-    if (stats.isDirectory()) {
-      removeRecursive(entryPath)
-    } else {
-      fs.unlinkSync(entryPath)
-    }
-  }
-  fs.rmdirSync(dir)
-}
+export function removeRecursive(dir: string) {
+  fs.rmSync(dir, { recursive: true, force: true })
+}

src/index.ts (6)

158-158: Improve URL construction for scoped packages

The URL construction for npm packages has different logic for scoped vs. non-scoped packages embedded in a single line, making it harder to understand.

-  const url = `${getGlobalNpmRegistry()}${pkg}/-/${pkg.startsWith('@') ? pkg.split('/')[1] : pkg}-${version}.tgz`
+  const packageName = pkg.startsWith('@') ? pkg.split('/')[1] : pkg
+  const url = `${getGlobalNpmRegistry()}${pkg}/-/${packageName}-${version}.tgz`

---

50-51: Use imported getErrorMessage helper

You've imported the getErrorMessage helper function but aren't using it consistently in the code.

-      `Invalid gzip data in archive: ${(err as Error | undefined)?.message || String(err)}`,
+      `Invalid gzip data in archive: ${getErrorMessage(err)}`,

---

23-43: Consider adding retry logic to fetch function

The fetch function handles redirects but doesn't have retry logic for transient network errors, which can be common when downloading from npm.

Consider adding retry logic with exponential backoff for more robust downloads:

+function wait(ms: number) {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
-function fetch(url: string) {
+async function fetch(url: string, retries = 3, backoff = 300) {
+  try {
+    return await fetchOnce(url);
+  } catch (err) {
+    if (retries <= 0) throw err;
+    await wait(backoff);
+    return fetch(url, retries - 1, backoff * 2);
+  }
+}
+
+function fetchOnce(url: string) {
  return new Promise<Buffer>((resolve, reject) => {
    https
      .get(url, res => {
        if (
          (res.statusCode === 301 || res.statusCode === 302) &&
          res.headers.location
        ) {
-          fetch(res.headers.location).then(resolve, reject)
+          fetchOnce(res.headers.location).then(resolve, reject)
          return
        }
        if (res.statusCode !== 200) {
          return reject(new Error(`Server responded with ${res.statusCode}`))
        }
        const chunks: Buffer[] = []
        res.on('data', (chunk: Buffer) => chunks.push(chunk))
        res.on('end', () => resolve(Buffer.concat(chunks)))
      })
      .on('error', reject)
  })
}

---

173-258: Reduce cognitive complexity of checkAndPreparePackage function

The function is marked with an ESLint disable comment for cognitive complexity. Consider breaking it down into smaller, more focused functions.

You could extract the target handling loop into a separate function:

-// eslint-disable-next-line sonarjs/cognitive-complexity
 export async function checkAndPreparePackage(
   packageNameOrPackageJson: PackageJson | string,
   checkVersion?: boolean,
 ) {
   const packageJson =
     typeof packageNameOrPackageJson === 'string'
       ? (require(packageNameOrPackageJson + '/package.json') as PackageJson)
       : packageNameOrPackageJson

   const { napi, version: napiVersion } = getNapiInfoFromPackageJson(
     packageJson,
     checkVersion,
   )

   const { name, version, optionalDependencies } = packageJson

   if (checkVersion && version !== napiVersion) {
     throw new Error(
       `Inconsistent package versions found for \`${name}\` v${version} vs \`${napi.packageName}\` v${napiVersion}.`,
     )
   }

   const targets = getNapiNativeTargets()
+   
+   await findAndPrepareTarget(name, napi, version, optionalDependencies!, targets)
+ }
+ 
+ async function findAndPrepareTarget(
+   name: string,
+   napi: NonNullable<PackageJson['napi']>,
+   version: string,
+   optionalDependencies: NonNullable<PackageJson['optionalDependencies']>,
+   targets: string[]
+ ) {
   for (const target of targets) {
     const pkg = `${napi.packageName}-${target}`

-    /** @see {optionalDependencies} has been ensured to exist with @see {getNapiInfoFromPackageJson} */
-    if (!optionalDependencies![pkg]) {
+    if (!optionalDependencies[pkg]) {
       continue
     }

     const binaryPrefix = napi.binaryName ? `${napi.binaryName}.` : ''
     const subpath = `${binaryPrefix}${target}.node`

     try {
       // First check for the binary package from our "optionalDependencies". This
       // package should have been installed alongside this package at install time.
       require.resolve(`${pkg}/${subpath}`)
       break
     } catch {
       if (!isNpm()) {
         console.error(`${LOG_PREFIX}Failed to find package "${pkg}" on the file system

This can happen if you use the "--no-optional" flag. The "optionalDependencies"
package.json feature is used by ${name} to install the correct napi binary
for your current platform. This install script will now attempt to work around
this. If that fails, you need to remove the "--no-optional" flag to use ${name}.
`)
       }

       // If that didn't work, then someone probably installed <napi> with the
       // "--no-optional" flag. Attempt to compensate for this by downloading the
       // package using a nested call to "npm" instead.
       //
       // THIS MAY NOT WORK. Package installation uses "optionalDependencies" for
       // a reason: manually downloading the package has a lot of obscure edge
       // cases that fail because people have customized their environment in
       // some strange way that breaks downloading. This code path is just here
       // to be helpful but it's not the supported way of installing <napi>.
       const nodePath = downloadedNodePath(name, subpath)
       try {
         console.error(
           `${LOG_PREFIX}Trying to install package "${pkg}" using npm`,
         )
-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(name, pkg, optionalDependencies[pkg]!, subpath, nodePath)
         break
       } catch (err) {
         console.error(
           `${LOG_PREFIX}Failed to install package "${pkg}" using npm: ${getErrorMessage(err)}`,
         )

         // If that didn't also work, then something is likely wrong with the "npm"
         // command. Attempt to compensate for this by manually downloading the
         // package from the npm registry over HTTP as a last resort.
         try {
-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(pkg, optionalDependencies[pkg]!, subpath, nodePath)
           break
         } catch {
           throw new Error(`Failed to install package "${pkg}"`)
         }
       }
     }
   }
 }

---

202-202: Avoid non-null assertion operator

You're using the non-null assertion operator (!) which might not be safe in all cases.

-    if (!optionalDependencies![pkg]) {
+    // We ensured optionalDependencies exists in getNapiInfoFromPackageJson
+    if (!optionalDependencies?.[pkg]) {

---

216-222: Improve error message readability

The multi-line template literal is concatenated with the log prefix in a way that creates an odd line break after the first line.

-        console.error(`${LOG_PREFIX}Failed to find package "${pkg}" on the file system
-
-This can happen if you use the "--no-optional" flag. The "optionalDependencies"
-package.json feature is used by ${name} to install the correct napi binary
-for your current platform. This install script will now attempt to work around
-this. If that fails, you need to remove the "--no-optional" flag to use ${name}.
-`)
+        console.error(
+          `${LOG_PREFIX}Failed to find package "${pkg}" on the file system\n` +
+          `\n` +
+          `This can happen if you use the "--no-optional" flag. The "optionalDependencies"\n` +
+          `package.json feature is used by ${name} to install the correct napi binary\n` +
+          `for your current platform. This install script will now attempt to work around\n` +
+          `this. If that fails, you need to remove the "--no-optional" flag to use ${name}.`
+        )

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/helpers.ts (1)

38-41: ⚠️ Potential issue

Path‑truncation bug previously reported is still present

path.basename(subpath) removes nested directories so "darwin/x64/native.node" collapses to native.node, risking clashes across archs.

This is the exact issue flagged in a past review; please address it:

-return path.join(pkgLibDir, `${path.basename(subpath)}`)
+// preserve the relative directory structure
+return path.join(pkgLibDir, subpath)

src/index.ts (2)

106-109: ⚠️ Potential issue

Command‑injection & Windows quoting issues in installUsingNPM

Building a shell string with ${pkg}@${version} re‑introduces the injection vector previously highlighted. A malicious (or simply malformed) version like 1.2.3 && rm -rf / compromises the host.

Please switch to spawnSync('npm', [...args]), which bypasses the shell:

-    execSync(
-      `npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`,
-      { cwd: installDir, stdio: 'pipe', env },
-    )
+    const { status, error } = spawnSync(
+      'npm',
+      [
+        'install',
+        '--loglevel=error',
+        '--prefer-offline',
+        '--no-audit',
+        '--progress=false',
+        `${pkg}@${version}`,
+      ],
+      { cwd: installDir, stdio: 'pipe', env },
+    )
+    if (status !== 0) throw error ?? new Error('npm install failed')

---

238-241: ⚠️ Potential issue

Wrong package version passed to fallback installers

installUsingNPM(name, pkg, version, …) forwards the host package version (package.json.version) instead of the binary’s declared version in optionalDependencies[pkg].

Same bug occurs for downloadDirectlyFromNPM at L250‑251.
This installs or downloads the wrong tarball, leading to checksum/ABI mismatch.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(
+          name,
+          pkg,
+          optionalDependencies![pkg]!, // guaranteed by earlier guard
+          subpath,
+          nodePath,
+        )

and

-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(
+            pkg,
+            optionalDependencies![pkg]!,
+            subpath,
+            nodePath,
+          )

package.json (1)

10-13: Node 12 is EOL – consider dropping it from the engines range

"node": "^12.20.0 || ^14.18.0 || >=16.0.0" still lists Node 12 which left active LTS in Apr 2022.
Continuing support complicates your code (e.g. process.report is Node >= 12. Makes musl detection brittle). If you no longer test against 12, update the range to >=14.18 or higher.

src/helpers.ts (1)

9-18: Honor npm_config_registry before shelling out

getGlobalNpmRegistry() ignores the already‑parsed environment variable npm_config_registry and always spawns npm config get, incurring ~40 ms overhead on every call and failing when npm isn’t in PATH.

 export function getGlobalNpmRegistry() {
-  try {
+  if (process.env.npm_config_registry) {
+    const r = process.env.npm_config_registry
+    return r.endsWith('/') ? r : `${r}/`
+  }
+  try {

src/index.ts (1)

150-170: Consider streaming the tarball to disk instead of buffering

fetch() loads the entire .tgz into memory and extractFileFromTarGzip decompresses it again in‑memory, doubling peak RAM. Large prebuilds (e.g. TensorFlow) can OOM small CI agents.

For a future improvement, pipe the response into zlib.createUnzip() and parse the tar headers on the fly.

package.json (2)

78-79: ⚠️ Potential issue

Self‑referencing devDependency can break local installs

"napi-postinstall": "file:",

Adding the package itself as a file: devDependency causes an extra node_modules/napi-postinstall symlink that may confuse tools (e.g. Jest’s module resolver) and bloats the tarball.
Unless you rely on this for an e2e test, please remove the entry or gate it behind a workspace alias.

-    "napi-postinstall": "file:",

---

14-16: 🛠️ Refactor suggestion

🏷️ bin entry may ship an unusable executable without a shebang

"napi-postinstall": "./lib/cli.js" expects lib/cli.js to be directly runnable by the OS.
Please double‑check that the compiled file starts with a POSIX shebang (#!/usr/bin/env node) and has the executable bit set during npm pack, otherwise npx napi-postinstall will fail on *nix systems.

README.md (1)

79-82: Incomplete PackageJson interface in README
The PackageJson interface examples in the README omit the optionalDependencies and napi fields present in the actual API. It should include:

 export interface PackageJson {
   name: string
   version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+  }
 }

README.md (2)

16-16: Fix run‑on sentence in project description
The description sentence is currently a run‑on. Consider splitting into two sentences or using a semicolon. For example:

-The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation of the [`node-install`][node-install] functionality...
+The `postinstall` script helper for handling native bindings in legacy `npm` versions. This is a reimplementation of the [`node-install`][node-install] functionality...

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

---

59-59: Clarify CLI usage placeholder
Using #<napi-package-name> in a shell snippet may be misleading because # starts a comment. Prefer a placeholder without the hash:

-napi-postinstall unrs-resolver #<napi-package-name>
+napi-postinstall <napi-package-name>

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

src/helpers.ts (1)

38-41: ⚠️ Potential issue

Path‑truncation bug is still present
path.basename(subpath) discards intermediate directories, so a binary like darwin/x64/native.node flattens to native.node, causing collisions between targets and breaking require.resolve(). Preserve the relative directory structure instead.

-  return path.join(pkgLibDir, `${path.basename(subpath)}`)
+  // Keep the original sub‑directory layout to avoid name clashes
+  return path.join(pkgLibDir, subpath)

src/index.ts (2)

106-109: 🛠️ Refactor suggestion

Shell‑injection & Windows‑portability risk in npm install call
Building an execSync command string allows malicious pkg/version values (they come from package.json but defence‑in‑depth is recommended) and relies on a shell being present. Prefer spawnSync('npm', [...args]).

-import { execSync } from 'node:child_process'
+import { spawnSync } from 'node:child_process'
…
-    execSync(
-      `npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`,
-      { cwd: installDir, stdio: 'pipe', env },
-    )
+    const { status, error } = spawnSync(
+      'npm',
+      [
+        'install',
+        '--loglevel=error',
+        '--prefer-offline',
+        '--no-audit',
+        '--progress=false',
+        `${pkg}@${version}`,
+      ],
+      { cwd: installDir, stdio: 'pipe', env },
+    )
+    if (status !== 0) throw error ?? new Error('npm install failed')

---

238-259: ⚠️ Potential issue

Incorrect package version is forwarded during fallback installation
installUsingNPM() and downloadDirectlyFromNPM() receive the host package’s version (${version}) instead of the binary package’s version declared in optionalDependencies![pkg]. This installs/ downloads the wrong tarball and can lead to runtime ABI errors.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(name, pkg, optionalDependencies![pkg]!, subpath, nodePath)
…
-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(pkg, optionalDependencies![pkg]!, subpath, nodePath)

README.md (1)

78-83: PackageJson type is still incomplete in documentation
optionalDependencies and the napi field are essential for this library yet are omitted from the README type example, which may mislead users.

 export interface PackageJson {
   name: string
   version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+    triples?: { additional?: string[] }
+  }
 }

src/index.ts (1)

238-259: Consider trying the next target instead of hard‑failing
If installation for the first target (darwin-universal, linux-musl, …) fails, the loop aborts with an error even though a secondary target in napi.targets might already be present locally. Re‑ordering or continuing the loop after a failed attempt will improve robustness.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

README.md (1)

79-82: Extend the PackageJson interface to align with implementation
The PackageJson interface omits properties used in your code (e.g., optionalDependencies, napi). Please include them so the docs match the actual types.

 export interface PackageJson {
   name: string
   version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+  }
 }

src/helpers.ts (1)

38-41: ⚠️ Potential issue

Path‑truncation bug when resolving downloaded binaries
(duplicate of a past review)

path.basename(subpath) strips intermediate directories, so
"darwin/x64/native.node" collapses to native.node, causing collisions.

-return path.join(pkgLibDir, `${path.basename(subpath)}`)
+// preserve directory structure relative to the package root
+return path.join(pkgLibDir, subpath)

src/index.ts (2)

106-109: Shell‑string construction in execSync enables command‑injection
(same as previously flagged)

The interpolated shell string can be broken by malicious or malformed pkg/version values.
Please switch to spawnSync('npm', [...args]) to avoid invoking the shell.

---

239-251: ⚠️ Potential issue

Incorrect package version is used for fallback installation

version refers to the host package (${name}) version, not the binary package in optionalDependencies.
Passing it to both installUsingNPM and downloadDirectlyFromNPM fetches the wrong tarball, leading to checksum or ABI mismatches.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(
+          name,
+          pkg,
+          optionalDependencies![pkg]!, // use the binary’s declared version
+          subpath,
+          nodePath,
+        )
…
-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(
+            pkg,
+            optionalDependencies![pkg]!, // same here
+            subpath,
+            nodePath,
+          )

lib/types.js (1)

1-1: Redundant "use strict" directive
ES modules are strict by default; you can safely remove this line.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/cli.js (1)

2-2: Remove redundant "use strict" directive
CommonJS modules in Node.js are strict by default; this directive can be removed.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-2: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/constants.js (1)

1-1: Remove redundant "use strict" directive
CommonJS modules are strict by default in Node.js; you can safely remove this line.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

README.md (1)

16-16: Fix run-on sentence in introduction
The sentence here is a bit long and should be split for clarity. For example:

- The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation...
+ The `postinstall` script helper for handling native bindings in legacy `npm` versions. This is a reimplementation...

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

lib/target.js (1)

1-1: Remove redundant "use strict" directive

Node treats every CommonJS module as strict‑mode by default, so the leading directive is unnecessary noise.
Removing it prevents the Biome warning without changing behaviour.

-"use strict";
+// Strict mode is implicit in CommonJS modules

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/index.js (2)

1-1: Drop redundant "use strict"

Same reasoning as in lib/target.js; this keeps the compiled output clean and silences Biome.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

---

31-37: Use gunzipSync instead of unzipSync for npm tarballs

npm packages are *.tgz (gzip) archives; zlib.unzipSync works but incurs an
extra format‑detection step and fails on some edge‑cases.
gunzipSync is purpose‑built for gzip and slightly faster.

- buffer = node_zlib_1.default.unzipSync(buffer);
+ buffer = node_zlib_1.default.gunzipSync(buffer);

package.json (1)

78-79: Self‑reference in devDependencies is still risky

"napi-postinstall": "file:." works for local development but:

1. Confuses some package‑managers (pnpm ≤7, older Yarn) during hoisting.
2. Bloats the tarball (npm pack embeds the entire project twice).

Consider replacing with a workspace version ("workspace:*") or removing the
entry entirely—tests can import from src/lib directly.

src/helpers.ts (1)

9-18: Honor npm_config_registry environment variable first

npm config get registry spawns a process and fails when npm isn’t in PATH
(e.g. during pnpm or Yarn installs). A simpler, faster fallback is the
well‑known env var.

 export function getGlobalNpmRegistry() {
-  try {
+  const registryEnv = process.env.npm_config_registry
+  if (registryEnv) {
+    return registryEnv.endsWith('/') ? registryEnv : `${registryEnv}/`
+  }
+  try {

lib/helpers.js (2)

1-1: Remove the redundant "use strict" directive

JavaScript modules are automatically executed in strict mode. Keeping the directive here is harmless but clutters the top of every compiled file and is flagged by Biome.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

---

94-99: Fallback path for ldd lookup may miss common location

Some distributions place ldd under /usr/bin, others under /bin.
If /usr/bin/ldd is absent the function returns null even though the file exists elsewhere, degrading musl detection accuracy.

Consider trying the alternative path before giving up:

-    return node_fs_1.default.readFileSync('/usr/bin/ldd', 'utf8').includes('musl');
+    for (const p of ['/usr/bin/ldd', '/bin/ldd']) {
+      try {
+        if (node_fs_1.default.readFileSync(p, 'utf8').includes('musl')) return true;
+      } catch {}
+    }
+    return false;

src/index.ts (1)

23-43: fetch() has no redirect limit and can recurse indefinitely

If the registry or a proxy misbehaves and produces a redirect loop, the current implementation will recurse until the call stack overflows.

Add a hop counter (e.g. maxRedirects = 5) or switch to node-fetch/undici which handle this internally.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/cli.js (1)

5-5: Add error handling for the CLI invocation

Currently, unhandled promise rejections from checkAndPreparePackage won't set a non-zero exit code, which can mask failures in scripts. Wrap the call to catch errors and exit with code 1.

Apply this diff:

- void (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]));
+ (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]))
+  .catch(err => {
+    console.error(err);
+    process.exit(1);
+  });

README.md (1)

79-82: Incomplete PackageJson interface in README

The PackageJson interface in the README is incomplete compared to the actual implementation. It's missing the optionalDependencies and napi properties that are crucial for the package functionality.

export interface PackageJson {
  name: string
  version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+  }
}

lib/target.js (1)

31-33: ⚠️ Potential issue

Fix the eabi suffix normalization to avoid suffix duplication

Currently, the code will incorrectly handle target triples that end with "gnueabi" by splitting "gnu" and "eabi", causing incorrect platform detection.

- const triple = rawTriple.endsWith('eabi')
-     ? `${rawTriple.slice(0, -4)}-eabi`
+ const triple = rawTriple.endsWith('-eabi')
+     ? `${rawTriple.slice(0, -5)}-eabi`
      : rawTriple;

lib/index.js (2)

12-30: ⚠️ Potential issue

Fix redirect handling for HTTP responses

The current redirect handling has two issues:

1. It only handles 301 and 302 status codes, missing 303, 307, and 308 redirects
2. It doesn't properly handle relative URLs in the location header

- if ((res.statusCode === 301 || res.statusCode === 302) &&
-     res.headers.location) {
-   fetch(res.headers.location).then(resolve, reject);
+ const redirectCodes = [301, 302, 303, 307, 308];
+ if (redirectCodes.includes(res.statusCode) && res.headers.location) {
+   const nextUrl = res.headers.location.startsWith('http')
+     ? res.headers.location
+     : new URL(res.headers.location, url).toString();
+   fetch(nextUrl).then(resolve, reject);
   return;
 }

---

68-68: ⚠️ Potential issue

Fix shell injection vulnerability in npm install command

The current implementation embeds package name and version directly in a shell command, which presents a security risk if these values contain malicious shell commands.

- (0, node_child_process_1.execSync)(`npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`, { cwd: installDir, stdio: 'pipe', env });
+ (0, node_child_process_1.execFileSync)('npm', [
+   'install',
+   '--loglevel=error',
+   '--prefer-offline',
+   '--no-audit',
+   '--progress=false',
+   `${pkg}@${version}`
+ ], { cwd: installDir, stdio: 'pipe', env });

lib/helpers.js (1)

27-45: 🛠️ Refactor suggestion

Modernize recursive directory removal implementation

The current implementation uses the deprecated fs.rmdirSync() method, which will eventually be removed from Node.js. Modern Node.js versions provide a simpler and more robust solution.

 function removeRecursive(dir) {
-  for (const entry of node_fs_1.default.readdirSync(dir)) {
-    const entryPath = node_path_1.default.join(dir, entry);
-    let stats;
-    try {
-      stats = node_fs_1.default.lstatSync(entryPath);
-    }
-    catch (_a) {
-      continue;
-    }
-    if (stats.isDirectory()) {
-      removeRecursive(entryPath);
-    }
-    else {
-      node_fs_1.default.unlinkSync(entryPath);
-    }
-  }
-  node_fs_1.default.rmdirSync(dir);
+  // `rmSync` is available since Node 14.14 and replaces the
+  // deprecated `rmdirSync` for recursive deletions.
+  node_fs_1.default.rmSync(dir, { recursive: true, force: true });
 }

src/index.ts (3)

106-109: ⚠️ Potential issue

Security Issue: Potential command injection vulnerability.

The use of template literals with execSync can lead to command injection vulnerabilities if pkg or version contains malicious characters. This was previously flagged in a past review but remains unaddressed.

-import { execSync } from 'node:child_process'
+import { execSync, spawnSync } from 'node:child_process'

-    execSync(
-      `npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`,
-      { cwd: installDir, stdio: 'pipe', env },
-    )
+    const { status, error } = spawnSync(
+      'npm',
+      [
+        'install',
+        '--loglevel=error',
+        '--prefer-offline',
+        '--no-audit',
+        '--progress=false',
+        `${pkg}@${version}`,
+      ],
+      { cwd: installDir, stdio: 'pipe', env },
+    )
+    if (status !== 0) throw error ?? new Error('npm install failed')

---

239-239: ⚠️ Potential issue

Incorrect version used when installing the binary package.

This line uses the host package version (version) instead of the version declared in optionalDependencies[pkg]. This can lead to checksum mismatches or runtime ABI errors when checkVersion is false.

-        installUsingNPM(name, pkg, version, subpath, nodePath)
+        installUsingNPM(name, pkg, optionalDependencies![pkg]!, subpath, nodePath)

---

250-250: ⚠️ Potential issue

Incorrect version used when downloading the binary package.

Similarly to the previous issue, this line uses the host package version (version) instead of the version declared in optionalDependencies[pkg].

-          await downloadDirectlyFromNPM(pkg, version, subpath, nodePath)
+          await downloadDirectlyFromNPM(pkg, optionalDependencies![pkg]!, subpath, nodePath)

lib/types.js (1)

1-1: Consider removing redundant "use strict" directive

JavaScript modules are automatically in strict mode, making this directive unnecessary.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

README.md (1)

16-22: Fix run-on sentence in project description

The project description contains a run-on sentence that could be improved for clarity.

-The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].
+The `postinstall` script helper for handling native bindings in legacy `npm` versions. This is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

lib/target.js (1)

1-1: Remove redundant "use strict" directive

ES modules are automatically in strict mode, making this directive unnecessary.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/index.js (1)

1-1: Remove redundant "use strict" directive

ES modules are automatically in strict mode, making this directive unnecessary.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/helpers.js (1)

1-1: Remove redundant "use strict" directive

ES modules are automatically in strict mode, making this directive unnecessary.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

src/index.ts (3)

158-158: Improve package name handling in URL construction.

The logic for extracting the package name from scoped packages is complex and could be clearer. Consider extracting this into a separate utility function for better readability and maintainability.

-  const url = `${getGlobalNpmRegistry()}${pkg}/-/${pkg.startsWith('@') ? pkg.split('/')[1] : pkg}-${version}.tgz`
+  const packageFileName = pkg.startsWith('@') ? pkg.split('/')[1] : pkg
+  const url = `${getGlobalNpmRegistry()}${pkg}/-/${packageFileName}-${version}.tgz`

---

202-204: Avoid non-null assertion operator.

Although the comment indicates optionalDependencies has been verified to exist, using the non-null assertion operator (!) is generally not recommended as it bypasses TypeScript's type checking.

-    if (!optionalDependencies![pkg]) {
+    if (!optionalDependencies?.[pkg]) {
       continue
     }

---

196-198: Consider filtering targets based on the current platform.

The code iterates through all targets returned by getNapiNativeTargets() without filtering for the current platform first. This could be inefficient, especially when there are many targets.

  const targets = getNapiNativeTargets()
+  
+  // Filter targets to prioritize the current platform
+  const currentPlatformTargets = targets.filter(target => 
+    target.includes(process.platform) && target.includes(process.arch)
+  );
+  const targetList = currentPlatformTargets.length > 0 ? currentPlatformTargets : targets;

-  for (const target of targets) {
+  for (const target of targetList) {

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

lib/cli.js (1)

5-5: Add error handling for the CLI invocation

Currently, unhandled promise rejections from checkAndPreparePackage won't set a non-zero exit code, which can mask failures in scripts. Wrap the call to catch errors and exit with code 1.

Apply this diff:

- void (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]));
+ (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]))
+  .catch(err => {
+    console.error(err);
+    process.exit(1);
+  });

lib/constants.js (1)

8-8: Add error handling for package.json loading

The code directly requires package.json without any error handling. If the file is missing or invalid, this will throw an uncaught exception.

-_a = require(node_path_1.default.resolve(__dirname, '../package.json')), exports.name = _a.name, exports.version = _a.version;
+try {
+  _a = require(node_path_1.default.resolve(__dirname, '../package.json'));
+  exports.name = _a.name;
+  exports.version = _a.version;
+} catch (error) {
+  console.error('Failed to load package.json:', error);
+  exports.name = 'napi-postinstall';
+  exports.version = 'unknown';
+}

README.md (1)

79-82: Incomplete PackageJson interface in README

The PackageJson interface in the README is incomplete compared to the actual implementation. It's missing the optionalDependencies and napi properties that are crucial for the package functionality.

export interface PackageJson {
  name: string
  version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+  }
}

lib/target.js (1)

31-33: ⚠️ Potential issue

Double “‑eabi” suffix bug is still present
Previous feedback noted that checking rawTriple.endsWith('eabi') and slicing -4 produces armv7-unknown-linux-gnu‑eabi (duplicated hyphen) for triples already ending in gnueabi.

-const triple = rawTriple.endsWith('eabi')
-  ? `${rawTriple.slice(0, -4)}-eabi`
+const triple = rawTriple.endsWith('-eabi')
+  ? `${rawTriple.slice(0, -5)}-eabi`

This keeps existing “gnueabi” intact while normalising genuine “‑eabi” triples.

lib/index.js (2)

16-20: ⚠️ Potential issue

Redirect handler still ignores 303/307/308 and relative locations
Earlier review covered this; the current logic only follows 301/302 and assumes an absolute URL. This will break against the npm registry and other CDNs.

- if ((res.statusCode === 301 || res.statusCode === 302) && res.headers.location) {
-   fetch(res.headers.location).then(resolve, reject);
+ const redirects = [301, 302, 303, 307, 308];
+ if (redirects.includes(res.statusCode) && res.headers.location) {
+   const next = res.headers.location.startsWith('http')
+     ? res.headers.location
+     : new URL(res.headers.location, url).toString();
+   fetch(next).then(resolve, reject);
   return;
 }

---

68-69: ⚠️ Potential issue

Command‑injection / escaping issue in installUsingNPM
Interpolating ${pkg}@${version} inside a shell string lets a crafted version slip arbitrary commands (1.2.3 && rm -rf /). Switch to the argument form of execFileSync (or at least spawnSync) so Node handles escaping for you.

- execSync(`npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`, { cwd: installDir, stdio: 'pipe', env })
+ execSync(
+   'npm',
+   ['install', '--loglevel=error', '--prefer-offline', '--no-audit',
+    '--progress=false', `${pkg}@${version}`],
+   { cwd: installDir, stdio: 'pipe', env }
+ )

src/helpers.ts (1)

38-41: ⚠️ Potential issue

Binary sub‑path is truncated – may cause name collisions
Using path.basename(subpath) strips directory structure (darwin/x64/native.node → native.node). Multiple targets can therefore overwrite each other.

-return path.join(pkgLibDir, `${path.basename(subpath)}`)
+// Preserve relative folders to avoid collisions between different targets
+return path.join(pkgLibDir, subpath)

lib/types.js (1)

1-3: Redundant strict mode directive in ES module

This file appears to be a compiled JavaScript file with an empty module structure. The strict mode directive on line 1 is redundant since JavaScript modules are automatically in strict mode.

While this doesn't affect functionality (and may be auto-generated), consider updating your TypeScript compilation settings to avoid redundant strict mode directives:

-"use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 //# sourceMappingURL=types.js.map

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/cli.js (1)

1-2: Redundant strict mode directive in ES module

JavaScript modules are automatically in strict mode, making the strict mode directive redundant.

#!/usr/bin/env node
-"use strict";
 Object.defineProperty(exports, "__esModule", { value: true });

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-2: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/constants.js (1)

1-1: Redundant strict mode directive in ES module

JavaScript modules are automatically in strict mode, making the strict mode directive redundant.

-"use strict";
 var _a;
 Object.defineProperty(exports, "__esModule", { value: true });

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

README.md (1)

16-16: Improve sentence readability with proper punctuation

The description sentence lacks proper punctuation between clauses, making it harder to read.

-The `postinstall` script helper for handling native bindings in legacy `npm` versions, this is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].
+The `postinstall` script helper for handling native bindings in legacy `npm` versions. This is a reimplementation of the [`node-install`][node-install] functionality from [`esbuild`][esbuild] for [`napi-rs`][napi-rs] ecosystem packages like [`rollup`][rollup] and [`unrs-resolver`][unrs-resolver].

🧰 Tools

🪛 LanguageTool

[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...i-rs][napi-rs] ecosystem packages like [rollup][rollup] and [unrs-resolver`][u...

(AI_EN_LECTOR_MISSING_PUNCTUATION)

lib/target.js (1)

1-1: Remove redundant "use strict"
Node treats every .mjs/ES‑module file as strict‑mode already, so the directive can be deleted.

-"use strict";

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

lib/index.js (1)

1-1: Redundant "use strict" – safe to remove for the same reason as above.

🧰 Tools

🪛 Biome (1.9.4)

[error] 1-1: Redundant use strict directive.

The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.

(lint/suspicious/noRedundantUseStrict)

package.json (1)

78-78: Self‑referential devDependency may confuse some tooling
"napi-postinstall": "file:." points back to the package itself. While npm tolerates this in devDependencies, some package managers (pnpm, yarn‑berry) and security scanners flag it as a circular reference. Consider replacing with a workspace/monorepo alias or dropping it entirely when publishing.

src/helpers.ts (1)

35-35: fs.rmdirSync is deprecated
Node 16+ recommends fs.rmSync(dir, { recursive: true, force: true }). This avoids deprecation warnings and future breakage.

- fs.rmdirSync(dir)
+ fs.rmSync(dir, { recursive: true, force: true })

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: JounQin
PR: un-ts/napi-postinstall#3
File: lib/helpers.js:27-45
Timestamp: 2025-04-19T18:11:27.979Z
Learning: Skip reviewing files in the `lib` folder as requested by the repository owner. This folder likely contains compiled/generated code that doesn't need manual review.

lib/cli.js (1)

5-5: Add error handling for the CLI invocation

Currently, unhandled promise rejections from checkAndPreparePackage won't set a non-zero exit code, which can mask failures in scripts. Wrap the call to catch errors and exit with code 1.

- void (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]));
+ (0, index_js_1.checkAndPreparePackage)(process.argv[2], ['1', 'check', 'true', 'yes'].includes(process.argv[3]))
+  .catch(err => {
+    console.error(err);
+    process.exit(1);
+  });

lib/constants.js (1)

8-8: Add error handling for package.json loading

The code directly requires package.json without any error handling. If the file is missing or invalid, this will throw an uncaught exception.

-_a = require(node_path_1.default.resolve(__dirname, '../package.json')), exports.name = _a.name, exports.version = _a.version;
+try {
+  _a = require(node_path_1.default.resolve(__dirname, '../package.json'));
+  exports.name = _a.name;
+  exports.version = _a.version;
+} catch (error) {
+  console.error('Failed to load package.json:', error);
+  exports.name = 'napi-postinstall';
+  exports.version = 'unknown';
+}

README.md (1)

79-82: 🛠️ Refactor suggestion

Update the PackageJson interface to match implementation.

The PackageJson interface shown in the README is incomplete compared to the actual implementation in the code.

export interface PackageJson {
  name: string
  version: string
+  optionalDependencies?: Partial<Record<string, string>>
+  napi?: {
+    packageName?: string
+    binaryName?: string
+    targets?: string[]
+  }
}

lib/target.js (1)

31-33: ⚠️ Potential issue

Update normalization to only target "‑eabi" suffix.

Currently rawTriple.endsWith('eabi') will also match "gnueabi" (e.g. armv7-unknown-linux-gnueabi), which would incorrectly split "gnueabi" into "gnu" and "eabi".

-const triple = rawTriple.endsWith('eabi')
-    ? `${rawTriple.slice(0, -4)}-eabi`
+const triple = rawTriple.endsWith('-eabi')
+    ? `${rawTriple.slice(0, -5)}-eabi`
    : rawTriple;

lib/index.js (2)

16-19: 🛠️ Refactor suggestion

Improve HTTP redirect handling.

The current implementation has two issues:

1. It doesn't handle relative URLs in Location headers
2. It only supports 301/302 redirects, not 303/307/308 which are used by npm registries

-if ((res.statusCode === 301 || res.statusCode === 302) &&
-    res.headers.location) {
-    fetch(res.headers.location).then(resolve, reject);
+const redirectCodes = [301, 302, 303, 307, 308]
+if (redirectCodes.includes(res.statusCode) && res.headers.location) {
+    const nextUrl = res.headers.location.startsWith('http')
+        ? res.headers.location
+        : new URL(res.headers.location, url).toString()
+    fetch(nextUrl).then(resolve, reject);
    return;
}

---

61-69: 🛠️ Refactor suggestion

Shell-escaping risk when spawning npm install.

Embedding pkg & version directly in the shell command is a security risk if these values come from untrusted sources.

Use the array form of execFileSync to prevent shell injection:

-execSync(`npm install --loglevel=error --prefer-offline --no-audit --progress=false ${pkg}@${version}`, { cwd: installDir, stdio: 'pipe', env });
+execFileSync(
+  'npm',
+  ['install', '--loglevel=error', '--prefer-offline', '--no-audit',
+   '--progress=false', `${pkg}@${version}`],
+  { cwd: installDir, stdio: 'pipe', env }
+)