jest-runtime: move babel-core to peer dependencies

[vlad-zhukov]

Summary

It's still not possible to use Jest with Babel 7 due to some kind of a conflict between Babel versions because jest-runtime requires babel-core@^6. Moving babel-core to peer dependencies such as in babel-jest resolves the problem.

Test plan

Existing tests should pass.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[codecov-io]

Codecov Report

Merging #4557 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4557   +/-   ##
=======================================
  Coverage   56.16%   56.16%           
=======================================
  Files         185      185           
  Lines        6285     6285           
  Branches        3        3           
=======================================
  Hits         3530     3530           
  Misses       2754     2754           
  Partials        1        1

---

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 80bd4cd...8aac13a. Read the comment docs.

[SimenB]

This change means that you get a peer dep warning if you do not depend on babel-core somewhere in your tree.

Then you have to install it manually, or tests WILL fail to execute.

 FAIL  ./test.js
  ● Test suite failed to run

    Cannot find module 'babel-core'

      at Function.Module._resolveFilename (module.js:527:15)
      at Function.Module._load (module.js:476:23)
      at Module.require (module.js:568:17)
      at require (internal/module.js:11:18)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.684s
Ran all test suites.

Not sure how to handle it. It's not really nice that people not using babel at all will have to install babel-core manually just to be able to use Jest.

[SimenB]

@hzoo Your input would be greatly appreciated here.

Issue:

• Jest depends on babel-core (as it uses babel for code coverage and hoisting in order to do mocks)
• Jest also picks up a project's babel config so things just work:tm: without a require hook or something like that
• we want to allow users to use babel@7 if they want
• we don't want to force users to upgrade from 6 to 7 in order to use jest
• we don't want to force users who don't already depend on babel (lots of node only projects for instance, and not every web project uses babel either) to install babel-core

Is there some way you know of to achieve the following:

• If the user has installed either babel-core@6 or babel-core@7, use that
• If the user has not installed babel-core at all, install babel-core@latest somehow

[frenzzy]

Babel packages were moved to npm scope since v7 beta 5, could we add support for future versions of babel by using latest babel if available? Maybe something like this:

let transform;
let babelTransform;
try {
  ({ transform, babelTransform } = require('@babel/core'));
} catch (error) {
  ({ transform, babelTransform } = require('babel-core'));
}
// ...

[SimenB]

Auch, that actually makes it even worse for us.

@cpojer maybe we add babel-core as a dep in jest, and do module.parent.require? Then we get the user's dep if they define one, but our own if they don't (as it's hoisted to the top-level)

[loganfsmyth]

Since jest-runtime on its own doesn't read the user's config (babelrc: false), it doesn't seem like it should need to transition to Babel 7 at any particular time to me. This issue doesn't seem to clearly spell out what was broken when used with babel 7 at the project root. 6 should have just been installed in a nested node_modules right?

we don't want to force users who don't already depend on babel (lots of node only projects for instance, and not every web project uses babel either) to install babel-core

Does that apply to jest-runtime only, or babel-jest too?

For babel-jest things do get more complicated because of the @babel scope. The option I've thrown out here, is that we make kind of "bridge" packages to allow for gradual transition for packages that have the peerDep on babel-core instead of @babel/core. Let me know if this makes sense.

For babel-jest we expand its peerDeps with 7.0.0-bridge

"babel-core": "^6.0.0 || ^7.0.0-alpha || ^7.0.0-beta || ^7.0.0 || 7.0.0-bridge"

then this bridge package could have its own peerDependency on @babel/core and essentially do module.exports = require("@babel/core"); so people who want to use Babel 7.x on existing Jest versions would do

npm install babel-jest [email protected] @babel/core

to get all three packages. Otherwise the existing npm install babel-jest babel-core would still work for 6.x.

Then we could also do the opposite so once packages change the peerDep to @babel/core people wanting to use 6.x could do

npm i babel-jest @babel/[email protected] babel-core`

thoughts?

[vlad-zhukov]

@loganfsmyth The problem was that tests failed due to syntax errors (such as ones that point to import declarations) which means that babel in jest didn't do it's job.

6 should have just been installed in a nested node_modules right?

It kind of did, but yarn weirdly hoisted a few deeply nested packages (haven't tested in npm).

Also what about .babelrc and .babelrc.js?

[SimenB]

Does that apply to jest-runtime only, or babel-jest too?

Both. Jest uses babel for a couple of things:

• it hoists mocking calls so they can be placed wherever (https://github.com/kentcdodds/how-jest-mocking-works)
• It's also how we do code coverage (istanbul plugins).

(I think that's it)

I like the idea of a bridge module. What I really want is to use the user's installed babel-core if it exists. If it does not, I want to provide my own. I guess a bridging module combined with module.parent.require(which is admittedly pretty hacky) would solve it?

[loganfsmyth]

Let me break down my thoughts a bit more. The two places:

• In jest-runtime
  For this case, you're passing babelrc: false, with a hardcoded list of a single plugin. Given that, the specific version of Babel used in this case is 100% up to you because you control both the inputs and outputs of Babel's configuration. To me that means that this PR is incorrect, because you control the inputs and outputs, you should be safe to hard-code a specific Babel version. There is nothing to gain by letting the user control the version of Babel in use here.

• In babel-jest
  This is the harder case, because you don't pass babelrc: false so it will use the user's .babelrc config. In my opinion the peerDependency of "babel-core": "6.x | 7.0.0-beta | 7.0.0-bridge" seems reasonable here, because babel-jest is already a package that users have to opt into separately from Jest itself, so requiring them to choose the Babel version for this usecase makes sense to me. If they choose v6, then npm will be able to flatten the v6 version from jest-runtime into the root and they will share the same core, and if the user installs v7 as a peerDependency of babel-jest, then jest-runtime will end up with v6 installed as its own nested dependency.

I think given that, you can just rely on the user to install the correct babel-core that correlates with their .babelrc (which I'd assume will already be installed for use in babel-loader or babel-cli anyway). I don't know how I feel about automatically using the user's babel-core if it exists, but you could certainly add some logic like that to warn the user somehow if they've installed @babel/core in their root, but forgotten to install the bridge package meaning babel-jest would still try to use v6.

[borela]

Was the bridge package created?

[SimenB]

@cpojer thoughts here?

[cpojer]

I think it would be great if we could stop using babel-jest in jest-runtime but instead use babel for code coverage more directly. Could we make that happen?

[borela]

@cpojer We could make a bridge package babel-jest-scoped or babel-jest-v7 just to make it work with the scoped babel packages for now and then later try to eliminate babel jest in a major release.

[cpojer]

I'm not sure what the best solution is but it makes sense to me that:

• jest-runtime uses one version of babel to make code coverage work
• babel-jest works with either babel 6 or babel 7

We can make the change to both right now because the next Jest release will be a major.

[borela]

I personaly liked @loganfsmyth's idea of having a bridge package as a way to select which core to load. For now I edited my local jest package to load @babel/core instead of babel-core and it worked flawlessly.

[loganfsmyth]

I've just now published [email protected] that has its own peerDep on @babel/core@^7.0.0-0 so you should now be able to have babel-jest do

"peerDependencies": {
  "babel-core": "6.x | ^7.0.0-0"
}

as a generic block to support all of 6.x and 7.x, including the 7.x alphas, betas, and the new bridge package. You'll just need some docs for users on how to install the bridge package, if they want to use @babel/core as

npm install babel-jest babel-core@^7.0.0-0 @babel/core

[alfondotnet]

Awesome guys!, this is exciting!.

For the time being, I have it working with jest@test and babel-7-jest

[SimenB]

Do you need the second package?

It would be great to have an example in the docs with how to use babel 7 and jest together

[danez]

I added it in my PR see https://facebook.github.io/jest/docs/en/getting-started.html#using-babel
This already works with latest stable version, but you get peerDependency warnings

[loganfsmyth]

I believe this has been published in [email protected] right? So babel-7-jest shouldn't be needed.

[SimenB]

babel-runtime and babel-jest betas were published yesterday, and contain the fixes, yes. I haven't tested it, though

[alfondotnet]

hmm I might be dumb then, because:

  "devDependencies": {
    "@babel/cli": "^7.0.0-beta.32",
    "@babel/core": "^7.0.0-beta.32",
    "@babel/node": "^7.0.0-beta.32",
    "@babel/plugin-syntax-dynamic-import": "^7.0.0-beta.32",
    "@babel/plugin-syntax-object-rest-spread": "^7.0.0-beta.32",
    "@babel/plugin-transform-flow-strip-types": "^7.0.0-beta.32",
    "@babel/preset-env": "^7.0.0-beta.32",
    "@babel/register": "^7.0.0-beta.32",
    "babel-7-jest": "^21.3.1",
    "babel-eslint": "^8.0.2",
    "eslint": "^4.1.0",
    "eslint-config-prettier": "^2.8.0",
    "eslint-plugin-flowtype": "^2.34.0",
    "eslint-plugin-jest": "^21.3.2",
    "flow-bin": "^0.59.0",
    "flow-remove-types": "^1.2.1",
    "flow-typed": "^2.2.3",
    "jest": "^21.3.0-beta.9",
    "prettier": "^1.4.4"
  }

➜  test git:(master) ✗ yarn test
yarn test v1.0.2
$ jest --runInBand src/
 PASS  src/__tests__/utils.test.js

 test git:(master) ✗ yarn remove babel-7-jest; yarn add [email protected] --dev

➜  test git:(master) ✗ yarn test
yarn test v1.0.2
$ jest --runInBand src/
 FAIL  src/__tests__/utils.test.js
  ● Test suite failed to run

    TypeError: Cannot read property 'loose' of undefined (While processing preset: "/Users/alfonsoperez/repos/test/node_modules/@babel/preset-env/lib/index.js")

[danez]

you are missing babel-core@^7.0.0-0

[alfondotnet]

@danez, I wasn't sure about that one :D, and I installed instead @babel/core since

➜  test git:(master) ✗ npm install --save-dev babel-jest babel-core@^7.0.0-0 @babel/core

zsh: no matches found: babel-core@^7.0.0-0

??

[danez]

That seems to come from your zsh, that tries to do something with the command, maybe need to escape the ^

[alfondotnet]

Ohh you are right!, I missed that, thanks!, and sorry for the noise then!

[SimenB]

Should we put the command in quotes in the readme so it's more copy-pasteable?

PR welcome! 🙂

[ovidiuch]

It's great that we have a workaround.

But is there any plan to make babel-jest work with @babel/core directly and not have to install babel-core@bridge as well? If so I'd like to follow that thread. Thanks.

[SimenB]

Not for now as that would stop babel 6 from working. After babel 7 is promoted to stable that might change, but for now we'll keep the current behaviour

[ryami333]

Gidday @SimenB - now that Babel 7 is stable has this progressed at all? babel-jest is now the only dependency in my project with babel-* peer-dependencies since I've upgraded to Babel 7, and it would be nice to be able to remove that.

[SimenB]

See #6949

[chaimleib]

What if we could use @babel/jest? This would make the bridge unnecessary, and jest-runtime could import babel via @babel/jest.

In other words, how about if we combine the bridge with babel-jest to produce @babel/jest, following the new babel 7 naming convention?

[SimenB]

Install Jest 24, released today, and you don't need the bridge module anymore - just use @babel/core

[sumantka]

@SimenB could you expand on 'just use @babel/core' What does that mean. I've installed @babel/core and tried almost everything and at the point of giving up.

[SimenB]

Please see the docs: https://jestjs.io/docs/en/getting-started#using-babel

Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

[github-actions]

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.