[BUG] Regression in 10.4.0 results in failed install

[tnorling]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

Starting in 10.4.0 npm is failing to install a native addon package I own because it's attempting to run node-gyp rebuild and is not finding the binding.gyp file. We ship precompiled binaries and thus have no need to recompile or include the bindings. From searching through previous issues I gather that npm is automatically adding the default install script to some hidden metadata(?) during publish when a binding file exists. I'm not sure if this is expected behavior but if so it probably should have been a breaking change.

A couple additional notes:

• Downgrading to any version prior to 10.4.0 installation succeeds.
• Packing a local tarball and installing that instead succeeds
• If a package-lock exists from a previous successful install installation succeeds

Expected Behavior

Don't attempt to recompile binaries unless I've explicitly configured my package to do so on install.

Steps To Reproduce

1. Install npm version 10.4.0 or 10.5.0
2. Try to install a package which includes precompiled binaries but no install script such as @azure/msal-node-extensions
3. Observe no install script in package.json
4. Observe npm attempts to recompile binaries anyway and fails

Environment

• npm: 10.5.0
• Node.js: 20.12.0
• OS Name: Windows
• System Model Name:
• npm config:

; copy and paste output from `npm config ls` here

[wraithgar]

I can't reproduce this with the latest npm or with 10.5.0

~/D/n/s/gyp $ rm -rf package-lock.json node_modules/;npx npm@10.5.0 install

> gyp@1.0.0 npx
> npm install


added 42 packages, and audited 43 packages in 6s

found 0 vulnerabilities
~/D/n/s/gyp $ npm pkg get dependencies
{
  "@azure/msal-node-extensions": "^1.0.15"
}

I believe the dependency in question published a new version 1.0.15 that fixes this. AzureAD/microsoft-authentication-library-for-js#7022

[mikeharder]

@wraithgar: You can see the regression by attempting to install @azure/msal-node-extensions@1.0.14 in npm@10.3.0 and npm@10.4.0:

$ rm -rf package-lock.json node_modules/;npx npm@10.3.0 install @azure/msal-node-extensions@1.0.14
added 42 packages, and audited 43 packages in 1s
9 packages are looking for funding
  run `npm fund` for details
found 0 vulnerabilities

$ rm -rf package-lock.json node_modules/;npx npm@10.4.0 install @azure/msal-node-extensions@1.0.14
npm ERR! code 1
npm ERR! path .../node_modules/@azure/msal-node-extensions
npm ERR! command failed
npm ERR! command sh -c node-gyp rebuild

[ljharb]

@mikeharder what about with npm 10 latest? older versions of npm 10 aren't relevant

[mikeharder]

@ljharb: Regression was introduced in 10.4.0, still repros in latest 10.6.0:

$ rm -rf package-lock.json node_modules/;npx npm@10.6.0 install @azure/msal-node-extensions@1.0.14
npm error code 1
npm error path .../node_modules/@azure/msal-node-extensions
npm error command failed
npm error command sh -c node-gyp rebuild

[ljharb]

@mikeharder right, but #7364 (comment) suggests it's a bug in 1.0.14. Try with 1.0.15 of that package.

[mikeharder]

@ljharb: Yes, 1.0.15 of that package includes a workaround to this breaking change / regression in npm. But it's still a breaking change / regression you might want to know about.

[wraithgar]

It's not a regression, it was a bugfix that exposed broken behavior in some packages. npm has always ran that script if there is no install script but there's a binding.gyp present, but was not consistently finding it. It is now.

You can see a two year old issue about it where it was happening in npm 7 and 8 at #5234.

[tnorling]

I think the part that is unexpected here is that there isn't a binding file in the published package. It wasn't immediately clear to me that it's making the binding / no binding determination based on the contents of the folder at publish time rather than the contents of the package actually published. Whether this is a bug or a regression ultimately doesn't matter to me but the issue is that it worked in one version and then didn't work in the next with no clear indication of what was wrong.

I'm suggesting npm do one of the following:

1. Base the decision on whether or not to run the default install script on whether the binding file is present in the published package
2. Log a warning or throw an error if you attempt to publish a package without an install script or binding file

I don't know all the intricate details of how npm works under the hood but the fact of the matter is that npm first allowed us to publish a package which worked for several years and then overnight broke installation of all versions of that package.

And yes, we added a no-op install script in 1.0.15. It feels like a hack tbh but if that's the recommended pattern to address this we'll keep it, I just suggest that it be officially documented.

[wraithgar]

Log a warning or throw an error if you attempt to publish a package without an install script or binding file

This is part of our long-term plan in making npm to stop modifying the manifest at publish time.

[tnorling]

I'd argue that that should have been a prerequisite before making this change but I'm glad to hear it's on the roadmap. Is including a blank install script in the package the recommended best practice to deal with this or is there a better way to ship a package with precompiled binaries?

[wraithgar]

Yeah for now having an existing install entry in scripts will prevent npm from "helpfully" trying to build via node-gyp.