chore: upgrade to jest 24 #15778

SimenB · 2019-05-30T12:33:30Z

This won't work on CI since pretty-format uses react-is internally, and since yarn finds react-is in this repo, it won't install it. I don't know how to tell yarn to install a version of react-is into pretty-formats node_modules? To test this locally, I just did cp -R ../jest/node_modules/react-is node_modules/pretty-format/node_modules which worked fine. Maybe @arcanis can help?

All tests pass locally 🙂

~~This contains #15779 with the upgrade to jest 24 applied on top of it~~

package.json

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js

packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js

SimenB · 2019-05-30T12:34:52Z

packages/react-test-renderer/src/__tests__/ReactTestRendererAsync-test.js

@@ -160,7 +160,7 @@ describe('ReactTestRendererAsync', () => {

      expect(() =>
        expect(Scheduler).toFlushAndYieldThrough(['foo', 'baz']),
-      ).toThrow('Expected value to equal:');
+      ).toThrow('// deep equality');


we've changed the error messages

packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee

packages/react/src/__tests__/createReactClassIntegration-test.js

scripts/jest/config.base.js

SimenB · 2019-05-30T12:36:11Z

scripts/jest/config.base.js

@@ -12,12 +12,13 @@ module.exports = {
    '.*': require.resolve('./preprocessor.js'),
  },
  setupFiles: [require.resolve('./setupEnvironment.js')],
-  setupTestFrameworkScriptFile: require.resolve('./setupTests.js'),
+  setupFilesAfterEnv: [require.resolve('./setupTests.js')],


renamed option

SimenB · 2019-05-30T12:36:43Z

scripts/jest/preprocessor.js

@@ -53,6 +53,9 @@ module.exports = {
    if (filePath.match(/\.ts$/) && !filePath.match(/\.d\.ts$/)) {
      return tsPreprocessor.compile(src, filePath);
    }
+    if (filePath.match(/\.json$/) ) {


Jest passes json files through transformers now, and babel didn't like it 🙂

scripts/jest/preprocessor.js

sizebot · 2019-05-30T12:37:20Z

	Fails
🚫	node` failed.

Log

Error:  Error: /home/circleci/project/babel.config.js: Error while loading config - .sourceFileName must be a string, or undefined
    at assertString (/home/circleci/project/node_modules/@babel/core/lib/config/validation/option-assertions.js:118:11)
    at Object.keys.forEach.key (/home/circleci/project/node_modules/@babel/core/lib/config/validation/options.js:107:5)
    at Array.forEach (<anonymous>)
    at validateNested (/home/circleci/project/node_modules/@babel/core/lib/config/validation/options.js:83:21)
    at validate (/home/circleci/project/node_modules/@babel/core/lib/config/validation/options.js:74:10)
    at loadPrivatePartialConfig (/home/circleci/project/node_modules/@babel/core/lib/config/partial.js:66:50)
    at loadFullConfig (/home/circleci/project/node_modules/@babel/core/lib/config/full.js:43:39)
    at transformSync (/home/circleci/project/node_modules/@babel/core/lib/transform.js:41:38)
    at Object.transform (/home/circleci/project/node_modules/@babel/core/lib/transform.js:22:38)
    at Object.exports.babelify (/home/circleci/project/node_modules/danger/distribution/runner/runners/utils/transpiler.js:71:24)

Generated by 🚫 dangerJS

arcanis · 2019-05-30T12:53:57Z

since yarn finds react-is in this repo, it won't install it. I don't know how to tell yarn to install a version of react-is into pretty-formats node_modules?

Not sure I understand - you mean that /node_modules/react-is exists, so Yarn doesn't create /node_modules/pretty-format/node_modules/react-is? Yarn should only do this if the version at the top-level is compatible with the version requested by pretty-format. Isn't it the case?

SimenB · 2019-05-30T12:58:58Z

Not sure I understand - you mean that /node_modules/react-is exists, so Yarn doesn't create /node_modules/pretty-format/node_modules/react-is?

Correct.

Yarn should only do this if the version at the top-level is compatible with the version requested by pretty-format. Isn't it the case?

It is semver compatible, but the package has not been built/compiled to cjs so it doesn't work. Its main point to this file: https://github.com/facebook/react/blob/3b2302253f13c9cd049fac10784e10b6c582e3b6/packages/react-is/index.js

I'd like to tell yarn to ignore the react-is from the workspace and install it into pretty-format (at least, I think that's what I want 😀)

arcanis · 2019-05-30T13:02:32Z

Oh I see, so you want to ignore the local workspace .. hm .. this should be doable with resolutions. Something like this, maybe?

{
  "resolutions": {
    "pretty-format/react-is": "npm:react-is@<insert the semver range>"
  }
}

SimenB · 2019-05-30T13:07:42Z

That sorta works - it does indeed install from the npm registry, but it installs it to node_modules/react-is meaning the one in the workspace is not there. That might break other code importing it (via some haste resolution or some such)?

arcanis · 2019-05-30T13:58:33Z

Damn, that's likely a bug in the resolutions implementation (fwiw we have tests on this in the v2 trunk, so I know it works properly there) 😢

I don't see an easy fix then ... maybe a resolutions entry on pretty-format instead, to point to a local copy that would list react-is@npm:^<range>? It starts to be a bit hacky 🤔

It's interesting we never got this problem before, though!

threepointone · 2019-08-03T10:58:33Z

Now's a good time for us to land this. What do you folks think is the best option right to resolve this pretty-format/react-is issue?

SimenB · 2019-08-03T11:23:24Z

I'm not sure :( I didn't try out @arcanis's last suggestion since he said it was hacky. Maybe give that a go? I'm on vacation for a few more days, so if you have time, that'd be great. Otherwise I'll give it a whack some time next week

(Landing #15779 in the meantime would be great, to reduce this diff)

threepointone · 2019-08-03T11:54:16Z

Cool cool, enjoy your vacation! I’ll poke at this myself soon

SimenB · 2019-08-14T12:01:25Z

I've rebased this now that the other PR landed, but it still has the same react-is issue. Not sure how to deal with it, tbh. Could have some fancy postinstall that downloads react-is from the registry and unpacks it into node_modules/pretty-format/node_modules/react-is, but that feels pretty brittle

SimenB · 2019-08-29T13:12:37Z

🎉

SimenB · 2019-08-29T16:05:06Z

Rebased and squashed the extra commits to migrate devtools over. So now it's one commit migrating to v24, and the one commit I'm rather hesitant to have my name next to as a separate one for the hacky react-is workaround. Should make it easier to review at least!

(I often use this repo to test out breaking changes in Jest itself, especially around fake timers, which is why I wanna see it upgraded. 🙂 Makes it easier)

SimenB · 2019-08-29T16:12:52Z

packages/react-devtools-extensions/package.json

@@ -25,7 +25,6 @@
    "archiver": "^3.0.0",
    "babel-core": "^7.0.0-bridge",
    "babel-eslint": "^9.0.0",
-    "babel-jest": "^24.7.1",


these are just remnants after migrating it into this monorepo, I guess? They're unused

SimenB · 2019-08-29T16:13:46Z

packages/react-devtools-extensions/package.json

@@ -25,7 +25,6 @@
    "archiver": "^3.0.0",
    "babel-core": "^7.0.0-bridge",


this is quite possible also redundant now? not sure... but if it was here for jest@23, then it can go 🙂

SimenB · 2019-09-05T07:54:47Z

@threepointone any news from your meeting?

SimenB · 2019-09-28T17:22:16Z

@threepointone ping 🙂

threepointone

Alright, I'm biting the bullet and approving this. We need jest 24 badly, and this hack is probably the way forward for now. Thanks @SimenB for your patience and effort! You're the best.

threepointone · 2019-10-02T12:31:36Z

I've approved, but could you rebase one last time and make sure nothing's broken? I don't expect it to be, but want to be certain before I hit merge :)

SimenB · 2019-10-02T14:37:09Z

Awesome! I'll give it a rebase tonight or tomorrow

SimenB · 2019-10-03T07:15:48Z

rebased, and CI still seems happy 👍

threepointone · 2019-10-03T17:14:26Z

bing bang bosh, it's in!

SimenB · 2019-10-03T17:46:11Z

woo, thanks! 🎉

threepointone · 2019-10-03T17:52:24Z

No, thank you! We can finally start using inline snapshots yay 😃

* chore: upgrade to jest 24 * download react-is from npm manually

This was added during an upgrade to Jest 24 in #15778 By now we're at Jest 29. I think if CI passes we might not need this hack anymore.

Summary: This was added during an upgrade to Jest 24 in facebook/react#15778 By now we're at Jest 29. I think if CI passes we might not need this hack anymore. DiffTrain build for commit facebook/react@c57a0f6. Reviewed By: poteto Differential Revision: D45238250 Pulled By: kassens fbshipit-source-id: 1dfb90dc08a2765a6d499a5754a311ecae002314

This was added during an upgrade to Jest 24 in facebook#15778 By now we're at Jest 29. I think if CI passes we might not need this hack anymore.

facebook-github-bot added the CLA Signed label May 30, 2019