-
-
Notifications
You must be signed in to change notification settings - Fork 733
fix: treat HMR's warnUnexpectedRequire as undefined module factory
#12016
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for rspack canceled.
|
📦 Binary Size-limit
🙈 Size remains the same at 47.81MB |
CodSpeed Performance ReportMerging #12016 will not alter performanceComparing Summary
|
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 error throwing behavior to the Hot Module Replacement (HMR) runtime when a disposed module is required. Previously, the system only warned about accessing disposed modules; now it throws an error after the warning to prevent continued execution with undefined module factories.
Key changes:
- Modified the
warnUnexpectedRequirefunction to throw an error after logging the warning - Updated test expectations to handle the newly thrown error
- Updated snapshot tests to reflect increased bundle sizes due to the added error handling code
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/rspack_plugin_runtime/src/runtime_module/runtime/javascript_hot_module_replacement.js |
Added throw Error() statement to warnUnexpectedRequire function to prevent execution with undefined factory |
tests/rspack-test/hotCases/runtime/require-disposed-module-warning/index.js |
Wrapped getInner() call in try-catch to handle the newly thrown error |
tests/rspack-test/hotCases/sharing/share-plugin/__snapshots__/web/1.snap.txt |
Updated snapshot to reflect increased bundle size and added error throwing code |
tests/rspack-test/hotCases/module/access-dropped-module/__snapshots__/web/1.snap.txt |
Updated snapshot to reflect increased bundle size and added error throwing code |
tests/rspack-test/hotCases/disposing/runtime-independent-filename/__snapshots__/web/1.snap.txt |
Updated snapshot to reflect increased bundle size and added error throwing code |
tests/rspack-test/hotCases/disposing/remove-chunk-with-shared-in-other-runtime/__snapshots__/web/1.snap.txt |
Updated snapshot to reflect increased bundle size and added error throwing code |
tests/rspack-test/hotCases/chunks/update-chunk-loading-runtime/__snapshots__/web/1.snap.txt |
Updated snapshot to reflect increased bundle size and added error throwing code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
close #11922
This case is similar to #11324 (comment).
A small difference is that the missing module's factory is replaced with an HMR runtime warning function. So when the required module is not ready yet, users have to reload to recover from this error.
As a workaround, we throw the specified error
RuntimeError: factory is undefinedto trigger rspack-contrib/rspack-plugin-react-refresh#50 to do a browser reload.By the way, we are planning a new HMR runtime to ensure a hot updated module's dependence chunks are loaded before it's required.
Related links
Checklist