Initial Stack-Free SSR Implementation - #9673
Merged
Merged
Conversation
sebmarkbage
reviewed
May 12, 2017
| var ReactDOMInjection = require('ReactDOMInjection'); | ||
| var ReactDOMStackInjection = require('ReactDOMStackInjection'); | ||
| var ReactServerRendering = require('ReactServerRendering'); | ||
| var ReactDOMServerRendering = require('ReactDOMServerRendering'); |
Contributor
There was a problem hiding this comment.
Let's put this into another file so that we can get the roll out. ReactDOMStreamServer?
| 'src/addons/**/*.js', | ||
| ], | ||
| }, | ||
| // TODO: there is no Fiber version of ReactDOMServer. |
Contributor
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.
gaearon
reviewed
May 12, 2017
|
|
||
| function resolve(child, context) { | ||
| if (Array.isArray(child)) { | ||
| throw new Error('well that was unexpected'); |
Contributor
There was a problem hiding this comment.
Replace with an invariant?
Contributor
Author
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`?', |
Contributor
Author
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.
Contributor
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.
sebmarkbage
suggested changes
May 15, 2017
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.
sebmarkbage
reviewed
May 17, 2017
| manglePropertiesOnProd: false, | ||
| name: 'react-dom/server', | ||
| paths: [ | ||
| 'src/isomorphic/**/*.js', |
Contributor
There was a problem hiding this comment.
Oops. These changes are actually pretty bad. We're building all of isomorphic into these packages now.
koba04
reviewed
Nov 26, 2017
| props = Object.assign( | ||
| { | ||
| selected: undefined, | ||
| children: undefined, |
Contributor
There was a problem hiding this comment.
Is there any (perf) benefit to pass same shape objects as arguments of Object.assign?
Contributor
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.
mrizwanashiq
pushed a commit
to mrizwanashiq/react
that referenced
this pull request
Jun 25, 2026
* Server Rendering initial commit 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. * Fix build system, add shortcut scripts * Make more ReactServerRendering-test unit tests pass * Make ReactServerRendering-test pass with copious help from Ben 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. * remove some unnecessary cruft * Run prettier on ReactDOMServerRendering.js * Fix more unit tests with Ben * Add support for input and textarea by copy pasting a bunch of code from ReactDOMInput/ReactDOMTextarea * Fix context unit tests with more copy paste :) * progress on select * Holy shit, 100% of ReactDOMServerIntegration tests are passing * Checkpoint to fix some of the ReactDOMComponent-test tests * Fix missing checkPropTypes * Fix some unit tests in ReactDOMComponent-test * Run prettier on everything, thanks Ben * get rid of ssr tests that are looking for component file names * add assertValidProps check * rename flattenChildren to flattenOptionChildren per Ben * Fix all the lint crap * Move things around in the file and turn ReactDOMServerRenderer into a class * remove changes I added to package.json * remove separate createOpenTagMarkup file for now since everything should be copy pasted into a single file * re-record Fiber tests * Revert ReactDOMServer.js and the fiber tests per Sebastian This also reverts the changes I made to ReactDOMComponent-test.js which removed the stack which is missing in the new server renderer" * Rename files based on feedback 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. * Change throw to invariant, even though we probably need to remove this at some point * fix prettier.. sigh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.