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
Version 4.8.5 introduces a new console error on every file unit tested with jest #1252
Comments
Look like some checks, which are good for the browser, might be not be welcomed on the server. |
I don't know why hot is being called. I don't have anything configured to use it in jest. And it previously wasn't apparently, since this error wouldn't show. Is it something from the new version? |
I signed on to report the same error. The error is a result of this change: |
v4.8.6 would fix it |
@theKashey 4.8.6 only fixes it for ssr for jest we need to take into account the env which i thinks is test by default in jest or some other mechanic. Please reopen this issue ;) |
@theKashey apparantly we could also use process.env.JEST_WORKER_ID !== undefined to see if running under jest |
@theKashey can confirm with @tommarien. Upgraded to v4.8.6 and I am still seeing this error when running jest. |
The better way for now - remove the warning for now. |
removed in 4.8.7 |
@theKashey According to me still a problem in 4.8.7 caused by Line 9 in 7e153c4
|
So it always was just a Let me think about a proper way to fix this, but for now the best way to address this issue is to remove babel plugin in jest environment. It's adding RHL registration code to the every file, but you don't need it for tests. |
Ok. So obviously that test should not be there, but "production" mode of |
@theKashey, any update on this? This is keeping us from being able to upgrade to 4.8.5-4.8.7 as well, even when I remove the babel plugin. |
Just wondering how. The problem is an indication that RHL is trying to activate Usually, you shall have only one place with the explisit import of RHL - like In the same time, RHL would be injected to the each and every file by Yet again - having this message is an indicator of some misconfiguration, which is the real problem. You don't want RHL to be active in the testing environment, believe me.
In short - if not babel, as long as you have it removed - then what? |
We get the error on "a lot" (13) test suites, but not all. Slightly more specifically, I would expect that if it happens on our "view" tests, it would happen on all of them. It does not. It happens on less than 50% of them. I'll do a bit of poking into the tests to see if I can identify a pattern. In the meantime, here is the result of console.trace for us. |
Look like |
Hi Anton
If we would document it on the readme with how we have to set it up for testing so that the proxyIng does not happen. Then I would have no problems with the solution as is. It is just a pest coming from 4.8.4 to see all that red going on. The setup with when and how Babel plugin. But to be honest I haven’t found any diff between running in dev mode with or without plugin. We should have a way to disable hot loader completely for tests as it is on production
…On 23 May 2019, 01:57 +0200, Anton Korzunov ***@***.***>, wrote:
Look like console.trace is not quite jest friendly :(. So, @eemt, is RHL required legitly for all 13 tests? Is there any way to not require it?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Agree, right now it's |
For jest I think node_env is test while running, could we add it to the disabled state. Seems like a common approach what do you think ?
…On 23 May 2019, 08:15 +0200, Anton Korzunov ***@***.***>, wrote:
Agree, right now it's NODE_ENV === "production", partially BABEL_ENV === "production"(disables only babel plugin), and the absence of window (which may present during the tests).
If that's not enough - we should create a better way(s) to control it, and document it better than what we have today.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@tommarien that's correct. At least jest by defaults sets |
For mocha you can set whatever you want, but usually you have to set something different from |
@theKashey for mocha we usually set the node_env also ;), want me to wip up a pr ? |
Try v4.8.8. Sorry @tommarien - saw you message too late :D |
v4.8.8 fixes it for me. |
For me either. 👍 |
Just verified, 4.8.8 is a fix thanks @theKashey |
4.8.8 fixed it for me as well. 👍 |
Also good with 4.8.8. |
From the first try! |
I have a load of tests whereby I need to explicitly set It would be nice to have a workaround for this - for now, I will need to pin to 4.8.4... |
|
Description
Whereas before, my unit test suite would pass without errors or warnings, simply by upgrading my version from 4.8.4 to 4.8.5, I get the following console error on every file:
Expected behavior
No error as with version 4.8.4, since I haven't touched any files, other thank package.json.
Actual behavior
Environment
React Hot Loader version: 4.8.5
Run these commands in the project folder and fill in their results:
node -v
: 11.1.0npm -v
: 6.9.0Then, specify:
The text was updated successfully, but these errors were encountered: