Ensure test modules can require.resolve absolute paths
#11943
Conversation
|
Hi @airhorns! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
airhorns
force-pushed
the
absolute-requires-without-paths
branch
from
d2c0c84 to
7ad3de1
Compare
airhorns
changed the title
require.resolve absolute paths
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Before this change, the runtime's `require.resolve` function would always try to resolve paths relative to the passed `paths` option. This prevents requiring paths which are absolute! Normal node `require.resolve` is fine requiring absolute paths, including when passed `{ paths: [] }`, so this updates the jest require algorithm to detect absolute paths and handle them without trying to make the path relative to the passed `paths` option. This is similar to how node's own require.resolve works, see https://github.com/nodejs/node/blob/05002373176e8758c8c604f06659e171de4ca902/lib/internal/modules/cjs/loader.js#L240-L241
Fixes jestjs#11927.
airhorns
force-pushed
the
absolute-requires-without-paths
branch
from
7ad3de1 to
52bd70f
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Codecov Report
@@ Coverage Diff @@
## main #11943 +/- ##
=======================================
Coverage 68.75% 68.76%
=======================================
Files 322 322
Lines 16615 16619 +4
Branches 4793 4795 +2
=======================================
+ Hits 11424 11428 +4
Misses 5159 5159
Partials 32 32
Continue to review full report at Codecov.
|
|
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
Before this change, the runtime's
require.resolvefunction would always try to resolve paths relative to the passedpathsoption. This prevents requiring paths which are absolute! Normal noderequire.resolveis fine requiring absolute paths, including when passed{ paths: [] }, so this updates the jest require algorithm to detect absolute paths and handle them without trying to make the path relative to the passedpathsoption. This is similar to how node's own require.resolve works, see https://github.com/nodejs/node/blob/05002373176e8758c8c604f06659e171de4ca902/lib/internal/modules/cjs/loader.js#L240-L241Fixes #11927.
Test plan
The added unit test fails on
mainand passes on this branch. Woop woop!