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

Clarify behavior of transformIgnorePatterns when including multiple patterns. #11796

Merged
merged 6 commits into from
Sep 29, 2021

Conversation

AndrewSouthpaw
Copy link
Contributor

Summary

I recently bumped into an mystery working with transformIgnorePatterns, where I had two negative lookaheads for node_modules, like so:

{
  "transformIgnorePatterns": [
    "node_modules/(?!(foo)/)",
    "node_modules/(?!(bar)/)"
  ]
}

As I came to realize, this meant that both node_modules/foo and node_modules/bar folders were not transformed, because they would always match in the other pattern. I missed the significance of the line:

If the file path matches any of the patterns, it will not be transformed.

Maybe this is completely evident to other people, but it does seem like an easy detail to miss while working with it.

I figured I'd share these suggestions, and if the team thinks it's a worthwhile addition, I can add the changes in the other doc versions.

Test plan

Checked running website locally.

@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2021

Codecov Report

Merging #11796 (35e6dc9) into master (98f10e6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11796   +/-   ##
=======================================
  Coverage   69.04%   69.04%           
=======================================
  Files         312      312           
  Lines       16366    16366           
  Branches     4746     4746           
=======================================
  Hits        11300    11300           
  Misses       5039     5039           
  Partials       27       27           

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 98f10e6...35e6dc9. Read the comment docs.

Copy link
Contributor

@sigveio sigveio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AndrewSouthpaw! Seems like a good addition to clarify this considering that not everyone knows how Jest treats this under the hood, or are experienced with RegExp. 🙂

Here's a few things I spotted looking over your PR.

docs/TutorialReactNative.md Outdated Show resolved Hide resolved
docs/Configuration.md Outdated Show resolved Hide resolved
docs/Configuration.md Outdated Show resolved Hide resolved
docs/TutorialReactNative.md Outdated Show resolved Hide resolved
@AndrewSouthpaw
Copy link
Contributor Author

@sigveio thanks for the thoughtful and thorough feedback! I'm glad you think the additions are useful.

I've pushed up some changes, let me know if they address your concerns. Once it looks good, I'll copy the changes to the versioned docs as well.

Copy link
Contributor

@sigveio sigveio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigveio thanks for the thoughtful and thorough feedback! I'm glad you think the additions are useful.

You are welcome! 😌

I've pushed up some changes, let me know if they address your concerns.

Thanks - I've added some follow-up comments.

With regards to the patterns; what Jest does internally is essentially this after loading the config:

https://github.com/facebook/jest/blob/98f10e698ae986c19fef2d8117be2341bcfb8f7f/packages/jest-config/src/normalize.ts#L325-L327

To substitute in <rootDir> for those using that in their patterns. And then in the transformer:

https://github.com/facebook/jest/blob/98f10e698ae986c19fef2d8117be2341bcfb8f7f/packages/jest-transform/src/ScriptTransformer.ts#L967

To construct a combined and appropriately escaped pattern in the form of a RegExp object.

So if you'd like to verify your patterns, you could do the same: run it through the RegExp() constructor and console.log() it to get the string. And use a testing tool like this to try it out: https://regex101.com/r/JsLIDM/1

(Note that I added the flags gm in the tool there just to allow multiple matches across multiple lines for the purpose of that demonstration. Lines where node_modules is colored = what Jest would ignore for transformations.)

docs/TutorialReactNative.md Outdated Show resolved Hide resolved
docs/Configuration.md Outdated Show resolved Hide resolved
docs/Configuration.md Show resolved Hide resolved
docs/Configuration.md Show resolved Hide resolved
@AndrewSouthpaw
Copy link
Contributor Author

@sigveio Ready for another review! I copied the changes over, but the actual tweaks can be seen in 5b13a37.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is awesome, thank you!! Since this was opened I've added a 27.2 version - would you mind merging in main and updating that as well? 🙂

…nsform-ignore-patterns

* 'main' of https://github.com/facebook/jest: (38 commits)
  chore: update `npm` instructions in README (jestjs#11890)
  chore: force patched version of ansi-regex (jestjs#11889)
  chore: update lockfile after publish
  v27.2.1
  chore: update changelog for release
  make the syntax error message more helpful for TS users (jestjs#11807)
  fix: include path to test file in "after teardown" error (jestjs#11885)
  docs: add link to the typescript integration instructions (jestjs#11806)
  fix: properly return mocks when using jest.isolatedModules (jestjs#11882)
  chore: remove node version pinning from CI (jestjs#11866)
  chore: remove node 13 as condition in some tests (jestjs#11880)
  chore: fix typo in docs
  chore: update lockfile after publish
  v27.2.0
  chore: roll new website version
  chore: update changelog for release
  chore: update lock
  feat: support `conditions` from test environments (jestjs#11863)
  Revert "chore: remove unneeded yarn patch for react native (jestjs#11853)"
  chore: supress experimental warnings in tests
  ...
@AndrewSouthpaw
Copy link
Contributor Author

Done! Thanks for the heads up @SimenB!

@SimenB SimenB merged commit fb0e09c into jestjs:main Sep 29, 2021
@AndrewSouthpaw AndrewSouthpaw deleted the docs-transform-ignore-patterns branch October 13, 2021 03:44
@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 Nov 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.

5 participants