fix(core): use default import from node:module to avoid naming conflict #1433
Conversation
✅ Deploy Preview for rslib ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
e3369c2 to
ff006d0
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a potential naming conflict issue by switching from a named import to a default import when injecting the require shim in ESM outputs. The change prevents collisions when user code defines a variable named createRequire.
Key changes:
- Updated the require shim to use
import __rslib_shim_module__ from 'node:module'instead ofimport { createRequire } from "node:module" - Updated test snapshots to reflect the new shim format
- Updated both English and Chinese documentation with the new code examples
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/core/src/plugins/shims.ts |
Changed the require shim from named import to default import with namespaced identifier |
tests/integration/shims/index.test.ts |
Updated test snapshot to match new shim format |
website/docs/en/config/lib/shims.mdx |
Updated English documentation with new code example |
website/docs/zh/config/lib/shims.mdx |
Updated Chinese documentation with new code example |
After thoroughly reviewing all changes, I found no issues with this PR. The implementation is correct, all tests have been properly updated, and the documentation accurately reflects the changes. The solution effectively addresses the naming conflict issue while maintaining compatibility and following established conventions in the codebase.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR updates
requireShimto use the default import fromnode:moduleinstead of a named import.In the previous PR (#1431), we switched to
import { createRequire } from "node:module". However, if the user's code also defines a variable namedcreateRequire, this shim injection could cause a naming conflict.see https://github.com/rstackjs/rstack-ecosystem-ci/actions/runs/20743602225/job/59555591523
This PR resolves the issue by using a namespaced default import:
Related Links
Checklist