Jest: Different solution for babel-ignored files (#7765)

Created on 4 Feb 2019  路  4Comments  路  Source: facebook/jest

I am referring to the bug already reported here https://github.com/facebook/jest/issues/7765
More specifically, I'm questioning the solution just merged via https://github.com/facebook/jest/pull/7797

Problem

I believe that printing the warning is not a user-friendly solution to this. My reasoning for that is that in a Babel/Jest setup I have two concerns:

  • I want to transpile my code using babel and use babel's configuration system to tell it what files to transpile how.
  • I want to write and run my tests based on Jest.

Jest shouldn't care about my babel configuration. I already define that in potentially numerous/complex .babelrc/js/overrides/env/ignore.

The change in the PR #7797 forces me to re-define a distilled (i.e. "effective") version of my include/eclude/ignore configuration for Jest.

Proposed solution

I believe that loadBabelConfig() returning null indicates that Babel decided to explicitly not transpile a file.


(I was trying to add this to the PR before the merge but was minutes too late. Let's see, maybe I'm mistaken here)

Discussion

All 4 comments

I think #7797 is still better than what is in 24.0.0, since you'll get an error either way - the difference is we now give you an actionable error.


That said, I do agree with your reasoning (duplicating config is bad). However, we currently implement skipping transforms via transformIgnorePattern - there's no way for a transformer to opt-out on its own. So we either have to add a new API to transformers or do weird stuff in getCacheKey.

This is sorta similar to webpack aliases - we require you to copy them to moduleNameMapper as we have no intention of attempting to extract this information from webpack's config. However, we have a way tighter integration with Babel (and we bundle babel-jest), so it might make sense to not be as strict with it.

@thymikee @jeysal @rubennorte thoughts here?

We read quite a bunch of stuff from Babel already, so if there's a simple way to map only, ignore etc to transformIgnorePatterns that would be nice from our side. But it's one more thing to maintain. It would also open us up for further "integrations" like that and possibly making it even harder to upgrade to newer Babel versions (and we remember how painful migration to v7 was)

That said, I do agree with your reasoning (duplicating config is bad). However, we currently implement skipping transforms via transformIgnorePattern - there's no way for a transformer to opt-out on its own. So we either have to add a new API to transformers or do weird stuff in getCacheKey.

We read the Babel configuration in both the getCacheKey and the process methods in the transformer, so I don't see anything that prevents us from ignoring a file for which the configuration is null (except when instrumentation is enabled, when we must use the istanbul plugin).

@thymikee that's possible too, but we could do it in the process function anyway (if a file doesn't need to be transformed we can just return the original source). The only problem would be the opposite case, when transformIgnorePattern ignores a file and the transformer wouldn't.

Was this page helpful?
0 / 5 - 0 ratings