Initial Stack-Free SSR Implementation #9673
Conversation
src/renderers/dom/ReactDOMServer.js
Outdated
| @@ -13,15 +13,15 @@ | |||
|
|
|||
| var ReactDOMInjection = require('ReactDOMInjection'); | |||
| var ReactDOMStackInjection = require('ReactDOMStackInjection'); | |||
| var ReactServerRendering = require('ReactServerRendering'); | |||
| var ReactDOMServerRendering = require('ReactDOMServerRendering'); | |||
There was a problem hiding this comment.
Let's put this into another file so that we can get the roll out. ReactDOMStreamServer?
| 'src/ReactVersion.js', | ||
| 'src/shared/**/*.js', | ||
| 'src/addons/**/*.js', | ||
| ], | ||
| }, | ||
| // TODO: there is no Fiber version of ReactDOMServer. |
There was a problem hiding this comment.
Let's take this chance to duplicate the entry above to a new flat bundle entry for the new ReactDOMStreamServer module.
|
|
||
| function resolve(child, context) { | ||
| if (Array.isArray(child)) { | ||
| throw new Error('well that was unexpected'); |
There was a problem hiding this comment.
Replace with an invariant?
There was a problem hiding this comment.
Oh, duh.. @spicyj, what happens when child is passed in as an array (in a post-fiber world). Was this before we thought about fragments, since your original diff was from over a year ago? :P
| ); | ||
| expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( | ||
| 'Warning: Unknown event handler property onclick. Did you mean ' + | ||
| '`onClick`?\n in input (at **)', | ||
| 'Warning: Unknown event handler property onclick. Did you mean `onClick`?', |
There was a problem hiding this comment.
cc @spicyj again. We don't have the stacks in the server renderer so Ben told me to just remove these, but I'd be fine with trying to figure out how to add them back.
There was a problem hiding this comment.
Ah okay. I don't really mind since it's easier to debug things on the client anyway.
Originally authored by spicyj, tweakes to rebase it and make it run by tomocchino. Adding ReactDOMServerRendering and a createTagMarkup utility function which will be used in it.
This is pretty hacky and I just inlined everything, but at least I sort of understand how it works now, and all of the tests are passing. There are also only 68 tests failing in the integration test suite.
…om ReactDOMInput/ReactDOMTextarea
…uld be copy pasted into a single file
This also reverts the changes I made to ReactDOMComponent-test.js which removed the stack which is missing in the new server renderer"
Moving src/renderers/dom/server/ReactDOMServerRendering.js to src/renderers/server/ReactServerRenderer.js and add src/renderers/server/ReactServer.js which makes this new codepath completely separate.
| fbEntry: 'src/renderers/dom/ReactDOMServer.js', | ||
| hasteName: 'ReactDOMServerStack', | ||
| isRenderer: true, | ||
| label: 'dom-server', | ||
| manglePropertiesOnProd: false, | ||
| name: 'react-dom/server', | ||
| paths: [ | ||
| 'src/isomorphic/**/*.js', |
There was a problem hiding this comment.
Oops. These changes are actually pretty bad. We're building all of isomorphic into these packages now.
| props = Object.assign( | ||
| { | ||
| selected: undefined, | ||
| children: undefined, |
There was a problem hiding this comment.
Is there any (perf) benefit to pass same shape objects as arguments of Object.assign?
There was a problem hiding this comment.
Likely depends on the engine and how it implements hidden class optimizations. In theory it helps provide a better layout for the fields.
This is an initial implementation of server rendering that doesn't depend on stack. It's basically all copy pasted code, shoved into a single file, but it will hopefully be able to be cleaned up and refactored if we decide to land this.