-
Notifications
You must be signed in to change notification settings - Fork 88
fix(contracts): guard module path resolution without import.meta.url #986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds fallback handling for module path resolution to support both ESM and CJS bundles. The main purpose is to improve compatibility when bundlers strip import.meta.url by falling back to __filename.
- Introduced a new
getCurrentModulePath()helper function that safely resolves module paths with fallback logic - Updated
getBaseDirectory()to use the new helper and handle cases where module path cannot be resolved - Updated the CLI script detection logic to gracefully handle undefined module paths
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| return fileURLToPath(moduleUrl); | ||
| } catch (error) { | ||
| console.warn('Failed to resolve fileURLToPath from import.meta.url:', error); |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should include guidance on the fallback behavior. Consider updating to: 'Failed to resolve fileURLToPath from import.meta.url, attempting fallback to __filename:' to better inform users that the error is handled gracefully.
| console.warn('Failed to resolve fileURLToPath from import.meta.url:', error); | |
| console.warn('Failed to resolve fileURLToPath from import.meta.url, attempting fallback to __filename:', error); |
| } | ||
| } | ||
|
|
||
| if (typeof __filename === 'string') { |
Copilot
AI
Oct 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check typeof __filename === 'string' is redundant. In Node.js/Bun environments where __filename exists, it is always a string. The check should be typeof __filename !== 'undefined' to detect its existence, consistent with line 97.
| if (typeof __filename === 'string') { | |
| if (typeof __filename !== 'undefined') { |
…ImportMeta for clarity and consistency
WHAT
guard getCurrentModulePath() so both bundlers and runtime fall back to __filename when import.meta.url isn’t available