React: React Dom Server: Broken Build for version 15.4.1

[hitmands]

Do you want to request a feature or report a bug?
BUG

What is the current behavior?
It breaks the code execution

file: react-dom/dist/react-dom-server.min.js

117:[function(t,n,r){"use strict";var o=e.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;n.exports=o.ReactCurrentOwner},{}]

Beautified:

function(t, n, r) {
  "use strict";
  var o = e.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;

  n.exports = o.ReactCurrentOwner
}

Output:

TypeError: Cannot read property 'ReactCurrentOwner' of undefined
    at Object.r.117

What is the expected behavior?
var ReactDomServer = require('react-dom/dist/react-dom-server.min.js'); with the desired goal of having a server-side rendering for production which skips unnecessary comments, warnings, etc. React Documentation Page

• Which versions of React: 15.4.1;
• platform: NodeJS (server side rendering)
• Did this work in previous versions of React? I don't know

[gaearon]

You are likely using different versions of React and ReactDOMServer. Please make sure you use the exact same version. I also recommend updating both to 15.4.2.

[gaearon]

Also, I don't think importing the UMD bundle is supported in Node environment unless you are also doing the same for React. You can't use ReactDOMServer single-file distribution together with React CommonJS entry point.

[hitmands]

Hi Dan,
thanks for the reply,
I'm using the version 15.4.1 for both React and ReactDom (Reply 1), I'm going to update them to the 15.4.2 version but I hope that there are not incompatibilities between two patch releases (unless documentation).

For the 2 Reply I will have a check and let you know as soon as possible.
Thanks again

[gaearon]

This is a patch release so no, it shouldn't be a problem. If anything, 15.4.2 fixes some issues.

But I am pretty certain that if you can't mix UMD with CommonJS packages so that's the likely source of the problem.

[hitmands]

Requiring the UMD Bundle for react is working fine var React = require('react/dist/react.min'); but it is still throwing for ReactDomServer, so the problem doesn't depend on the necessity of using the same approach between React and ReactDomServer...
Plus, I updated react and react-dom to the 15.4.2 version.

In react-dom/dist/react-dom-server.min i can see a line module.exports = e(require("react")); which requires React by itself, so my node_modules don't affect this.

Furthermore, logging through the bundle I can see that:

function(t, n, r) {
  "use strict";
  var o = e.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
  // e is an instanceof React and has no __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED field
  n.exports = o.ReactCurrentOwner
}

[gaearon]

Hmm I think you're right. I don't we ever supported using UMD builds in the Node environment.
Is there a reason you're not using react-dom/server instead?

[hitmands]

ReactDomServer.renderToString renders with debug info such as warnings and something else, I don't want this behaviour for my production environments. As the doc says, we should use the minified build for production...

I'm trying to optimize performances in general.

[gaearon]

Yea, we should make it clearer the advice to use minified builds is mostly for client-side since we don't have a good way to do this on the server side yet.

On the server side, it should be enough to run your app with NODE_ENV set to 'production', e.g. NODE_ENV=production node myserver.js. Does this help?

[gaearon]

I'll close since we don't support it.

For more heavy server side optimization tips check out this PR: https://github.com/taobaofed/server-side-rendering-comparison/pull/5

I hope it helps!

[aweary]

@gaearon it's worth noting that this did work with 15.3.0 and earlier (at least, it didn't throw). I've seen this recommended a few times in SSR performance articles/talks, so it might not be an uncommon pattern.

[aweary]

(see this popular talk on SSR performance in React that recommends it)

[gaearon]

Hmm, I wasn't aware that this used to work.

[jddxf]

Seems this would happen since 15.4.0.Not sure if it's the desired behavior.
See #7164

[gaearon]

Yes, that's the main reason, but we can't revert it (there are good reasons for this change).
So if we fix it, we need to fix forward.

[magv]

Note that not only the server build of react-dom is affected; this fails in the same way with React 15.4.2:

var React = require("react/dist/react.min.js");
var ReactDOM = require("react-dom/dist/react-dom.min.js");

(The use case for such an operation is loading the minified version of React inside Atom).

[gaearon]

@magv

This accidentally happened to work because package boundaries were fake, but now they are real. However, now I’m not sure we could support this even if we wanted to.

These are UMD bundles. However you are loading them in a CommonJS environment. How would you expect them to “find” one another? They don’t know each other’s locations.

Since you’re in CommonJS, they can’t set globals, like they do in the browser. So react-dom/dist/react-dom.min.js can’t guess that React is located at react/dist/react.min.js.

I understand it’s frustrating this doesn’t work anymore, but it was never officially supported (you won’t find this usage suggested anywhere in the docs), and as far as I understand, it’s not even feasible to support after the true package split.

The real solution here is to bundle your code if you rely on this. If you bundle the code (e.g. with webpack, browserify or rollup), you can always specify alias (or equivalent) to tell the bundler to use react/dist/react.min.js instead of react, and react-dom/dist/react-dom.min.js instead of react-dom. Since the bundler resolves paths everywhere, it would also override the path in react-dom.min.js to refer to the correct React. Without the bundler, we can’t do this.

Let me know if I missed something, but so far it seems like we can’t fix it. Sorry! For server rendering performance, we intend to provide flat precompiled bundles in the future which should have the same effect.

[hitmands]

I was using webpack and that's the reason because require('react/dist/react.min.js') was working even using the umd bundle in a commonjs environment.
I think that closing the bug could be acceptable, but at the same time we should focus on making server-side usage more clear, both in docs and in components.

[gaearon]

With Webpack, using alias in configuration for this should work well.

[magv]

@gaearon, in the case of Atom you can, in fact, set globals by using window.

Alternatively you could, if you wanted to, provide an additional set of minified files for react and react-dom that require their correct minified counterparts, and thus work in server environments (or something to this effect -- my understanding of the glue react-dom uses to find react is minimal at the moment).

As to using bundling tools, this is certainly an option, but a rather inconvenient one as far as Atom extension development goes (since it introduces a build stage).

Of course, if you've decided not to support importing from minified files in react and react-dom packages, I won't argue further.

[szimek]

@gaearon Is it really enough to set NODE_ENV=production to ensure that react and react-dom/server don't load and execute unnecessary code in production? Is it mentioned anywhere in the docs?

I'm using Webpack 2.6.1 and I'm trying to use minified versions of react and react-dom/server (both 15.5.4) using the following config:

resolve: {
  alias: {
    react: require.resolve('react/dist/react.min.js'),
    'react-dom/server': require.resolve('react-dom/dist/react-dom-server.min.js'),
  },
},

but I'm getting exactly the same error, i.e. Cannot read property 'ReactCurrentOwner' of undefined.

[luchillo17]

@szimek I'm having the exact same error, except in React-Native and no react-dom, any idea where to look?

[gaearon]

@luchillo17 You seem to be mixing incompatible versions of react and react-native.

[luchillo17]

Oh i didn't think that'd be a thing, is there an official compatibility chart?

[gaearon]

React Native always supports only one version of react package. You can find out which one it is by checking node_modules/react-native/package.json and looking for react peer dependency there. You should already get a warning if you use the wrong one.

[luchillo17]

@gaearon I've looked at that package.json file and the version is 0.44.3, the peer dependency is 16.0.0-alpha.6 and the react version i have installed is 16.0.0-alpha.6.

At this point i'm considering starting a new project and moving my source folder.

[luchillo17]

I've been testing more, this is my output in a component with just React-Native's Text component:

And this is with Native-Base's Text component:

Please don't tell me Native-Base also needs to ensure some compatibility.

[gaearon]

Please provide a project reproducing the problem. People generally use RN without such issues so you need to give more details to help you.

[luchillo17]

Can i tell react-native-cli to use npm instead of yarn?, i think most people don't use yarn right?

[gaearon]

Sorry, you'd need to ask this in react-native repository. I don't know.

[luchillo17]

So i guess providing you with a repo would mean nothing since it's a React-Native app?

[gaearon]

I can take a look at it to see if anything is wrong on the react side of things but I can't answer questions like

Can i tell react-native-cli to use npm instead of yarn?

because react-native-cli is not part of this repo.

[luchillo17]

Also i'm using typescript, maybe that's part of the issue?

[gaearon]

I don't know. As for your report,

I've looked at that package.json file and the version is 0.44.3

This doesn't sound right. You can see ReactGlobalSharedState.js in your stack trace. This file doesn't exist in [email protected] (try searching). So if you see it in the redbox, you are not running 0.44.

That file was added in [email protected]. But 0.45 also bumped minimal required react version which wouldn't crash with it.

So something is wrong with your report.

[luchillo17]

Ok i've updated deps, now dependencies are like this:

And error has changed to:

[gaearon]

Please provide minimal code reproducing the problem. It's likely the problem is exactly what the error message says. In future versions we'll provide more details there.

[luchillo17]

I'll try making it from zero because the current error message says Maximum call stack size exceeded (which makes me think i have an infinite loop somewhere).

The error above shows after activating Debug JS Remotely.

[luchillo17]

@gaearon I've started a new one, still no business logic, only adding typescript, now it shows Maximum call stack size exceeded from start until i activate Debug JS Remotely from menu, this one i can publish to Github, do you want to take a look?

[luchillo17]

@gaearon I've put my base project here:
https://github.com/luchillo17/react-native-error-repo
Can you see if it works for you?

[gaearon]

If it's not related to the issue you originally described, please create a new issue with your project in React Native repo. Thanks!

[luchillo17]

@gaearon I've created one in React Native repo since it might be related to them instead of just React:
https://github.com/facebook/react-native/issues/14614