fix: run on Node 20 — load ESM-only prompts via dynamic import - #28
Open
karaposu wants to merge 1 commit into
Open
fix: run on Node 20 — load ESM-only prompts via dynamic import#28karaposu wants to merge 1 commit into
karaposu wants to merge 1 commit into
Conversation
The CLI declares `engines: node >=20.0.0`, but `@inquirer/prompts` is ESM-only and was imported statically in init.ts and add-mcp.ts. Under `module: commonjs` tsc compiles those to require(), which throws ERR_REQUIRE_ESM on any Node before the require(ESM) backport (< 20.19 / < 22.12). Since index.ts imports both command modules at startup, every invocation crashes — including `brightdata --version`. Reproduced on Node 20.17.0 against v0.3.5: 'Error [ERR_REQUIRE_ESM]: require() of ES Module ... @inquirer/prompts ... not supported'. - Load @inquirer/prompts lazily through utils/load-prompts.ts, which hides the import() behind new Function() so tsc cannot down-compile it back to require(). Same technique already used by load_open() in browser_auth.ts. - Prompts are now loaded inside the handlers that use them, so non-interactive commands no longer pay for the ESM graph at startup. - Add utils/node-version.ts: a floor-20 guard that prints an actionable message instead of a stack trace on older runtimes. - Add CI (the repo had none): type-check + tests on a Node 20.17.0 / 24 matrix, plus a build-and-run smoke step. 20.17.0 is pinned because it predates the require(ESM) backport — testing only the latest 20.x would pass code that still crashes for users on the declared floor. The smoke step exists because Vitest cannot execute the dynamic-import loader, so unit tests alone cannot catch this class of regression. - Migrate the add-mcp test mock from @inquirer/prompts to load-prompts. Full suite green (373); verified on real Node 20.17.0 and Node 24.
This was referenced Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
package.jsondeclaresengines: { "node": ">=20.0.0" }, but the published CLI crashes on startup for a large part of that range.@inquirer/promptsis ESM-only and is imported statically insrc/commands/init.tsandsrc/commands/add-mcp.ts. Undermodule: commonjs, tsc compiles those imports torequire(), which throwsERR_REQUIRE_ESMon any Node released before therequire(ESM)backport (< 20.19, and < 22.12). Becausesrc/index.tsimports both command modules at startup, every invocation fails — includingbrightdata --version.Reproduced against current
main(v0.3.5) on Node 20.17.0:With this PR, same runtime:
The fix
src/utils/load-prompts.ts— loads@inquirer/promptslazily throughnew Function('specifier', 'return import(specifier);'), which hides theimport()from tsc so it is emitted as a genuine dynamic import instead of being down-compiled back torequire(). This is the same technique already used byload_open()insrc/utils/browser_auth.ts, so the pattern is not new to this codebase. The module promise is cached, so prompts load at most once.init.ts/add-mcp.ts— prompts are destructured inside the handlers that use them. Side benefit: non-interactive commands no longer pay for loading the inquirer graph at startup.src/utils/node-version.ts— a floor-20 guard called first inmain(), so anything below the supported floor gets an actionable message instead of a stack trace. The floor is the dependency minimum (commander's ownengines), not the require(ESM) boundary — the loader above removes that boundary entirely, which is what keeps Node 20 supported rather than dropping it.CI
The repo currently has no CI workflow (only
release.yml), so type-check and tests have never run on a PR. This adds one, with two deliberate choices:20.17.0and24. Usingnode-version: 20would install 20.19+, whererequire(ESM)is legal — it would have passed on the broken code. 20.17.0 is a runtime the declared>=20.0.0still promises to support, so it is the version that actually guards this.new Functiondynamic import, so unit tests structurally cannot cover the loader. Running the built CLI is what catches anERR_REQUIRE_ESMregression.Happy to split the CI into a separate PR if you'd prefer this one minimal.
Testing
tsc --noEmitclean.Note on prior work
This is a re-application of #14, which was merged into
devon 2026-06-15 but never reachedmain—devhas had no commits since that merge, whilemainhas moved 18 commits ahead, so the fix has been absent from every release since. This PR targetsmaindirectly and is rebased on currentmain, resolving the overlap with theresolve_keychange inadd-mcp.ts.I have a follow-up (#16, currently open against
dev) adding a genericdatasets triggercommand — happy to retarget that tomaintoo ifdevis no longer the integration branch.