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 notifications@github.com, 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 notifications@github.com, 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.