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

Allow custom resolver to be used with[out] moduleNameMapper #4174

Merged
merged 5 commits into from
Aug 31, 2017
Merged

Allow custom resolver to be used with[out] moduleNameMapper #4174

merged 5 commits into from
Aug 31, 2017

Conversation

midzelis
Copy link
Contributor

@midzelis midzelis commented Aug 1, 2017

Also, improve error message when moduleNameMapper doesn't resolve to a module.

Summary

  1. The existing moduleNameMapper implementation doesn't invoke the customer resolver with the custom resolver option. This makes it impossible to write custom resolvers that work in all cases.
  2. If a moduleNameMapper was not specified, then the custom resolver would not be run at all.

This PR solves the two problems above.

  1. It passes through the resolver configuration option so that the custom resolver could be used after the moduleMapMapper mapped the module. Additionally, the error message was improved with exactly which moduleName (including the mapped version) was causing the problem.
  2. If the moduleNameMapper isn't specified, it sends everything through to the resolver, unmapped. Additionally, the rootDir is passed to the resolver, to help it aid resolution, in case it may be dependent on the rootDir config option.

Test plan
Added 2 tests. Configuration documentation were updated with new argument passed to custom resolver.

@codecov-io
Copy link

codecov-io commented Aug 1, 2017

Codecov Report

Merging #4174 into master will increase coverage by 0.37%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4174      +/-   ##
==========================================
+ Coverage   56.17%   56.55%   +0.37%     
==========================================
  Files         191      191              
  Lines        6424     6426       +2     
  Branches        6        6              
==========================================
+ Hits         3609     3634      +25     
+ Misses       2812     2789      -23     
  Partials        3        3
Impacted Files Coverage Δ
...ackages/jest-cli/src/reporters/default_reporter.js 91.42% <ø> (ø) ⬆️
packages/jest-config/src/index.js 0% <ø> (ø) ⬆️
packages/jest-runtime/src/index.js 85.71% <ø> (ø) ⬆️
packages/jest-resolve/src/default_resolver.js 3.33% <ø> (ø) ⬆️
packages/jest-runner/src/run_test.js 4.44% <ø> (ø) ⬆️
packages/jest-cli/src/run_jest.js 0% <0%> (ø) ⬆️
packages/jest-resolve/src/index.js 36.69% <100%> (+22.67%) ⬆️
packages/jest-cli/src/reporters/utils.js 61.38% <100%> (ø) ⬆️

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 fab51f8...530504e. Read the comment docs.

@midzelis
Copy link
Contributor Author

midzelis commented Aug 8, 2017

@cpojer whats the status of this PR? Am I missing anything before it can committed?

1 similar comment
@midzelis
Copy link
Contributor Author

@cpojer whats the status of this PR? Am I missing anything before it can committed?

@cpojer
Copy link
Member

cpojer commented Aug 29, 2017

You'll need to rebase this diff, otherwise we'll be unable to merge it. I'm fine with these changes, and the cwd changes should fix a few other issues also. Thanks for contributing.

@midzelis midzelis closed this Aug 30, 2017
Min Idzelis added 4 commits August 30, 2017 17:11
…apper.

Also, improve error message when moduleNameMapper doesn't resolve to a module.
rootDir was modified after initialization to be process.cwd. In case of --runInBand,
The resolver was intialized before the rootDir value was altered. In case of parallel,
the Resolver was initialized in each spawned process using values of rootDir after
initialization.

Fix is to introduce a new config.cwd option that will be set to the parent processes
cwd. Adjust the console reporters to favor config.cwd over config.rootDir.
@midzelis midzelis reopened this Aug 30, 2017
@cpojer
Copy link
Member

cpojer commented Aug 31, 2017

Could you try pushing to this branch again? Seems like all builds failed for some reason.

@midzelis
Copy link
Contributor Author

Closing/reopening to trigger integration tests: circleci/appveyor/travis-ci

@midzelis midzelis closed this Aug 31, 2017
@midzelis midzelis reopened this Aug 31, 2017
@cpojer cpojer merged commit de74b38 into jestjs:master Aug 31, 2017
@cpojer
Copy link
Member

cpojer commented Aug 31, 2017

Thanks for your contribution. This was definitely a rough edge in Jest and a missing feature for as long as moduleNameMapper has existed, so I'm glad you contributed a fix for it. In the next few days (possibly tomorrow) I'll release a test release with this code under jest@test. Watch our for that!

@midzelis
Copy link
Contributor Author

midzelis commented Sep 1, 2017

Thanks for merging!

@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.

4 participants