Skip to content

fix: yargs esm fix test - #8460

Merged
jskeet merged 3 commits into
googleapis:mainfrom
quirogas:feat/yargs-esm-fix
Jun 9, 2026
Merged

jskeet merged 3 commits into
googleapis:mainfrom
quirogas:feat/yargs-esm-fix

Conversation

@quirogas

@quirogas quirogas commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor

Recently, Node.js v24 and v26 were added to the GHA test matrix (#8445). However, the test runners (c8 and mocha) both depend on [email protected].
In Node.js >= 24, when a package specifies "type": "module" in its package.json, Node.js treats extensionless files inside that package as ES Modules (ESM). Since [email protected] contains an extensionless file named yargs that uses CommonJS require(), booting up c8 or mocha on Node >= 24 crashes immediately with:

ReferenceError: require is not defined in ES module scope, you can use import instead

We cannot simply upgrade yargs to 18.0.0 globally because yargs@18 is ESM-only, which would immediately break our test suite on Node 18 (our current minimum published version) because Node 18 does not support require(esm) by default.

The Solution

To support both Node 18 and Node 24/26 concurrently during this transitional period, we implement a dynamic pnpmfile hook:

  1. Root .pnpmfile.cjs: Added a root-level readPackage hook that dynamically scans package dependencies. If it detects yargs and the running Node.js version is Node >= 24, it dynamically overrides yargs to 18.0.0 (which is fully ESM-compatible and doesn't crash). On Node 18/20/22, it leaves it at 17.7.2 to ensure backward compatibility.
  2. CI Integration (ci/run_single_test.sh): Updated the test runner script to pass the --pnpmfile flag pointing to the root .pnpmfile.cjs during all package installations.

This is a zero-risk, highly-isolated, transitional solution. Once Node 18 is officially retired, this patch can be easily removed by running pnpm patch-remove yargs and upgrading to yargs@18 globally.

Related Pull Requests

@quirogas quirogas self-assigned this Jun 9, 2026
@quirogas quirogas changed the title Feat/yargs esm fix fix: yargs esm fix test Jun 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a .pnpmfile.cjs hook to override the yargs dependency version to 18.0.0 when running on Node.js version 24 or higher, and updates the CI test script to use this pnpmfile during installation. The review feedback suggests simplifying the Node.js version parsing using process.versions.node and removing verbose logging to prevent flooding the installation logs.

Comment thread .pnpmfile.cjs
@quirogas
quirogas requested a review from pearigee June 9, 2026 09:15
@quirogas
quirogas marked this pull request as ready for review June 9, 2026 09:19
@quirogas
quirogas requested a review from a team as a code owner June 9, 2026 09:19
@jskeet
jskeet merged commit 517ac68 into googleapis:main Jun 9, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants