fix: set MODULE_NOT_FOUND code when module is not resolved from paths #8487
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8487 +/- ##
==========================================
- Coverage 62.25% 62.24% -0.02%
==========================================
Files 266 266
Lines 10746 10748 +2
Branches 2625 2626 +1
==========================================
Hits 6690 6690
- Misses 3471 3473 +2
Partials 585 585
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @clarkdo, I agree the require implementation should emulate Node's as much as possible. According to Node's docs on this (plus I tried it in the REPL), this is not specific to require.resolve, but behaves the same with regular require. It would be great if we could also make Jest's require work that way. I think there's multiple places creating "module not found" errors, but maybe we could just create a helper function for that?
Also, since this is a documented feature in the regular require, I think we'll need a test for this :)
😸 Thanks for the review, changes are applied. I'll add test and changelog shortly |
There was a problem hiding this comment.
Node 6 failure looks super annoying, the lower error (stack trace looks different) probably causes the upper one (regex mismatch). Dropping the error subclass would probably be an easy although ugly fix. Or waiting until Node 6 is dropped 😄
|
snapshots need to be updated once again 🙈 |
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| export default class ModuleNotFoundError extends Error { |
There was a problem hiding this comment.
The Node 6 snapshot failures show pretty important change (not sure why it's different on higher versions though), since now we don't point to the place error happened, but to the error itself. We have a utility Error class called ErrorWithStack – maybe you could play with it and extend it directly?
There was a problem hiding this comment.
It looks like jest-resolve doesn't depend jest-utils, so can I use Error.captureStackTrace instead of extend ?
There was a problem hiding this comment.
If I can add dependency for jest-resolve, maybe we can move ModuleNotFoundError to jest-util as well
There was a problem hiding this comment.
Not sure about that, @SimenB?
|
We've dropped node 6. I merged in master, hopefully this passes CI and we can finally merge this 👍 |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Relate to nuxt/nuxt#5796
When we use
require.resolve(module, { paths: [...] }), the code of thrown exception is notMODULE_NOT_FOUNDwhich is inconsistent with Node.jsrequire.resolvebehaviour.Test plan