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

Create a custom Jest environment #509

Open
SimenB opened this issue Sep 11, 2022 · 11 comments
Open

Create a custom Jest environment #509

SimenB opened this issue Sep 11, 2022 · 11 comments

Comments

@SimenB
Copy link

SimenB commented Sep 11, 2022

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
Copy link
Member

kelset commented Sep 12, 2022

cc @cortinico

@thymikee
Copy link
Member

Yes please!

@cortinico
Copy link
Member

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
Copy link

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
Copy link
Author

SimenB commented Sep 12, 2022

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
Copy link
Author

SimenB commented Oct 4, 2022

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
Copy link
Member

kelset commented Oct 4, 2022

@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
Copy link
Author

SimenB commented Oct 4, 2022

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
Copy link
Member

kelset commented Oct 4, 2022

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 :)

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Oct 17, 2022
…34971)

Summary:
This PR is the follow up to the conversation started here by SimenB: react-native-community/discussions-and-proposals#509

Basically, we want to move RN to use its own custom environment so that we can tweak it going forward - this PR in fact only sets up the groundwork for that; robhogan mentioned that with this in place, Meta engineers can
> iterate on it (with jest-environment-node as a starting point) against our internal product tests

This is also connected to Rob's work to bring Jest 29 into the codebase #34724 and my "mirror" PR to bring template in main up to the same version (#34972)

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - move Jest config to use a custom react-native Jest env

Pull Request resolved: #34971

Test Plan: Tested that `yarn test` in main works fine after the changes; CI and Meta's internal CI will also serve the purpose of verifying that it works (but there's no reason not to since it's still pretty much just relying on `node`).

Reviewed By: huntie

Differential Revision: D40379760

Pulled By: robhogan

fbshipit-source-id: 2c6d0bc86d337fda9befce0799bda2f56cc4466c
@SimenB
Copy link
Author

SimenB commented Oct 18, 2022

First iteration landed in facebook/react-native#34971 👍 Not sure if this should be kept open while you work on it?

@kelset
Copy link
Member

kelset commented Oct 18, 2022

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 :)

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Nov 10, 2022
Summary:
Like discussed in react-native-community/discussions-and-proposals#509, RN should override the default `node` and `node-addons` conditions.

You might consider supporting (or just setting) [`customExportConditions`](https://github.com/facebook/jest/blob/4670d3be0d80d47844673eb163666253e788f006/packages/jest-environment-node/src/index.ts#L187-L189) instead, but the default of the node env should be overwritten 🙂

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - use `'react-native'` export conditions in Jest environment

Pull Request resolved: #35203

Test Plan: Green CI?

Reviewed By: lunaleaps

Differential Revision: D41081783

Pulled By: jacdebug

fbshipit-source-id: 844c70d92a58c5432ba5b9e5e99c8f50045ef8ac
kelset pushed a commit to facebook/react-native that referenced this issue Nov 30, 2022
Summary:
Like discussed in react-native-community/discussions-and-proposals#509, RN should override the default `node` and `node-addons` conditions.

You might consider supporting (or just setting) [`customExportConditions`](https://github.com/facebook/jest/blob/4670d3be0d80d47844673eb163666253e788f006/packages/jest-environment-node/src/index.ts#L187-L189) instead, but the default of the node env should be overwritten 🙂

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - use `'react-native'` export conditions in Jest environment

Pull Request resolved: #35203

Test Plan: Green CI?

Reviewed By: lunaleaps

Differential Revision: D41081783

Pulled By: jacdebug

fbshipit-source-id: 844c70d92a58c5432ba5b9e5e99c8f50045ef8ac
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
…acebook#34971)

Summary:
This PR is the follow up to the conversation started here by SimenB: react-native-community/discussions-and-proposals#509

Basically, we want to move RN to use its own custom environment so that we can tweak it going forward - this PR in fact only sets up the groundwork for that; robhogan mentioned that with this in place, Meta engineers can
> iterate on it (with jest-environment-node as a starting point) against our internal product tests

This is also connected to Rob's work to bring Jest 29 into the codebase facebook#34724 and my "mirror" PR to bring template in main up to the same version (facebook#34972)

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - move Jest config to use a custom react-native Jest env

Pull Request resolved: facebook#34971

Test Plan: Tested that `yarn test` in main works fine after the changes; CI and Meta's internal CI will also serve the purpose of verifying that it works (but there's no reason not to since it's still pretty much just relying on `node`).

Reviewed By: huntie

Differential Revision: D40379760

Pulled By: robhogan

fbshipit-source-id: 2c6d0bc86d337fda9befce0799bda2f56cc4466c
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
Summary:
Like discussed in react-native-community/discussions-and-proposals#509, RN should override the default `node` and `node-addons` conditions.

You might consider supporting (or just setting) [`customExportConditions`](https://github.com/facebook/jest/blob/4670d3be0d80d47844673eb163666253e788f006/packages/jest-environment-node/src/index.ts#L187-L189) instead, but the default of the node env should be overwritten 🙂

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[General] [Changed] - use `'react-native'` export conditions in Jest environment

Pull Request resolved: facebook#35203

Test Plan: Green CI?

Reviewed By: lunaleaps

Differential Revision: D41081783

Pulled By: jacdebug

fbshipit-source-id: 844c70d92a58c5432ba5b9e5e99c8f50045ef8ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants