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

global.Promise replacement in Jest setup causing problems. #29303

Closed
chrisbobbe opened this issue Jul 8, 2020 · 2 comments
Closed

global.Promise replacement in Jest setup causing problems. #29303

chrisbobbe opened this issue Jul 8, 2020 · 2 comments

Comments

@chrisbobbe
Copy link

chrisbobbe commented Jul 8, 2020

Hi! 👋

What is the purpose of replacing the global Promise in this line of jest/setup.js:

global.Promise = jest.requireActual('promise');

It looks like it was first introduced in 3ff3987 (quite a while ago) and has remained in a similar form since then. There's no explanation of this choice specifically (if I understand correctly), so I think if it must remain, it would be good to put a code comment there. 🙂

Description

'promise' refers to the NPM package with that name.

It interferes with Jest when using their new "modern" implementation of fake timers (documented here), as I describe at jestjs/jest#10221.

One interesting note on that issue, from a Jest maintainer, is this:

As an aside, one should never replace global.Promise [...]. E.g. when using async-await you will always get the native Promise regardless of the value of global.Promise.

Might it be OK and appropriate to remove the line?

React Native version:

System:
    OS: macOS 10.15.5
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 4.84 GB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 10.20.1 - ~/.nvm/versions/node/v10.20.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - ~/.nvm/versions/node/v10.20.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.5, DriverKit 19.0, macOS 10.15, tvOS 13.4, watchOS 6.2
    Android SDK:
      API Levels: 23, 25, 26, 27, 28, 29
      Build Tools: 27.0.3, 28.0.3, 29.0.2
      System Images: android-28 | Google Play Intel x86 Atom
  IDEs:
    Android Studio: 3.6 AI-192.7142.36.36.6241897
    Xcode: 11.5/11E608c - /usr/bin/xcodebuild
  npmPackages:
    react: 16.8.6 => 16.8.6 
    react-native: 0.60.6 => 0.60.6

Steps To Reproduce

Reproduction of the issue (and more discussion) at jestjs/jest#10221.

Expected Results

React Native's Jest setup doesn't interfere with ordinary, well-documented ways of testing with Jest.

@chrisbobbe
Copy link
Author

Copy-pasting the error output I mention at jestjs/jest#10221 to help others find this issue:

  ✕ 1 equals 1 (5006 ms)

  ● 1 equals 1

    : Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout.Error:

      3 | jest.useFakeTimers('modern');
      4 | 
    > 5 | test('1 equals 1', async () => {
        | ^
      6 |   await Promise.resolve();
      7 |   expect(1).toEqual(1);
      8 | });

      at new Spec (node_modules/jest-jasmine2/build/jasmine/Spec.js:116:22)
      at Object.<anonymous> (fetchData.test.js:5:1)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        5.762 s, estimated 6 s
Ran all test suites.
error Command failed with exit code 1.

chrisbobbe added a commit to chrisbobbe/react-native that referenced this issue Aug 24, 2020
It looks like this line was introduced in 3ff3987, in 2015, and it
has remained in a similar form since then. I haven't found any
explanation for it.

At jestjs/jest#10221 [1], a core Jest maintainer says,

"""
As an aside, one should never replace `global.Promise` [...]. E.g.
when using `async-await` you will always get the native `Promise`
regardless of the value of `global.Promise`.
"""

jestjs/jest#10221 is one issue this line has raised, for anyone
using the latest features of Jest to test async code in their React
Native projects.

[1] jestjs/jest#10221 (comment)

Fixes: facebook#29303
chrisbobbe added a commit to chrisbobbe/react-native that referenced this issue Aug 25, 2020
It looks like this line was introduced in 3ff3987, in 2015, and it
has remained in a similar form since then. I haven't found any
explanation for it.

At jestjs/jest#10221 [1], a core Jest maintainer says,

"""
As an aside, one should never replace `global.Promise` [...]. E.g.
when using `async-await` you will always get the native `Promise`
regardless of the value of `global.Promise`.
"""

jestjs/jest#10221 is one issue this line has raised, for anyone
using the latest features of Jest to test async code in their React
Native projects.

[1] jestjs/jest#10221 (comment)

Fixes: facebook#29303
chrisbobbe added a commit to chrisbobbe/react-native that referenced this issue Aug 25, 2020
It looks like this line was introduced in 3ff3987, in 2015, and it
has remained in a similar form since then. I haven't found any
explanation for it.

At jestjs/jest#10221 [1], a core Jest maintainer says,

"""
As an aside, one should never replace `global.Promise` [...]. E.g.
when using `async-await` you will always get the native `Promise`
regardless of the value of `global.Promise`.
"""

jestjs/jest#10221 is one issue this line has raised, for anyone
using the latest features of Jest to test async code in their React
Native projects.

[1] jestjs/jest#10221 (comment)

Fixes: facebook#29303
chrisbobbe added a commit to chrisbobbe/react-native that referenced this issue Aug 25, 2020
It looks like this line was introduced in 3ff3987, in 2015, and it
has remained in a similar form since then. I haven't found any
explanation for it.

At jestjs/jest#10221 [1], a core Jest maintainer says,

"""
As an aside, one should never replace `global.Promise` [...]. E.g.
when using `async-await` you will always get the native `Promise`
regardless of the value of `global.Promise`.
"""

jestjs/jest#10221 is one issue this line has raised, for anyone
using the latest features of Jest to test async code in their React
Native projects.

[1] jestjs/jest#10221 (comment)

Fixes: facebook#29303
chrisbobbe added a commit to chrisbobbe/react-native that referenced this issue Aug 25, 2020
It looks like this line was introduced in 3ff3987, in 2015, and it
has remained in a similar form since then. I haven't found any
explanation for it.

At jestjs/jest#10221 [1], a core Jest maintainer says,

"""
As an aside, one should never replace `global.Promise` [...]. E.g.
when using `async-await` you will always get the native `Promise`
regardless of the value of `global.Promise`.
"""

jestjs/jest#10221 is one issue this line has raised, for anyone
using the latest features of Jest to test async code in their React
Native projects.

[1] jestjs/jest#10221 (comment)

Fixes: facebook#29303
facebook-github-bot pushed a commit that referenced this issue Sep 12, 2022
Summary:
Pull Request resolved: #34659

We've used this Promise polyfill in Jest setup since at least 2015 ([`3ff3987`](3ff3987)), when native Promise implementations were either non-existent or new and unstable. We no longer need it.

It causes issues with "modern" timers in Jest, as documented in:
 - #29303
 - jestjs/jest#10221

It can also obscure real issues due to its default silent handling of uncaught rejections, eg: D39418412.

Changelog:
[General][Changed] - Don't polyfill Promise in Jest setup

Reviewed By: huntie

Differential Revision: D39417597

fbshipit-source-id: d12433ed66c06a402632c2e1d525aad112ef9b0c
@robhogan
Copy link
Contributor

Fixed in #34659

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
Summary:
Pull Request resolved: facebook#34659

We've used this Promise polyfill in Jest setup since at least 2015 ([`3ff3987`](facebook@3ff3987)), when native Promise implementations were either non-existent or new and unstable. We no longer need it.

It causes issues with "modern" timers in Jest, as documented in:
 - facebook#29303
 - jestjs/jest#10221

It can also obscure real issues due to its default silent handling of uncaught rejections, eg: D39418412.

Changelog:
[General][Changed] - Don't polyfill Promise in Jest setup

Reviewed By: huntie

Differential Revision: D39417597

fbshipit-source-id: d12433ed66c06a402632c2e1d525aad112ef9b0c
tungdo194 pushed a commit to tungdo194/rn-test that referenced this issue Apr 28, 2024
Summary:
We've used this Promise polyfill in Jest setup since at least 2015 ([`3ff3987`](facebook/react-native@3ff3987)), when native Promise implementations were either non-existent or new and unstable. We no longer need it.

It causes issues with "modern" timers in Jest, as documented in:
 - facebook/react-native#29303
 - jestjs/jest#10221

It can also obscure real issues due to its default silent handling of uncaught rejections, eg: D39418412.

Changelog:
[General][Changed] - Don't polyfill Promise in Jest setup

Differential Revision: D39417597

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

Successfully merging a pull request may close this issue.

2 participants