-
-
Notifications
You must be signed in to change notification settings - Fork 9.2k
fix: add null check for module existence with helpful error message #20221
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
fix: add null check for module existence with helpful error message #20221
Conversation
🦋 Changeset detectedLatest commit: 186250b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
| Template.indent([ | ||
| "throw new Error('Cannot find module with id \"' + moduleId + '\". The module might not be loaded or was removed.');" | ||
| ]), | ||
| "}", |
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.
It should be only for development mode
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.
Development-only error:
Wrapped the helpful error in a check using outputOptions.pathinfo, which is only true in development (see lib/config/defaults.js, where output.output.pathinfo is set based on development). So the error is thrown only in dev builds.
| return `__webpack_modules__[${moduleIdExpr}](${args.join(",")});`; | ||
| return `__webpack_modules__[${moduleIdExpr}](${args.join( | ||
| "," | ||
| )});`; |
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.
Remove unrelated changes
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.
The webpack_modules${moduleIdExpr}; calls are unchanged now. The only change in
JavascriptModulesPlugin.js
is the new development-only check right after the cache lookup and before creating the module.
alexander-akait
left a comment
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.
Also please add a test case
| "// Check if module exists (development only)", | ||
| "if (__webpack_modules__[moduleId] === undefined) {", | ||
| Template.indent([ | ||
| "throw new Error('Cannot find module with id \"' + moduleId + '\". The module might not be loaded or was removed.');" |
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.
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.
Updated! The error now matches Node.js format:
Message: Cannot find module ''
Error code: MODULE_NOT_FOUND
Also rebased on latest main.
Adds a check before module execution to provide a helpful error message when a module ID is not found in __webpack_modules__, instead of the cryptic 'Cannot read properties of undefined (reading call)' error. Fixes webpack#18021
7e95e7f to
a203dbd
Compare
CodSpeed Performance ReportMerging #20221 will not alter performanceComparing Summary
|
alexander-akait
left a comment
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.
Looks good, let's fix tests and we can merge, thanks
57b424b to
dd198d7
Compare
|
Please update snapshots for cached too |
dd198d7 to
4455b9f
Compare
|
Just run |
4455b9f to
ac878f2
Compare
|
Thanks |
|
This PR is packaged and the instant preview is available (22c48fb). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@22c48fb
yarn add -D webpack@https://pkg.pr.new/webpack@22c48fb
pnpm add -D webpack@https://pkg.pr.new/webpack@22c48fb |

Adds a check before module execution to provide a helpful error message when a module ID is not found in
__webpack_modules__, instead of the crypticCannot read properties of undefined (reading 'call')error.Fixes #18021
Summary
When a module ID is missing from
__webpack_modules__, the webpack runtime currently tries to execute it and crashes with:TypeError: Cannot read properties of undefined (reading 'call')This error originates from the generated runtime in JavascriptModulesPlugin and does not clearly tell users that the actual problem is a missing/unloaded module.
This PR adds a defensive check before executing the module factory:
__webpack_modules__[moduleId]isundefined, we now throw a descriptiveErrorexplaining that the module with the given ID cannot be found and may not be loaded or was removed.What kind of change does this PR introduce?
Did you add tests for your changes?
This change is a small defensive check inside the runtime path and is mainly focused on improving the error message, not changing the control flow. The existing runtime/missing-module tests already cover the behavior that an
Erroris thrown when a module is missing; this PR only makes the message clearer.I am happy to add a dedicated test if maintainers can suggest the preferred place and pattern (e.g. extending test/cases/runtime/error-handling or one of the
missing-module-*runtime cases) to assert the new message.Does this PR introduce a breaking change?
The behavior (throwing an error when a module is missing) stays the same; only the error message becomes more descriptive.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
No documentation updates are required. This change only improves the runtime error message surfaced to users when a module cannot be found.