Skip to content
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

Add option to define a custom resolver #2998

Merged
merged 4 commits into from
Mar 3, 2017

Conversation

dlmr
Copy link
Contributor

@dlmr dlmr commented Feb 24, 2017

Summary

This PR addresses #2837, implementing a new configuration option called resolver that can be used to implement a custom resolver.

This is a non breaking change.

Feedback about the approach is greatly appreciated and if this is a good way to solve #2837 in a general way. This works great for my use case.

Test plan

All existing tests pass along with the new one added for testing the new functionality.

@dlmr dlmr force-pushed the feature/custom-resolver branch 2 times, most recently from bbca42f to f512150 Compare February 24, 2017 14:48
@PAkerstrand
Copy link

Any updates for this? We need this type of functionality with linked dependencies during development...

@Phoenixmatrix
Copy link

I still don't know how Jest works internally that well. The resolver in this PR, does it take the original arguments and return a path to the new module, or does it return an actual module? (I'm having trouble telling since your mock returns a string, so it could be either).

For my use case, I need to be able to resolve paths to actual modules (the object itself), not just a path since the module could be in a totally different format (eg: AMD or even something custom), so CommonJS would not be able to load it.

@dlmr
Copy link
Contributor Author

dlmr commented Feb 27, 2017

This resolver is expected to return a path to where the module is located, it does not allow you to return the actual module.

If we need to also make it possible to directly return the module we need to do a larger change somewhere else, not in jest-resolve. I'm unsure where that change would need to be performed but my guess right now is jest-runtime.

Both of the dependencies that @cpojer mentions in #2837, jest-resolve-dependencies and jest-resolve, operate on paths.

@dlmr
Copy link
Contributor Author

dlmr commented Feb 27, 2017

@Phoenixmatrix Looked at it a little more now and it seems that jest-runtime is doing the actual loading of the modules. jest-runtime/src/index.js#L273-L294

It seems that this is already possible to replace with an existing configuration option, moduleLoader. jest-cli/src/runTest.js#L30

@codecov-io
Copy link

Codecov Report

Merging #2998 into master will decrease coverage by -0.01%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##           master    #2998      +/-   ##
==========================================
- Coverage   67.86%   67.86%   -0.01%     
==========================================
  Files         147      148       +1     
  Lines        5349     5352       +3     
==========================================
+ Hits         3630     3632       +2     
- Misses       1719     1720       +1
Impacted Files Coverage Δ
packages/jest-runtime/src/index.js 85.02% <ø> (ø)
packages/jest-config/src/validConfig.js 100% <ø> (ø)
packages/jest-config/src/normalize.js 85.9% <ø> (ø)
packages/jest-resolve/src/defaultResolver.js 0% <0%> (ø)
packages/jest-resolve/src/index.js 16.16% <100%> (+2.3%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b179ac1...1166570. Read the comment docs.

@dlmr
Copy link
Contributor Author

dlmr commented Mar 1, 2017

This PR is somewhat related to #2925 since it affects the use of resolve and make Jest less dependent on it.

@cpojer cpojer merged commit bb6ec69 into jestjs:master Mar 3, 2017
@dlmr dlmr deleted the feature/custom-resolver branch March 3, 2017 14:11
skovhus pushed a commit to skovhus/jest that referenced this pull request Apr 29, 2017
* Add option to define a custom resolver

* Add documentation for resolver config option

* Add test for when overriding default resolver

* Made it more clear in the docs what resolver is expected to return
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Add option to define a custom resolver

* Add documentation for resolver config option

* Add test for when overriding default resolver

* Made it more clear in the docs what resolver is expected to return
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants