react-native-community / discussions-and-proposals

Discussions and proposals related to the main React Native project
https://reactnative.dev
1.67k stars 126 forks source link

Create a custom Jest environment #509

Open SimenB opened 2 years ago

SimenB commented 2 years ago

Introduction

The React Native Jest preset currently sets up jest-environment-node as a test environment, see https://github.com/facebook/react-native/blob/4e70376dc722b6beb7b5b6d6c042b748a9ed14fd/jest-preset.js#L27. This environment does not properly emulate an RN environment, and leads to unnecessary incompatibilities between tests and actual production code - meaning less reliable tests.

Details

I would like to see React Native ship their own environment instead of relying on one of the two shipped by Jest.

By not doing do, a bunch of Node-specific APIs are available in tests which are not available at runtime (e.g. Buffer, MessageChannel, a native fetch instead of your polyfill etc.).

Another issue is the exports support introduced in Jest 28 - this means the Node environment will resolve the node and node-addon condition instead of e.g. falling back to the main field, leading to different modules being loaded in test and in "real" code. And since Metro will be supporting it, this means the test env won't resolve modules correctly once modules start using exports for RN modules.

The API is defined here: https://github.com/facebook/jest/blob/74d27a746d6279141871f761efd1d7859e49e2dc/packages/jest-environment/src/index.ts#L40-L51

If this proposal is accepted, I'm happy to help review (or write) code needed for this to happen.

Discussion points

React Native defaults to a test environment which behaves more different than needed from a "real" environment. It would be better if RN shipped its own environment to more accurately match the environment of production code.

kelset commented 2 years ago

cc @cortinico

thymikee commented 2 years ago

Yes please!

cortinico commented 2 years ago

React Native defaults to a test environment which behaves more different than needed from a "real" environment. It would be better if RN shipped its own environment to more accurately match the environment of production code.

Totally agree πŸ‘

If you're willing to send over a Pull Request, we can keep on discuss over there. This can be also included as a new package and published from within the react-native repo (see the #480 effort).

robhogan commented 2 years ago

This sounds eminently sensible - I've been deep into RN's Jest use just recently on a mission to update from Jest 26, and several of the sticking points have been due to the way we Frankenstein an RN-ish environment from jest-environment-node via a setup file, which is then subject to getting clobbered by reasonable updates to Jest. A first-class RN environment is a much cleaner and more sustainable solution.

IMO, it'd be easiest for someone at Meta to take this on and iterate on it (with jest-environment-node as a starting point) against our internal product tests, and it makes sense to bring our Jest version up to date first (expected this week or next). I'd be happy to give it a go once that's done.

SimenB commented 2 years ago

IMO, it'd be easiest for someone at Meta to take this on and iterate on it (with jest-environment-node as a starting point)

This sounds reasonable - at least to get something that's "production ready" rather than just a prototype.

Feel free to ping me (somewhere) if you want some feedback or bounce ideas πŸ‘


Also happy to hear you'll be updation the Jest version! If there are any sticking points, feel free to ping me on them as well and I'd be happy to make changes to ease migration (to a point πŸ˜…)


And lastly - publishing as a separate module (or at least as a separately exported thing) probably makes sense so people more easily can extend it.

SimenB commented 1 year ago

Note that as a start (when updating to Jest 28 or 29 in the template) - you should at least ship your own environment which extends the base node env and supplies more correct exportConditions even if it's the same otherwise.

Sorta like this: https://github.com/facebook/jest/blob/86a810db0924cc220ce3f5138a3b85b13a62ae73/e2e/resolve-conditions/deno-env.js

kelset commented 1 year ago

@SimenB we are considering following up the work that @robhogan has been doing by doing mirror upgrades into the template, de-facto bumping Jest from 26 to 29 (matching the version used internally) for RN0.71 (currently, main branch) - I'm happy to pair with you in trying to get the template setup correct, how could we do this?

SimenB commented 1 year ago

Exciting to hear! πŸŽ‰

I'm heading on vacation for a week tomorrow, but happy to do a zoom call (or whatever you wanna use) when I'm back? Back home (CET) midday Tuesday (pretty much exactly one week from now), but probably jet lagged after a week in PST so Wednesday or later next week should work for me.

Also happy to move this discussion to discord or some other better suited format than a GH issue πŸ™‚

kelset commented 1 year ago

ok, enjoy your trip! I'll write a note to self to check in back to you next Wednesday - we can move over DMs or alternatives to discuss details once you are back :)

SimenB commented 1 year ago

First iteration landed in https://github.com/facebook/react-native/pull/34971 πŸ‘ Not sure if this should be kept open while you work on it?

kelset commented 1 year ago

I reckon we can close this since the basework is merged, and there's another RFC that start digging into how to use it more properly.

But I'll leave it to @robhogan to decide :)