fix: resolve ERR_REQUIRE_ESM for inquirer v9+ (#1197)#1278
fix: resolve ERR_REQUIRE_ESM for inquirer v9+ (#1197)#1278bmadcode merged 1 commit intobmad-code-org:mainfrom
Conversation
Inquirer v9+ is ESM-only, causing ERR_REQUIRE_ESM when loaded via
require() in CommonJS. Convert all require('inquirer') calls to
dynamic import('inquirer') across 8 CLI files.
Fixes bmad-code-org#1197
📝 WalkthroughWalkthroughThis PR converts static CommonJS Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @tools/cli/commands/install.js:
- Line 74: The dynamic import of inquirer (const { default: inquirer } = await
import('inquirer');) lacks error handling and is inconsistent with other files
that use getInquirer(); wrap the dynamic import in a try/catch to catch import
failures and emit a clear error message (or exit gracefully) using the same
logger/exit pattern used elsewhere, and either replace the inline import with
the shared getInquirer() helper to match config-collector.js and ui.js or add a
short comment explaining why an inline import is used for this single-use case
for consistency.
In @tools/cli/installers/lib/core/config-collector.js:
- Around line 8-15: The dynamic import inside getInquirer currently can reject
and cause an unhelpful crash; wrap the await import('inquirer') in a try/catch
inside getInquirer, set _inquirer only on success, and on failure log a clear,
actionable message including the original error (e.g., via console.error or the
module's logger) and either rethrow a new Error with context or terminate
gracefully so callers don't get an unhandled rejection.
In @tools/cli/lib/ui.js:
- Around line 8-15: The dynamic import in getInquirer currently has no error
handling, so failures will cause unhandled rejections; wrap the
import('inquirer') call in a try/catch inside getInquirer, set _inquirer only on
success, and on failure log or throw a clear, actionable error (e.g., using
console.error or a project logger) that mentions inquirer failed to load and
suggests reinstalling the package, then rethrow or return a controlled fallback
so callers of getInquirer (the 13 methods using it) receive a predictable error
rather than an unhandled rejection.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tools/cli/commands/install.jstools/cli/installers/lib/core/config-collector.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/ide/antigravity.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/lib/ui.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Focus on inconsistencies, contradictions, edge cases and serious issues.
Avoid commenting on minor issues such as linting, formatting and style issues.
When providing code suggestions, use GitHub's suggestion format:<code changes>
Files:
tools/cli/commands/install.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/ide/antigravity.jstools/cli/lib/ui.jstools/cli/installers/lib/core/config-collector.jstools/cli/installers/lib/core/installer.js
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: CLI tooling code. Check for: missing error handling on fs operations,
path.join vs string concatenation, proper cleanup in error paths.
Flag any process.exit() without error message.
Files:
tools/cli/commands/install.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/ide/antigravity.jstools/cli/lib/ui.jstools/cli/installers/lib/core/config-collector.jstools/cli/installers/lib/core/installer.js
🧬 Code graph analysis (5)
tools/cli/installers/lib/ide/github-copilot.js (1)
tools/cli/commands/install.js (1)
inquirer(74-74)
tools/cli/installers/lib/ide/claude-code.js (1)
tools/cli/commands/install.js (1)
inquirer(74-74)
tools/cli/installers/lib/ide/antigravity.js (1)
tools/cli/commands/install.js (1)
inquirer(74-74)
tools/cli/lib/ui.js (2)
tools/cli/installers/lib/core/config-collector.js (1)
_inquirer(9-9)tools/cli/commands/install.js (1)
inquirer(74-74)
tools/cli/installers/lib/core/config-collector.js (3)
tools/cli/lib/ui.js (1)
_inquirer(9-9)tools/cli/commands/install.js (1)
inquirer(74-74)tools/cli/lib/yaml-format.js (1)
initializeModules(10-14)
🔇 Additional comments (14)
tools/cli/installers/lib/ide/github-copilot.js (1)
24-24: LGTM - Dynamic import correctly implemented.The conversion from static
require('inquirer')to dynamicawait import('inquirer')correctly addresses the ERR_REQUIRE_ESM issue with inquirer v9+. The destructuring of the default export is correct, and the function is already async.tools/cli/installers/lib/ide/claude-code.js (3)
60-60: LGTM - Dynamic import correctly implemented.The dynamic import is properly placed inside the async function and correctly destructures the default export.
308-308: LGTM - Consistent dynamic import pattern.The dynamic import follows the same pattern as other locations in this file and correctly addresses the ESM compatibility issue.
345-345: LGTM - Dynamic import correctly implemented.The lazy-loading approach ensures inquirer is only loaded when the prompt is actually needed.
tools/cli/installers/lib/ide/codex.js (1)
24-24: LGTM - Dynamic import correctly implemented.The conversion to dynamic import is correct and consistent with the pattern used across other IDE setup files. This properly resolves the ERR_REQUIRE_ESM issue for inquirer v9+.
tools/cli/installers/lib/ide/antigravity.js (3)
61-61: LGTM - Dynamic import correctly implemented.The dynamic import is properly placed and correctly structured for ESM compatibility.
300-300: LGTM - Consistent dynamic import pattern.The lazy-loading approach is appropriate for this interactive configuration path.
337-337: LGTM - Dynamic import correctly implemented.The dynamic import ensures inquirer is only loaded when prompting is actually needed.
tools/cli/installers/lib/core/installer.js (3)
2142-2142: LGTM - Dynamic import correctly implemented.The conversion from static require to dynamic import is correct and addresses the ERR_REQUIRE_ESM issue with inquirer v9+.
2159-2159: LGTM - Dynamic import correctly implemented.The lazy-loading pattern ensures inquirer is only loaded when the legacy V4 migration prompt is actually needed.
2440-2440: LGTM - Dynamic import correctly implemented.The dynamic import is properly structured and ensures inquirer is loaded only when handling missing custom sources that require user interaction.
tools/cli/installers/lib/core/config-collector.js (2)
186-186: LGTM!Properly defers inquirer loading until needed. The method is already async, so the await is correct.
505-505: LGTM!Properly defers inquirer loading until needed. The method is already async, so the await is correct.
tools/cli/lib/ui.js (1)
26-26: LGTM!All 13 usage sites properly defer inquirer loading until needed. The methods are already async, so the await calls are correct. The lazy-loading pattern prevents ERR_REQUIRE_ESM on initial module load.
Also applies to: 462-462, 595-595, 620-620, 652-652, 759-759, 840-840, 861-861, 918-918, 1105-1105, 1256-1256, 1394-1394, 1589-1589
| console.log(chalk.dim(' • ElevenLabs AI (150+ premium voices)')); | ||
| console.log(chalk.dim(' • Piper TTS (50+ free voices)\n')); | ||
|
|
||
| const { default: inquirer } = await import('inquirer'); |
There was a problem hiding this comment.
Add error handling and consider consistency with other files.
Two concerns:
-
Missing error handling: If
import('inquirer')fails, the process will crash with an unclear error message rather than gracefully explaining the problem. -
Pattern inconsistency: This file uses inline
await import('inquirer')whileconfig-collector.jsandui.jsuse a lazy-loading helper functiongetInquirer(). For a single-use case like this, inline is acceptable, but the inconsistency may confuse future maintainers.
🛡️ Proposed fix to add error handling
- const { default: inquirer } = await import('inquirer');
+ let inquirer;
+ try {
+ inquirer = (await import('inquirer')).default;
+ } catch (error) {
+ console.error(chalk.red('Failed to load inquirer module:'), error.message);
+ console.error(chalk.dim('Try running: npm install inquirer'));
+ process.exit(1);
+ }
await inquirer.prompt([Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @tools/cli/commands/install.js at line 74, The dynamic import of inquirer
(const { default: inquirer } = await import('inquirer');) lacks error handling
and is inconsistent with other files that use getInquirer(); wrap the dynamic
import in a try/catch to catch import failures and emit a clear error message
(or exit gracefully) using the same logger/exit pattern used elsewhere, and
either replace the inline import with the shared getInquirer() helper to match
config-collector.js and ui.js or add a short comment explaining why an inline
import is used for this single-use case for consistency.
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | ||
| let _inquirer = null; | ||
| async function getInquirer() { | ||
| if (!_inquirer) { | ||
| _inquirer = (await import('inquirer')).default; | ||
| } | ||
| return _inquirer; | ||
| } |
There was a problem hiding this comment.
Add error handling for the dynamic import.
If import('inquirer') fails (e.g., package not installed, corrupted node_modules), the unhandled rejection will crash the process with a cryptic error rather than a helpful message.
🛡️ Proposed fix to add error handling
// Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM
let _inquirer = null;
async function getInquirer() {
if (!_inquirer) {
- _inquirer = (await import('inquirer')).default;
+ try {
+ _inquirer = (await import('inquirer')).default;
+ } catch (error) {
+ console.error(chalk.red('Failed to load inquirer module:'), error.message);
+ console.error(chalk.dim('Try running: npm install inquirer'));
+ process.exit(1);
+ }
}
return _inquirer;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | |
| let _inquirer = null; | |
| async function getInquirer() { | |
| if (!_inquirer) { | |
| _inquirer = (await import('inquirer')).default; | |
| } | |
| return _inquirer; | |
| } | |
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | |
| let _inquirer = null; | |
| async function getInquirer() { | |
| if (!_inquirer) { | |
| try { | |
| _inquirer = (await import('inquirer')).default; | |
| } catch (error) { | |
| console.error(chalk.red('Failed to load inquirer module:'), error.message); | |
| console.error(chalk.dim('Try running: npm install inquirer')); | |
| process.exit(1); | |
| } | |
| } | |
| return _inquirer; | |
| } |
🤖 Prompt for AI Agents
In @tools/cli/installers/lib/core/config-collector.js around lines 8 - 15, The
dynamic import inside getInquirer currently can reject and cause an unhelpful
crash; wrap the await import('inquirer') in a try/catch inside getInquirer, set
_inquirer only on success, and on failure log a clear, actionable message
including the original error (e.g., via console.error or the module's logger)
and either rethrow a new Error with context or terminate gracefully so callers
don't get an unhandled rejection.
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | ||
| let _inquirer = null; | ||
| async function getInquirer() { | ||
| if (!_inquirer) { | ||
| _inquirer = (await import('inquirer')).default; | ||
| } | ||
| return _inquirer; | ||
| } |
There was a problem hiding this comment.
Add error handling for the dynamic import.
If import('inquirer') fails (e.g., package not installed, corrupted node_modules), the unhandled rejection will crash the process with a cryptic error rather than a helpful message. Given this helper is called by 13 different methods in this file, proper error handling is essential.
🛡️ Proposed fix to add error handling
// Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM
let _inquirer = null;
async function getInquirer() {
if (!_inquirer) {
- _inquirer = (await import('inquirer')).default;
+ try {
+ _inquirer = (await import('inquirer')).default;
+ } catch (error) {
+ console.error(chalk.red('Failed to load inquirer module:'), error.message);
+ console.error(chalk.dim('Try running: npm install inquirer'));
+ process.exit(1);
+ }
}
return _inquirer;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | |
| let _inquirer = null; | |
| async function getInquirer() { | |
| if (!_inquirer) { | |
| _inquirer = (await import('inquirer')).default; | |
| } | |
| return _inquirer; | |
| } | |
| // Lazy-load inquirer (ESM module) to avoid ERR_REQUIRE_ESM | |
| let _inquirer = null; | |
| async function getInquirer() { | |
| if (!_inquirer) { | |
| try { | |
| _inquirer = (await import('inquirer')).default; | |
| } catch (error) { | |
| console.error(chalk.red('Failed to load inquirer module:'), error.message); | |
| console.error(chalk.dim('Try running: npm install inquirer')); | |
| process.exit(1); | |
| } | |
| } | |
| return _inquirer; | |
| } |
🤖 Prompt for AI Agents
In @tools/cli/lib/ui.js around lines 8 - 15, The dynamic import in getInquirer
currently has no error handling, so failures will cause unhandled rejections;
wrap the import('inquirer') call in a try/catch inside getInquirer, set
_inquirer only on success, and on failure log or throw a clear, actionable error
(e.g., using console.error or a project logger) that mentions inquirer failed to
load and suggests reinstalling the package, then rethrow or return a controlled
fallback so callers of getInquirer (the 13 methods using it) receive a
predictable error rather than an unhandled rejection.
|
I really hate mixing requires and imports. Isn't there a version of "downgrade something back until it doesn't need to be ESM", followed by a somewhat bigger "switch the whole shebang to ESM"? |
@alexeyv For this PR, I went with dynamic imports because:
That said, a proper ESM migration would definitely be cleaner. Happy to help with that if maintainers want to go that route! |
|
thanks @Q00 - merged! Will run some tests also on mac and verify a few other scenarios and try to get this published with the next version today or tomorrow! |
|
@Q00 : if you can figure out a clean conversion of the whole thing into ESM, I'm not gonna promise you a bronze horse statue, but a reasonable degree of eternal gratitude? for sure! |
|
@alexeyv Challenge accepted! I think a phased approach is best to ensure stability. Here is my 3-step plan:
I'll get started on Step 1 and keep you posted! |
3 participants
What
Convert require('inquirer') to dynamic import('inquirer') in 8 CLI files
Why
Inquirer v9+ is ESM-only, causing ERR_REQUIRE_ESM crash during installation
Fixes #1197
How
Testing
Tested
npx ./bmad-method-6.0.0-alpha.22.tgz installon macOS and Windows - all prompts workingScreenshots