Skip to content

fix(esm_lib_plugin): should set original module for get_binding#12584

Merged
JSerFeng merged 1 commit intomainfrom
fix/require-external
Dec 30, 2025
Merged

fix(esm_lib_plugin): should set original module for get_binding#12584
JSerFeng merged 1 commit intomainfrom
fix/require-external

Conversation

@JSerFeng
Copy link
Contributor

Summary

Should set correct original module for required module, so that required module will render before access

Related links

Checklist

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

Copilot AI review requested due to automatic review settings December 30, 2025 07:36
@netlify
Copy link

netlify bot commented Dec 30, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 2a33f8d
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/695380ecc2797b0008c56b58

@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Dec 30, 2025
Copy link
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 a bug in the ESM library plugin where required modules were not being associated with their originating module. By setting the correct original module for required modules via the from parameter in add_require calls, the fix ensures that required modules are rendered before they are accessed.

  • Updated two add_require calls in get_binding to pass the from parameter instead of None
  • Added test case demonstrating the fix with module import ordering
  • Updated inline code formatting in documentation

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/rspack_plugin_esm_library/src/link.rs Fixed two locations where add_require was called with None instead of the from parameter, ensuring proper module rendering order
crates/rspack_plugin_esm_library/src/chunk_link.rs Improved documentation formatting by adding backticks around inline code example
tests/rspack-test/esmOutputCases/interop/contains-cjs/lib.js New test file containing the test logic that was moved from index.js
tests/rspack-test/esmOutputCases/interop/contains-cjs/index.js Simplified to just import the lib module, testing the module ordering fix
tests/rspack-test/esmOutputCases/interop/contains-cjs/__snapshots__/esm.snap.txt Updated snapshot showing correct module comment ordering (// ./lib.js before // ./index.js)

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

@JSerFeng JSerFeng enabled auto-merge (squash) December 30, 2025 07:44
@github-actions
Copy link
Contributor

Rsdoctor Bundle Diff Analysis

Found 5 projects in monorepo, 1 project with changes.

📊 Quick Summary
Project Total Size Change
react-10k 5.7 MB 0
react-1k 823.4 KB 0
react-5k 2.7 MB 0
rome 984.3 KB -23.0 B (-0.0%)
ui-components 2.1 MB 0
📋 Detailed Reports (Click to expand)

📁 rome

Path: ../build-tools-performance/cases/rome/dist/rsdoctor-data.json

📌 Baseline Commit: 4a0f619226 | PR: #12582

Metric Current Baseline Change
📊 Total Size 984.3 KB 984.4 KB -23.0 B (-0.0%)
📄 JavaScript 984.3 KB 984.4 KB -23.0 B (-0.0%)
🎨 CSS 0 B 0 B 0
🌐 HTML 0 B 0 B 0
📁 Other Assets 0 B 0 B 0

📦 Download Diff Report: rome Bundle Diff

Generated by Rsdoctor GitHub Action

@github-actions
Copy link
Contributor

📦 Binary Size-limit

Comparing 2a33f8d to chore: bump swc_core from 52.0.0 to 54.0.0 and swc_experimental from 0.3.5 to 0.4.0 (#12582) by CPunisher

🙈 Size remains the same at 47.85MB

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

CodSpeed Performance Report

Merging #12584 will not alter performance

Comparing fix/require-external (2a33f8d) with main (4a0f619)

Summary

✅ 16 untouched
⏩ 1 skipped1

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.

@JSerFeng JSerFeng merged commit 2ecf0ec into main Dec 30, 2025
87 of 89 checks passed
@JSerFeng JSerFeng deleted the fix/require-external branch December 30, 2025 08:09
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) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants