fix(node): use eval for require to avoid bundler issue #3239
Conversation
🦋 Changeset detectedLatest commit: d6e8545 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![squadronai[bot]](https://avatars.githubusercontent.com/in/901096?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This pull request introduces a fix for a bundler issue in the Node.js package. The core change is the use of eval('require') instead of the standard require function to handle hot reloading. This change ensures that the application can properly load dependencies and function correctly in the presence of a bundler, which was causing issues previously.
The change is focused on improving the integration of the Node.js package with the existing codebase by addressing a critical bug related to the bundler. The pull request is a non-breaking change that fixes an issue, without introducing any new features.
File Summaries
| File | Summary |
|---|---|
| packages/node/src/utils/hot-reload.ts | The code changes introduce a fix for a bundler issue by using eval('require') instead of the standard require function to handle hot reloading. This change ensures that the application can properly load dependencies and function correctly in the presence of a bundler. |
There was a problem hiding this comment.
Incremental Review
Comments posted: 2
Configuration
Squadron Mode: essential
Commits Reviewed
9ec100313de189b61a316f134926e37417e906e0...d6e8545701c1dacca831756e0093d26149434891
Files Reviewed
- packages/node/src/utils/hot-reload.ts
Files Ignored
These files were ignored due to the filter in the squadron.yaml file.
- .changeset/eighty-schools-smell.md
- .github/workflows/e2e-next-dev.yml
| //@ts-ignore | ||
| return typeof __non_webpack_require__ !== 'undefined' | ||
| ? __non_webpack_require__ | ||
| : require; | ||
| : eval('require'); |
There was a problem hiding this comment.
The current implementation using eval('require') poses security risks and may be blocked by Content Security Policy (CSP). A safer alternative would be to use Function constructor or check for the global require object:
| //@ts-ignore | |
| return typeof __non_webpack_require__ !== 'undefined' | |
| ? __non_webpack_require__ | |
| : require; | |
| : eval('require'); | |
| //@ts-ignore | |
| return typeof __non_webpack_require__ !== 'undefined' | |
| ? __non_webpack_require__ | |
| : Function('return require')(); |
Additionally, consider adding a comment explaining why this workaround is necessary for webpack bundling to help future maintainers understand the purpose of this code.
There was a problem hiding this comment.
Yep, we have this CSP problem :(
@ScriptedAlchemy
| @@ -12,7 +12,7 @@ | |||
| //@ts-ignore | |||
There was a problem hiding this comment.
The @ts-ignore comment should be replaced with a more specific @ts-expect-error to better document the expected type error. This helps catch issues if the type error is resolved in the future.
Description
Related Issue
Types of changes
Checklist