React-hot-loader: Version 4.8.5 introduces a new console error on every file unit tested with jest

[Powerade]

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:

1. node -v: 11.1.0
2. npm -v: 6.9.0

Then, specify:

1. Operating system: Osx
2. Browser and version: Chrome, 74

[theKashey]

Look like some checks, which are good for the browser, might be not be welcomed on the server.
But why hot is called from jest tests?

[Powerade]

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?

[eemt]

I signed on to report the same error. The error is a result of this change:
https://github.com/gaearon/react-hot-loader/commit/4811d57

[theKashey]

v4.8.6 would fix it

[tommarien]

@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 ;)

[tommarien]

@theKashey apparantly we could also use process.env.JEST_WORKER_ID !== undefined to see if running under jest

[nearbycoder]

@theKashey can confirm with @tommarien. Upgraded to v4.8.6 and I am still seeing this error when running jest.

[theKashey]

The better way for now - remove the warning for now.

[theKashey]

removed in 4.8.7

[tommarien]

@theKashey According to me still a problem in 4.8.7 caused by https://github.com/gaearon/react-hot-loader/blob/7e153c4e2123b7f70068290c1ccea2ea35c9a7ab/index.js#L9

[theKashey]

So it always was just a console.error and I've fixed throw new Error in hot, which does a bit different thing.

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.

[theKashey]

Ok. So obviously that test should not be there, but "production" mode of hot(as long as it would be used) should have a "devmode" condition to check hot existence to point on configuration mistakes.

[ztoben]

@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.

[theKashey]

Just wondering how. The problem is an indication that RHL is trying to activate dev mode, which is something you don't want, as long as it would wrap all your _class based components_ with proxies, and do other things, you might not want in tests.

Usually, you shall have only one place with the explisit import of RHL - like App.js. Import hot, and mark _default export_ as hot.
Usually, this component is not tested, as well as you are not testing ReactDom.render (aka index.js). Or, at least, that's the single one file.

In the same time, RHL would be injected to the each and every file by babel or webpack, if enabled, and would, in this case, poison all your tests.

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.
Could I ask a few questions:

• Do you have this message for every file? Ie A LOT? One message is ok. React is asking to install DevTools every time :).
• could you change our index.js(directly in node_modules), to use console.trace (or just place a breakpoint there) - we have to found a __reason__ why it's called.

In short - if not babel, as long as you have it removed - then what?

[eemt]

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.
Trace: React-Hot-Loader: Hot Module Replacement is not enabled at Object.<anonymous> (C:\...\Dev\node_modules\react-hot-loader\index.js:9:11) at Runtime._execModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:888:13) at Runtime._loadModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:577:12) at Runtime.requireModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:433:10) at Runtime.requireModuleOrMock (C:\...\Dev\node_modules\jest-runtime\build\index.js:598:21) at Object.<anonymous> (C:\...\Dev\...\client\process\ProcessRoot.js:4:1) at Runtime._execModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:888:13) at Runtime._loadModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:577:12) at Runtime.requireModule (C:\...\Dev\node_modules\jest-runtime\build\index.js:433:10) at Runtime.requireModuleOrMock (C:\...\Dev\node_modules\jest-runtime\build\index.js:598:21)

[theKashey]

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?

[tommarien]

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 [e-mail], 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.

[theKashey]

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.

[tommarien]

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 [e-mail], 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.

[karolisgrinkevicius-home24]

@tommarien that's correct. At least jest by defaults sets NODE_ENVto test but I'm not certain about other testing frameworks.

[theKashey]

For mocha you can set whatever you want, but usually you __have to__ set something different from development.
I am a bit hesitant to use === 'development' condition, as long as it would be a breaking change, but === 'test' sounds good.

[tommarien]

@theKashey for mocha we usually set the node_env also ;), want me to wip up a pr ?

[theKashey]

Try v4.8.8. Sorry @tommarien - saw you message too late :D

[hpohlmeyer]

v4.8.8 fixes it for me.

[karolisgrinkevicius-home24]

For me either. 👍

[tommarien]

Just verified, 4.8.8 is a fix thanks @theKashey

[dcastil]

4.8.8 fixed it for me as well. 👍

[eemt]

Also good with 4.8.8.
Thanks!

[theKashey]

From the first try!

[asos-tomp]

I have a load of tests whereby I need to explicitly set process.env.NODE_ENV to something _other_ than "test" - for example testing routes that are only added to development builds, etc.

It would be nice to have a workaround for this - for now, I will need to pin to 4.8.4...

[theKashey]

• you might configure NODE_ENV and BABEL_ENV independently.
• you might not use jsdom, thus remove window
• I might add another _env variable_ to control RHL, but still, - it's a question how you are going to use it.
• __I might move error above to the production version of AppContainer__, scoping error to the "real usage". Babel plugin is able to remove it in a real production builds.