Skip to content

fix: require.resolve() replaced as require()#12773

Merged
LingyuCoder merged 9 commits intoweb-infra-dev:mainfrom
intellild:issue_10479
Jan 21, 2026
Merged

fix: require.resolve() replaced as require()#12773
LingyuCoder merged 9 commits intoweb-infra-dev:mainfrom
intellild:issue_10479

Conversation

@intellild
Copy link
Copy Markdown
Contributor

@intellild intellild commented Jan 18, 2026

Summary

*require.resolve() and require.resolveWeak should not be replaced as require()

  • if require is a local variable, should not be transpiled as commonjs import

Related links

#10479

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link
Copy Markdown

netlify bot commented Jan 18, 2026

Deploy Preview for rspack canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit c3eb7bc
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/696f79024192d7000845424f

@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Jan 18, 2026
@intellild intellild changed the title fix issue 10479 fix require.resolve() replaced as require() Jan 18, 2026
@intellild intellild changed the title fix require.resolve() replaced as require() fix: require.resolve() replaced as require() Jan 18, 2026
@intellild intellild marked this pull request as ready for review January 18, 2026 13:21
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Jan 18, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing intellild:issue_10479 (c3eb7bc) with main (6f161b5)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@intellild intellild marked this pull request as draft January 18, 2026 14:23
@intellild intellild marked this pull request as ready for review January 20, 2026 12:46
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 fixes an issue where require.resolve() and require.resolveWeak() were incorrectly being replaced as require() calls, and prevents transpilation when require is a local variable rather than the global CommonJS require.

Changes:

  • Fixed identifier evaluation for require.resolve and require.resolveWeak to preserve correct identifier names
  • Added should_process_resolve() check to detect when require is a local variable and skip CommonJS transformation
  • Added test case to verify the fix works correctly

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs Fixed bug where require.resolve and require.resolveWeak were incorrectly evaluated with root_info as "require" instead of their actual names; added should_process_resolve() to check if require is a local variable; integrated check into resolve handlers
tests/rspack-test/normalCases/require/issue-10479/require.js Creates a local require variable using createRequire to test the fix
tests/rspack-test/normalCases/require/issue-10479/index.js Verifies that require.resolve() is not transformed when require is a local variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@LingyuCoder
Copy link
Copy Markdown
Contributor

LGTM, thanks

@LingyuCoder LingyuCoder merged commit 48885c7 into web-infra-dev:main Jan 21, 2026
58 checks passed
@chenjiahan
Copy link
Copy Markdown
Member

Thank you!

LingyuCoder pushed a commit that referenced this pull request Jan 21, 2026
* fix issue 10479

* fix issue 10479

* fix issue 10479

* revert incorrect changes

* fix `require.resolve()`` replaced as `require()`

* check if `require` is a local variable

* fix test

* fix test
chenjiahan pushed a commit that referenced this pull request Jan 21, 2026
* fix issue 10479

* fix issue 10479

* fix issue 10479

* revert incorrect changes

* fix `require.resolve()`` replaced as `require()`

* check if `require` is a local variable

* fix test

* fix test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants