fix CommonJS require broken by import.meta.url in CJS output (#157)#166
Merged
9cb14c1ec0 merged 1 commit intomasterfrom Mar 6, 2026
Merged
fix CommonJS require broken by import.meta.url in CJS output (#157)#1669cb14c1ec0 merged 1 commit intomasterfrom
9cb14c1ec0 merged 1 commit intomasterfrom
Conversation
- Wrap import.meta.url in eval() so it's hidden from the CJS parser,
preventing SyntaxError when loading dist/cjs files via require()
- Add dist/cjs/package.json {"type":"commonjs"} and
dist/esm/package.json {"type":"module"} to explicitly declare module
types regardless of parent package.json settings
- Convert scripts/create-wrappers.js from ESM to CJS syntax
Closes #157
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements dual-build support (CommonJS and ESM) for the venom-bot package to address cross-environment compatibility. Changes include updating the wrapper creation script to generate separate cjs and esm output directories with package.json module-type markers, and modifying __dirname resolution in source files to safely access import.meta.url via eval() for CJS parser compatibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/whatsapp.ts`:
- Around line 11-21: Suppress the Biome linter for the intentional eval in
getDirname by adding a Biome ignore comment immediately above the eval usage;
specifically, place a biome noGlobalEval suppression (e.g., /* biome-ignore
noGlobalEval */) right before the line that calls eval('import.meta.url') inside
getDirname so the fallback using fileURLToPath(import.meta.url) is allowed
without CI lint failure.
In `@src/utils/qr-generator.ts`:
- Around line 9-16: The intentional use of eval in getDirname triggers Biome's
no-eval rule; add an inline Biome ignore comment immediately before the eval
expression in src/utils/qr-generator.ts (inside the getDirname function) to
silence the linter for this intentional, constant-string usage — e.g., annotate
the eval('import.meta.url') call with the appropriate Biome ignore comment so
linting in CI won't fail while leaving the logic in getDirname unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
dist/cjs/api/whatsapp.jsis excluded by!**/dist/**dist/cjs/api/whatsapp.js.mapis excluded by!**/dist/**,!**/*.mapdist/cjs/package.jsonis excluded by!**/dist/**dist/cjs/utils/qr-generator.jsis excluded by!**/dist/**dist/cjs/utils/qr-generator.js.mapis excluded by!**/dist/**,!**/*.mapdist/esm/api/whatsapp.jsis excluded by!**/dist/**dist/esm/api/whatsapp.js.mapis excluded by!**/dist/**,!**/*.mapdist/esm/package.jsonis excluded by!**/dist/**dist/esm/utils/qr-generator.jsis excluded by!**/dist/**dist/esm/utils/qr-generator.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (3)
scripts/create-wrappers.jssrc/api/whatsapp.tssrc/utils/qr-generator.ts
Comment on lines
+11
to
21
| import { dirname } from 'path'; | ||
|
|
||
| // Cross-platform __dirname | ||
| // Cross-platform __dirname for CJS/ESM dual builds | ||
| // eval hides import.meta from the CJS parser to avoid SyntaxError | ||
| const getDirname = () => { | ||
| try { | ||
| if (typeof __dirname !== 'undefined') { | ||
| return __dirname; | ||
| } catch (e) { | ||
| // @ts-ignore | ||
| return dirname(fileURLToPath(import.meta.url)); | ||
| } | ||
| // @ts-ignore | ||
| return dirname(fileURLToPath(eval('import.meta.url'))); | ||
| }; |
There was a problem hiding this comment.
Suppress the Biome noGlobalEval error for this fallback.
This eval is intentional and constant-string, but Biome treats it as an error. Add an ignore to avoid CI lint failures.
✅ Suggested fix
// `@ts-ignore`
+ // biome-ignore lint/security/noGlobalEval -- needed to hide import.meta from CJS parser
return dirname(fileURLToPath(eval('import.meta.url')));📝 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.
Suggested change
| import { dirname } from 'path'; | |
| // Cross-platform __dirname | |
| // Cross-platform __dirname for CJS/ESM dual builds | |
| // eval hides import.meta from the CJS parser to avoid SyntaxError | |
| const getDirname = () => { | |
| try { | |
| if (typeof __dirname !== 'undefined') { | |
| return __dirname; | |
| } catch (e) { | |
| // @ts-ignore | |
| return dirname(fileURLToPath(import.meta.url)); | |
| } | |
| // @ts-ignore | |
| return dirname(fileURLToPath(eval('import.meta.url'))); | |
| }; | |
| import { dirname } from 'path'; | |
| // Cross-platform __dirname for CJS/ESM dual builds | |
| // eval hides import.meta from the CJS parser to avoid SyntaxError | |
| const getDirname = () => { | |
| if (typeof __dirname !== 'undefined') { | |
| return __dirname; | |
| } | |
| // `@ts-ignore` | |
| // biome-ignore lint/security/noGlobalEval -- needed to hide import.meta from CJS parser | |
| return dirname(fileURLToPath(eval('import.meta.url'))); | |
| }; |
🧰 Tools
🪛 Biome (2.4.4)
[error] 20-20: eval() exposes to security risks and performance issues.
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/whatsapp.ts` around lines 11 - 21, Suppress the Biome linter for the
intentional eval in getDirname by adding a Biome ignore comment immediately
above the eval usage; specifically, place a biome noGlobalEval suppression
(e.g., /* biome-ignore noGlobalEval */) right before the line that calls
eval('import.meta.url') inside getDirname so the fallback using
fileURLToPath(import.meta.url) is allowed without CI lint failure.
Comment on lines
+9
to
+16
| // Cross-platform __dirname for CJS/ESM dual builds | ||
| // eval hides import.meta from the CJS parser to avoid SyntaxError | ||
| const getDirname = () => { | ||
| if (typeof __dirname !== 'undefined') { | ||
| return __dirname; | ||
| } | ||
| // @ts-ignore | ||
| return dirname(fileURLToPath(import.meta.url)); | ||
| return dirname(fileURLToPath(eval('import.meta.url'))); |
There was a problem hiding this comment.
Add a Biome ignore for the intentional eval.
Biome flags eval as an error; if lint runs in CI, this will fail. Since the use is intentional and constant-string, add an inline ignore for the rule.
✅ Suggested fix
// `@ts-ignore`
+ // biome-ignore lint/security/noGlobalEval -- needed to hide import.meta from CJS parser
return dirname(fileURLToPath(eval('import.meta.url')));📝 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.
Suggested change
| // Cross-platform __dirname for CJS/ESM dual builds | |
| // eval hides import.meta from the CJS parser to avoid SyntaxError | |
| const getDirname = () => { | |
| if (typeof __dirname !== 'undefined') { | |
| return __dirname; | |
| } | |
| // @ts-ignore | |
| return dirname(fileURLToPath(import.meta.url)); | |
| return dirname(fileURLToPath(eval('import.meta.url'))); | |
| // Cross-platform __dirname for CJS/ESM dual builds | |
| // eval hides import.meta from the CJS parser to avoid SyntaxError | |
| const getDirname = () => { | |
| if (typeof __dirname !== 'undefined') { | |
| return __dirname; | |
| } | |
| // `@ts-ignore` | |
| // biome-ignore lint/security/noGlobalEval -- needed to hide import.meta from CJS parser | |
| return dirname(fileURLToPath(eval('import.meta.url'))); |
🧰 Tools
🪛 Biome (2.4.4)
[error] 16-16: eval() exposes to security risks and performance issues.
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/qr-generator.ts` around lines 9 - 16, The intentional use of eval
in getDirname triggers Biome's no-eval rule; add an inline Biome ignore comment
immediately before the eval expression in src/utils/qr-generator.ts (inside the
getDirname function) to silence the linter for this intentional, constant-string
usage — e.g., annotate the eval('import.meta.url') call with the appropriate
Biome ignore comment so linting in CI won't fail while leaving the logic in
getDirname unchanged.
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
1 participant
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.
Closes #157
Fixes #157 .
Changes proposed in this pull request
To test (it takes a while):
npm install github:<username>/venom#<branch>Summary by CodeRabbit