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

Bug: react test utils do not work together with esm module #18589

Closed
just-boris opened this issue Apr 13, 2020 · 2 comments
Closed

Bug: react test utils do not work together with esm module #18589

just-boris opened this issue Apr 13, 2020 · 2 comments

Comments

@just-boris
Copy link
Contributor

just-boris commented Apr 13, 2020

React version: 16.13.1

Steps To Reproduce

This code works fine when using plain Node.js, but fails: when esm module is enabled:

const { act } = require('react-dom/test-utils');

act(async () => {
  console.log('something');
}).then(
  () => console.log('success'),
  () => console.log('error')
);
$ node test.js
something
success
$ node -r esm test.js
something
Warning: This browser does not have a MessageChannel implementation, so enqueuing tasks via await act(async () => ...) will fail. Please file an issue at https://github.com/facebook/react/issues if you encounter this warning.
error

The warning is caused by an exception happened in this try/catch block:

try {
// read require off the module object to get around the bundlers.
// we don't want them to detect a require and bundle a Node polyfill.
const requireString = ('require' + Math.random()).slice(0, 7);
const nodeRequire = module && module[requireString];
// assuming we're in node, let's try to get node's
// version of setImmediate, bypassing fake timers if any.
enqueueTaskImpl = nodeRequire('timers').setImmediate;
} catch (_err) {

This is happening because esm module brings its own implementation of the module object with slightly different behavior which requires this context to be properly preserved.

This one-line change should do the fix:

-enqueueTaskImpl = nodeRequire('timers').setImmediate;
+enqueueTaskImpl = nodeRequire.call(module, 'timers').setImmediate;

This issue has also been reported to React-testing-library, which has similar code: testing-library/react-testing-library#614

Why should this be a fix in React and not in ESM?

React relied on undocumented behavior of Node modules loader. It was never guaranteed that it will work without proper this context.

In fact, it will not work as expected if you try it with any non built-in module

const nodeRequire = module.require;

nodeRequire('timers'); // built-in module, works
require('react') // proper require, also works
nodeRequire('react'); // Error: Cannot find module 'react', even though the module is installed

The current code in React may break in future if timers module will be replaced with anything else, or if Node.js changes their internal implementation.

@aweary
Copy link
Contributor

aweary commented Apr 13, 2020

Do you want to submit a PR @just-boris? It sounds like a reasonable thing for us to fix.

@aweary aweary added Component: Core Utilities and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Apr 13, 2020
@just-boris
Copy link
Contributor Author

@aweary, I could, however I am not sure about testing part. What kind of tests would you expect for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants