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

Update regex replace to not require full path matching #1723

Merged
merged 3 commits into from
Sep 17, 2016

Conversation

zackargyle
Copy link
Contributor

@zackargyle zackargyle commented Sep 16, 2016

Summary
The config option moduleNameMapper currently requires a full path match. This is because it is doing a regex replace on the original module name. This diff changes that so that instead we are using the mapped name directly. This makes it much easier to match any file of a specific type: like '\\.css'. There will be a small backwards compatability possibility for matches that depended on the previously incorrect behavior. (I found one such case in the Pinterest codebase that was resolved by using requireActual instead, which was more correct anyway).

The previous functionality is:

Before

moduleName regex mappedName Resolution
module/name/test /module/name/(.*)/ mapped_module_$1.js mapped_module_test.js
test/style.css /.css$/ mocks/ManuallyMocked test/style__mocks__/ManuallyMocked

After

moduleName regex mappedName Resolution
module/name/test /module/name/(.*)/ mapped_module_$1.js mapped_module_test.js
test/style.css /.css$/ mocks/ManuallyMocked mocks/ManuallyMocked

Test plan
I ran the changes over the 900 Pinterest tests as well as the jest codebase tests. I also added a test to make sure that the path is used correctly.
#1624

@ghost ghost added the CLA Signed ✔️ label Sep 16, 2016
@zackargyle
Copy link
Contributor Author

Doh. I'll fix up the flow issues.

@codecov-io
Copy link

Current coverage is 89.94% (diff: 100%)

Merging #1723 into master will not change coverage

@@             master      #1723   diff @@
==========================================
  Files            31         31          
  Lines          1144       1144          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1029       1029          
  Misses          115        115          
  Partials          0          0          

Powered by Codecov. Last update ec782b5...6941823

@cpojer
Copy link
Member

cpojer commented Sep 17, 2016

This looks great. Nice work @zackargyle

@cpojer cpojer merged commit a41450b into jestjs:master Sep 17, 2016
mthmulders pushed a commit to mthmulders/jest that referenced this pull request Oct 10, 2016
* Update regex replace to not require full path matching

* Fix flow issue

* Code style
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Update regex replace to not require full path matching

* Fix flow issue

* Code style
@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 14, 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.

3 participants