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

Difference between Node 14 and Node 16 in the basic example #1054

Closed
jensdev opened this issue Aug 10, 2022 · 8 comments
Closed

Difference between Node 14 and Node 16 in the basic example #1054

jensdev opened this issue Aug 10, 2022 · 8 comments

Comments

@jensdev
Copy link

jensdev commented Aug 10, 2022

Thank you for this amazing package

Describe the bug

When running when Node v14.17.5, I get the expected result.

➜  basic git:(main) node --version
v14.18.1
➜  basic git:(main) npx jest
 PASS  __tests__/App.test.tsx
  ✓ renders correctly (185 ms)
  ✓ User can sign in successully with correct credentials (281 ms)
  ✓ User will see errors for incorrect credentials (299 ms)
  ✓ User can sign in after incorrect attempt (567 ms)

Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        1.763 s, estimated 6 s
Ran all test suites.

But when using Node v16 or higher

➜  basic git:(main) node --version
v16.14.2
➜  basic git:(main) npx jest      
 PASS  __tests__/App.test.tsx
  ✓ renders correctly (189 ms)
  ✓ User can sign in successully with correct credentials (279 ms)
  ✓ User will see errors for incorrect credentials (300 ms)
  ✓ User can sign in after incorrect attempt (569 ms)

Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        1.755 s, estimated 2 s
Ran all test suites.
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

When I run it with detectOpenHandles.

➜  basic git:(main) npx jest --detectOpenHandles
 PASS  __tests__/App.test.tsx (7.786 s)
  ✓ renders correctly (4571 ms)
  ✓ User can sign in successully with correct credentials (293 ms)
  ✓ User will see errors for incorrect credentials (299 ms)
  ✓ User can sign in after incorrect attempt (577 ms)

Test Suites: 1 passed, 1 total
Tests:       4 passed, 4 total
Snapshots:   0 total
Time:        7.828 s
Ran all test suites.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  MESSAGEPORT

      1 | import * as React from 'react';
    > 2 | import { render, screen, fireEvent } from '@testing-library/react-native';
        | ^
      3 | import App from '../App';
      4 |
      5 | /**

      at node_modules/scheduler/cjs/scheduler.development.js:178:17
      at Object.<anonymous> (node_modules/scheduler/cjs/scheduler.development.js:645:5)
      at Object.<anonymous> (node_modules/scheduler/index.js:6:20)
      at node_modules/react-test-renderer/cjs/react-test-renderer.development.js:19:19
      at Object.<anonymous> (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:17382:5)
      at Object.<anonymous> (node_modules/react-test-renderer/index.js:6:20)
      at Object.<anonymous> (node_modules/@testing-library/react-native/src/act.ts:1:1)
      at Object.<anonymous> (node_modules/@testing-library/react-native/src/pure.ts:1:1)
      at Object.<anonymous> (node_modules/@testing-library/react-native/src/index.ts:1:1)
      at Object.<anonymous> (__tests__/App.test.tsx:2:1)

Am I missing something or should this just work?

Expected behavior

Same output for Node v16 and higher

Steps to Reproduce

Use Node v16 or higher

Screenshots

Versions

➜  basic git:(main) npx envinfo --npmPackages react,react-native,react-test-renderer,@testing-library/react-native

  npmPackages:
    @testing-library/react-native: ^11.0.0 => 11.0.0 
    react: 17.0.2 => 17.0.2 
    react-native: 0.68.2 => 0.68.2 
    react-test-renderer: ^17.0.2 => 17.0.2 
@mdjastrzebski
Copy link
Member

mdjastrzebski commented Aug 10, 2022

We've recently noted the same issue with Node 16 & 18. It seems to be similar to the issue #1007 reported by other users, for which we didn't have repro earlier.

We'll be looking to find root cause for this and provide a fix.

@jensdev do you have some spare time and interest to help us investigate this issue?

@jensdev
Copy link
Author

jensdev commented Aug 10, 2022

It seems to be caused by the scheduler package.
https://github.com/facebook/react/blob/main/packages/scheduler/src/forks/Scheduler.js#L560
Which points to this issue: facebook/react#20756
Which points to a fix here: facebook/react#20756 (comment) but React 17.1.0 never came I guess.

The following also seems to do the trick

delete global.MessageChannel;

or using the jsdom test environment also works ( https://github.com/facebook/jest/pull/12581/files ) but needs to be installed separately for jest 28. https://jestjs.io/docs/upgrading-to-jest28#jsdom

pachun added a commit to pachun/mix-me that referenced this issue Aug 11, 2022
RNTL = React Native Testing Library
https://callstack.github.io/react-native-testing-library/

We explicitly require Node 14 because of the issue described here:
callstack/react-native-testing-library#1054
@mdjastrzebski
Copy link
Member

@jensdev thanks for investigating the topic! Great job! 💯

I've checked that this behaviour seems to be fixed in React 18 (Expo 46).

If anyone needs to stay on React 17 then Dan Abramov created a small fix package that does the delete global.MessageChannel with some React + env version checks:

npm install react-16-node-hanging-test-fix 

Ref: facebook/react#20756 (comment)

@tomwanzek
Copy link

@mdjastrzebski Thanks for the cross-reference above. I was wondering why I had open handles pointing at import { cleanup } from "@testing-library/react-native"; after jumping up to Jest 29.

While we still use React 17.0.2, the above library worked like a charm...once I patched its version condition

diff --git a/node_modules/react-16-node-hanging-test-fix/index.js b/node_modules/react-16-node-hanging-test-fix/index.js
index 4a3b342..f15b540 100644
--- a/node_modules/react-16-node-hanging-test-fix/index.js
+++ b/node_modules/react-16-node-hanging-test-fix/index.js
@@ -16,7 +16,9 @@ if (Object.prototype.toString.call(process) !== '[object process]') {
   );
 }
 
-if (semverGt(version, '17.0.1')) {
+// NOTE: Patching the application boundary to 17.0.2, as
+// the actual fix will only become available in React 18
+if (semverGt(version, '17.0.2')) {
   console.error(
     'The `react-16-node-hanging-test-fix` package is no longer needed ' +
     'with React ' + version + ' and may cause issues. Remove this import.'

@tomwanzek
Copy link

So, we just upgraded a project using the react-16-node-hanging-test-fix patch approach from:

  • React Native 0.68.6,
  • React 17.x,
  • Jest 29.4.2
  • RN Testing Lib 11.5.2
  • Node 16.13.2

to

  • React Native 0.71.7
  • React 18.2.0
  • Jest 29.5.0
  • RN Testing Lib 12.1.2
  • Node 18.16.0

Based on the initial sem ver scope and discussions, we thought we could remove react-16-node-hanging-test-fix without re-introducing the hanging test issue. But somehow it appeared, we were re-encountering them. Reintroducing the patch with a tweaked sem ver condition to allow to become effective with React 18.2.0 we seem to be OK.

So 🤔

@mdjastrzebski
Copy link
Member

@tomwanzek hmmm, I haven't observed that before. Would you be able to submit a minimal reproduction repo for this issue? Perhaps if we are able to reproduce it locally we could advise something. If so, pls use examples/basic as a base for such repo.

@alfergus0n
Copy link

So, we just upgraded a project using the react-16-node-hanging-test-fix patch approach from:

* React Native 0.68.6,

* React 17.x,

* Jest 29.4.2

* RN Testing Lib 11.5.2

* Node 16.13.2

to

* React Native 0.71.7

* React 18.2.0

* Jest 29.5.0

* RN Testing Lib 12.1.2

* Node 18.16.0

Based on the initial sem ver scope and discussions, we thought we could remove react-16-node-hanging-test-fix without re-introducing the hanging test issue. But somehow it appeared, we were re-encountering them. Reintroducing the patch with a tweaked sem ver condition to allow to become effective with React 18.2.0 we seem to be OK.

So 🤔

my team are on the same boat, except we are still using enzyme (planning on rt test lib at some point). But basically, getting this open handle error message port error. I did what you just recommended and it fixes the issue. Which is ironic, because it throws a console error:

console.error
    The `react-16-node-hanging-test-fix` package is no longer needed with React 18.2.0 and may cause issues. Remove this import.

My testing is working, so i'm keeping it for the time being, thanks. the issue is obviously persisting regardless of what has been claimed

@mdjastrzebski
Copy link
Member

@tomwanzek @alfergus0n BTW the whole "patch" of that package is just:

const version = require('react').version;
const semverGt = require('semver/functions/gt');

if (Object.prototype.toString.call(process) !== '[object process]') {
  throw Error(
    'The `react-16-node-hanging-test-fix` package must only be used in a Node environment. ' +
    'Remove this import from your application code.'
  );
}

if (semverGt(version, '17.0.1')) {
  console.error(
    'The `react-16-node-hanging-test-fix` package is no longer needed ' +
    'with React ' + version + ' and may cause issues. Remove this import.'
  )
}

// This is the "fix".
delete global.MessageChannel;

https://www.npmjs.com/package/react-16-node-hanging-test-fix?activeTab=code

So you can just add delete global.MessageChannel with the same effect.

Otherwise, I'm not sure why is it happening. We would need some minimal repo to investigate it.

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

4 participants