diff --git a/.pnp.cjs b/.pnp.cjs index 0516a8e5d314..b73d7550cb76 100644 --- a/.pnp.cjs +++ b/.pnp.cjs @@ -31018,8 +31018,7 @@ const RAW_RUNTIME_STATE = ["npm:4.3.0", {\ "packageLocation": "./.yarn/unplugged/node-addon-api-npm-4.3.0-a07a1232df/node_modules/node-addon-api/",\ "packageDependencies": [\ - ["node-addon-api", "npm:4.3.0"],\ - ["node-gyp", "npm:12.1.0"]\ + ["node-addon-api", "npm:4.3.0"]\ ],\ "linkType": "HARD"\ }]\ diff --git a/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml b/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml new file mode 100644 index 000000000000..54e72a830b15 --- /dev/null +++ b/.yarn/versions/plugin-npm-node-gyp-build-scripts.yml @@ -0,0 +1,8 @@ +releases: + "@yarnpkg/plugin-npm": patch + +declined: + - "@yarnpkg/cli" + - "@yarnpkg/core" + - "@yarnpkg/plugin-compat" + - "@yarnpkg/plugin-npm-cli" diff --git a/packages/plugin-npm/sources/NpmSemverResolver.ts b/packages/plugin-npm/sources/NpmSemverResolver.ts index a045370c90e0..53559d0bc2f8 100644 --- a/packages/plugin-npm/sources/NpmSemverResolver.ts +++ b/packages/plugin-npm/sources/NpmSemverResolver.ts @@ -11,6 +11,7 @@ import * as npmHttpUtils const NODE_GYP_IDENT = structUtils.makeIdent(null, `node-gyp`); const NODE_GYP_MATCH = /\b(node-gyp|prebuild-install)\b/; +const NODE_GYP_INSTALL_SCRIPTS = [`preinstall`, `install`, `postinstall`]; export class NpmSemverResolver implements Resolver { supportsDescriptor(descriptor: Descriptor, opts: MinimalResolveOptions) { @@ -152,10 +153,15 @@ export class NpmSemverResolver implements Resolver { // Manually add node-gyp dependency if there is a script using it and not already set // This is because the npm registry will automatically add a `node-gyp rebuild` install script // in the metadata if there is not already an install script and a binding.gyp file exists. - // Also, node-gyp is not always set as a dependency in packages, so it will also be added if used in scripts. - if (!manifest.dependencies.has(NODE_GYP_IDENT.identHash) && !manifest.peerDependencies.has(NODE_GYP_IDENT.identHash)) { + // Also, node-gyp is not always set as a dependency in packages, so it will also be added + // if used in package scripts that may be reached from an install lifecycle script. + if ( + !manifest.dependencies.has(NODE_GYP_IDENT.identHash) && + !manifest.peerDependencies.has(NODE_GYP_IDENT.identHash) && + NODE_GYP_INSTALL_SCRIPTS.some(scriptName => manifest.scripts.has(scriptName)) + ) { for (const value of manifest.scripts.values()) { - if (value.match(NODE_GYP_MATCH)) { + if (NODE_GYP_MATCH.test(value)) { manifest.dependencies.set(NODE_GYP_IDENT.identHash, structUtils.makeDescriptor(NODE_GYP_IDENT, `latest`)); break; } diff --git a/packages/plugin-npm/tests/NpmSemverResolver.test.ts b/packages/plugin-npm/tests/NpmSemverResolver.test.ts index a406e5a0c6f3..82b2a3adc612 100644 --- a/packages/plugin-npm/tests/NpmSemverResolver.test.ts +++ b/packages/plugin-npm/tests/NpmSemverResolver.test.ts @@ -1,6 +1,15 @@ -import {structUtils} from '@yarnpkg/core'; +jest.mock(`../sources/npmHttpUtils`, () => ({ + getPackageMetadata: jest.fn(), +})); -import {NpmSemverResolver} from '../sources/NpmSemverResolver'; +import {structUtils} from '@yarnpkg/core'; + +const {NpmSemverResolver}: typeof import('../sources/NpmSemverResolver') = require(`../sources/NpmSemverResolver`); +const npmHttpUtils: typeof import('../sources/npmHttpUtils') = require(`../sources/npmHttpUtils`); + +afterEach(() => { + jest.clearAllMocks(); +}); describe(`NpmSemverResolver`, () => { describe(`getSatisfying`, () => { @@ -22,4 +31,71 @@ describe(`NpmSemverResolver`, () => { expect(results.locators[0].locatorHash).toEqual(locator.locatorHash); }); }); + + describe(`resolve`, () => { + const ident = structUtils.makeIdent(null, `native-package`); + const nodeGypIdent = structUtils.makeIdent(null, `node-gyp`); + + const makeResolveOptions = () => ({ + project: { + configuration: { + normalizeDependencyMap: (dependencies: Map) => dependencies, + }, + }, + } as any); + + const mockPackageMetadata = (scripts: Record) => { + const getPackageMetadata = npmHttpUtils.getPackageMetadata as jest.MockedFunction; + + getPackageMetadata.mockResolvedValue({ + versions: { + [`1.0.0`]: { + name: structUtils.stringifyIdent(ident), + version: `1.0.0`, + scripts, + }, + }, + } as any); + }; + + it(`shouldn't inject node-gyp when only a non-build script uses it`, async () => { + mockPackageMetadata({ + test: `node-gyp rebuild`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(false); + }); + + it(`should inject node-gyp when an install script uses it`, async () => { + mockPackageMetadata({ + install: `node-gyp rebuild`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(true); + }); + + it(`should inject node-gyp when an install script delegates to another script using it`, async () => { + mockPackageMetadata({ + build: `node-gyp rebuild`, + install: `yarn build`, + }); + + const resolver = new NpmSemverResolver(); + const locator = structUtils.makeLocator(ident, `npm:1.0.0`); + + const pkg = await resolver.resolve(locator, makeResolveOptions()); + + expect(pkg.dependencies.has(nodeGypIdent.identHash)).toEqual(true); + }); + }); }); diff --git a/yarn.lock b/yarn.lock index 6f067370dcf7..a0d9fba14908 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15929,8 +15929,6 @@ __metadata: "node-addon-api@npm:^4.3.0": version: 4.3.0 resolution: "node-addon-api@npm:4.3.0" - dependencies: - node-gyp: "npm:latest" checksum: 10/d3b38d16cb9ad0714d965331d0e38cef1c27750c2c3343cd3464a9ed8158501a2910ccbf2fd9fdc476e806a19dbc9e0524ff9d66a7c779d42a9752a63ba30b80 languageName: node linkType: hard