aickin / react-dom-stream

A streaming server-side rendering library for React.
2.01k stars 48 forks source link

React warning - Replacing React-rendered children with a new root component #4

Closed geekyme closed 9 years ago

geekyme commented 9 years ago

Got the following warning when I started using react-dom-stream.

warning.js?0260:45 Warning: render(...): Replacing React-rendered children with a new root component. If you intended to update the children of this node, you should instead have the existing children update their state and render the new components instead of calling ReactDOM.render.

How i'm using it:

...
ReactDOMStream.renderToString(
      <HtmlDocument
        lang={ req.locale }
        state={ state }
        ApplicationComponent={ Application }
        webpackStats={ webpackStats }
        currentUser={ currentUser }
        title={ "test" }
        req={ req }
      />,
      res
    ).then((hash) => {
      res.write(`<script>window.__REACT_DOM_STREAM_HASH__=${hash}</script>`);
      res.end();
    });

...

This is really huge HTML page (24.1KB). However, I do not see it the individual html fragments of being streamed down when I see view source.

Am I missing something?

geekyme commented 9 years ago

More information:

ApplicationComponent is being rendered into HtmlDocument this way:

...
<div id="root">
            <ApplicationComponent context={ context } host={ req.hostname } />
          </div>
aickin commented 9 years ago

It's certainly my fault, but my my. That's... deeply strange. I suspect it's similar to issue #3, which I am also having trouble reproducing. A few questions:

What version of node? How are you transforming JSX, and with what library/version? Does HTMLApplication do anything out of the ordinary?

aickin commented 9 years ago

Oh, and I should have said: thanks so much for trying and reporting your error?

aickin commented 9 years ago

Sorry, one more question: does it render correctly with React 0.14?

geekyme commented 9 years ago

Node v4.1.2. I'm using webpack with babel-loader to transform the JSX code. No HtmlComponent is just a component with the full document tags. ie. <html> <head>... <body>... etc.

I've attached my dependencies for you.

"dependencies": {
    "async": "1.4.2",
    "autoprefixer-loader": "2.0.0",
    "axis": "0.4.0",
    "babel": "5.8.23",
    "babel-core": "5.8.25",
    "babel-loader": "5.3.2",
    "bluebird": "2.9.32",
    "body-parser": "1.13.2",
    "bootstrap-styl": "4.0.5",
    "chai": "3.2.0",
    "classnames": "2.1.2",
    "compression": "1.5.1",
    "concat-stream": "1.5.0",
    "cookie-parser": "1.3.5",
    "cookies-js": "1.2.1",
    "cors": "2.7.1",
    "cropperjs": "0.1.1",
    "css-loader": "0.14.4",
    "csurf": "1.8.3",
    "debug": "2.2.0",
    "expose-loader": "0.7.0",
    "express": "4.12.4",
    "extract-text-webpack-plugin": "0.8.1",
    "fastclick": "1.0.6",
    "favicon": "0.0.2",
    "file-loader": "0.8.4",
    "fluxible": "1.0.0",
    "fluxible-action-utils": "0.2.4",
    "fluxible-addons-react": "0.2.0",
    "fluxible-plugin-fetchr": "0.3.4",
    "fluxible-plugin-middleware": "0.1.1",
    "fluxible-router": "0.3.0",
    "font-awesome-stylus": "4.3.0",
    "gulp": "3.9.0",
    "gulp-awspublish": "0.0.23",
    "gulp-cloudfront": "0.0.14",
    "gulp-load-plugins": "0.7.1",
    "helmet": "0.12.0",
    "hpp": "0.2.0",
    "i18n-abide": "0.0.22",
    "immutable": "3.7.3",
    "intl": "0.1.4",
    "intl-messageformat": "1.1.0",
    "ip": "0.3.3",
    "isparta": "3.0.4",
    "istanbul": "0.3.19",
    "jpickle": "0.4.1",
    "jsxgettext": "0.8.0",
    "keymirror": "0.1.1",
    "layzr.js": "1.4.0",
    "locale": "0.0.20",
    "lodash": "3.9.3",
    "mobile-detect": "1.2.0",
    "mocha": "2.3.2",
    "morgan": "1.6.1",
    "newrelic": "1.21.1",
    "node-libs-browser": "0.5.2",
    "planout": "1.0.3",
    "qs": "3.1.0",
    "raven": "0.7.3",
    "react": "0.14.0",
    "react-bootstrap": "0.27.1",
    "react-dom": "0.14.0",
    "react-dom-stream": "0.1.4",
    "react-helmet": "1.1.5",
    "react-intl": "1.2.0",
    "react-pure-render": "1.0.2",
    "react-typeahead": "1.0.8",
    "redis": "1.0.0",
    "require-dir": "0.3.0",
    "serialize-javascript": "1.0.0",
    "serve-favicon": "2.3.0",
    "sinon": "1.17.1",
    "sinon-chai": "2.8.0",
    "store2": "2.3.0",
    "strip-loader": "0.1.0",
    "style-loader": "0.12.3",
    "stylus-loader": "1.2.0",
    "superagent": "1.2.0",
    "swiper": "3.0.8",
    "url-loader": "0.5.6",
    "webpack": "1.9.10"
  },
  "devDependencies": {
    "babel-eslint": "4.1.3",
    "babel-plugin-rewire": "0.1.22",
    "babel-runtime": "5.5.6",
    "eslint": "1.6.0",
    "eslint-config-airbnb": "0.1.0",
    "eslint-plugin-react": "3.5.1",
    "esprima-fb": "15001.1.0-dev-harmony-fb",
    "jasmine-core": "2.3.4",
    "karma": "0.12.37",
    "karma-chrome-launcher": "0.2.0",
    "karma-cli": "0.0.4",
    "karma-commonjs": "0.0.13",
    "karma-firefox-launcher": "0.1.6",
    "karma-jasmine": "0.3.6",
    "karma-jasmine-html-reporter": "0.1.8",
    "karma-safari-launcher": "0.1.1",
    "karma-sourcemap-loader": "0.3.5",
    "karma-webpack": "1.5.1",
    "react-hot-loader": "1.2.8",
    "react-tools": "0.13.3",
    "stylint": "1.2.3",
    "webpack-dev-server": "1.10.1"
  }

and yes I am using React 0.14. Throws the error. Have not tried on 0.13

aickin commented 9 years ago

Thanks for the detail.

And sorry, what I meant about 0.14 was: can you use the built-in renderToString in React 0.14?

geekyme commented 9 years ago

Yup i could use the default renderToString method. Works nicely.

aickin commented 9 years ago

Cool. And I'm sorry I'm asking so many questions, and I thank you for your help, but is this a console error you are getting on the client or on the server?

geekyme commented 9 years ago

i'm getting it on the client side. Side point too - I do not see that it is streaming the markup.

geekyme commented 9 years ago

Anyway HtmlDocument looks something like this, hope it will help you better reproduce

      <html lang={lang} >
        <head>
          <meta charSet="UTF-8" />
          <title>{ title }</title>

          { css.map((href, k) =>
            <link key={k} rel="stylesheet" type="text/css" href={`${href}`} />)
          }

        </head>
        <body>

          <div id="root">
            <ApplicationComponent context={ context } host={ req.hostname } />
          </div>

          <script dangerouslySetInnerHTML={{__html: state}} />

          { script.map((src, k) => <script key={k} src={`${src}`} />) }
        </body>
      </html>
aickin commented 9 years ago

That's a great, great help. Let me try that in my testbed and see if I can repro the error. Cross your fingers!

geekyme commented 9 years ago

crossing now until it hurts!

geekyme commented 9 years ago

@aickin On the client side this is how i'm bootstrapping

const mountNode = document.getElementById("root");
      const Application = FluxibleApp.getComponent();
      const hash = window.__REACT_DOM_STREAM_HASH__;

      ReactDOMStream.render(<Application context={context.getComponentContext()} />, mountNode, hash, () => {
        debug("Application has been mounted");
      });

This is the flux framework i'm using, not that it matters - http://fluxible.io/

aickin commented 9 years ago

Client side looks perfect; I'm sure that the problem is that it's not rendering correctly on the server.

So I copied your HTML page and gave it dummy data... and it worked perfectly. Would you mind posting the HTML rendered both from ReactDOM.renderToString and from ReactDOMStream.renderToString? (I understand if it's sensitive info.)

I noticed you used "context" without using this.context. Was there a line above in the render function that read var context = this.context? Or are you using the new functional components in React 0.14?

geekyme commented 9 years ago

Weird. I can't post the full html though. Are you on gitter or something?

yes there's a line above that i omitted which declares all the variables properly.

aickin commented 9 years ago

I am now! Just sent you an invite to a room.

aickin commented 9 years ago

Discussed this with @geekyme on gitter, and it turns out that his code is based on isomorphic500, which has code that looks something approximately like this:

// render the dynamic code of the page to a string.
var appHtml = React.renderToString(<Application/>);

// now use React as a static templating language to create the <html>, 
// <head>, and <body> tags
var fullPageHtml = React.renderToStaticMarkup(<HtmlPage content={appHtml}/>);

res.write(fullPageHtml);

Because the framework embeds React-rendered markup inside other React-rendered markup, it's not possible to stream out the content. To fix this, we'd need to port the HtmlPage class to just simple res.write calls before and after the call to renderToString. Since that's outside the scope of this project, I'm closing this issue.

Thanks, @geekyme, for helping me track this down!

geekyme commented 9 years ago

I have confirmed that the library will work if anyone is using the same approach as me. If you would like to render your <head> tags and the rest of the dom elements outside of your React components, then you would need to res.write manually the rest of the dom nodes.

tiye commented 9 years ago

Same for me, I have to emit page head by myself, well, unpleasantly. Is that res.write solution mature now?

aickin commented 8 years ago

Late update on this issue: I just released version 0.3.0, which includes support for embedding streams as children in the element tree when using renderToStaticMarkup, so you should be able to port this kind of code more directly.

There's a snippet of example code in the README now showing how to use renderToStaticMarkup to render the page template and embed the output of renderToString.