-
Notifications
You must be signed in to change notification settings - Fork 1.1k
auto cleanup will break down testing in mocha
& esm
environment
#614
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
Comments
You can skip autocleanup by either setting the environment variable |
I think the goal is to make it so you can autocleanup when using esm. Pretty sure this should remain open. |
I think what I want is not remove async cleanup in testing but make sure |
Yeah, I'd like to figure out how to make that I don't have bandwidth to work on it though. @whodoeshua, do you have time to dive in? |
Oh now I see the issue. This only happens on node 12.13 but not prior versions, right? I guess we can switch to a proper import() and see how these are transpiled to commonJS modules. |
I think we need to be careful with this change. It seems like we're using
|
I find a problem that makes me very confused. I use four way to required timer as below and only the way that using currently will failure:
And I'm confuse with the comments in |
@threepointone is the one who wrote this code and knows the most about it. I'm pretty sure he got it from React source (which he may have written as well, I'm not sure). |
The failure case is likely the only one that isn't transpiled by webpack and uses the actual I'm pretty sure this hack was used to prevent rollup from adding a node polyfill but I don't know why/if you couldn't explicitly tell rollup to not do it. Either way @threepointone hopefully has more context. |
I just try different kind of require in So that mean webpack_require is the only way to make sure testing will be success? Then why don't we import a library to insure the testing is running like what we expect for? |
Actually this has nothing to do with webpack and you definitely don't need to be using webpack for this to work. |
It will fail since you are not allowed to mix module systems in I don't know about your use cases but usually you don't need esmodules in |
There is an library that only transpile to esmodule and I don't want to bundler it into my library. I want to just keep import in my release file. But I will bring some issue in my testing, so I need to use |
This seems to be an issue with React itself too: facebook/react#18589 This should be fixed by this one-liner: -enqueueTaskImpl = nodeRequire('timers').setImmediate;
+enqueueTaskImpl = nodeRequire.call(module, 'timers').setImmediate; However, I am not sure if fixing this on React-testing-library alone would help, or the React test-utils also need to be fixed too. |
If that fixes it for us, then let's go ahead and do that (seems pretty harmless to me). |
yeah this looks good. I’d recommend sending a PR to react as well. |
🎉 This issue has been resolved in version 10.0.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
@testing-library/react
version:9.5.0
react
version:^16.12.0
node
version:12.13.0
mocha
version:^6.2.2
,mochawesome
version:^4.1.0
,esm
version:^3.2.25
npm
(oryarn
) version:[email protected]
Relevant code or config:
Github demo as below:
https://github.com/whodoeshua/mocha-react-testing-library
package.json
What you did:
What happened:
Global
afterEach
hook will beak down testing with timeout error, cause ofReproduction:
https://github.com/whodoeshua/mocha-react-testing-library
Problem description:
This problem is cause of auto cleanup. Auto clean up is an async funtion and required
timers
module to make a fake timer. But require function will throw out a error onceesm
module has been required. And the async cleanup will never stop.Auto cleanup will call
flush-microtasks
.require function in
flush-microtasks
will throw out an error.The text was updated successfully, but these errors were encountered: