Skip to content

Conversation

@sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented May 17, 2017

Follow up to #9673

Rename ReactServer -> ReactDOMServerStream This file is going to be the replacement for ReactDOMServer.

I mock ReactDOMServer and user ReactDOMServerStream when we have the fiber flag enabled. I'm now also enabling this as the default for distributions builds (react-dom/server on npm and react-dom-server.production.min.js as umd bundle).

I'm using traverseStackChildren instead of traverseAllChildren because traverseAllChildren is now only in the isomorphic package and we don't want to build all of that that into the server package.

I also have to require lower case react for the builds to work.

I also pulled in two commits from #9580 which fixes ReactDOMServer in Fiber mode, which fixed tests we're no longer testing (the injection) and fixes some assertions in Fiber mode to ensure that we pass a proper amount of tests.

Test Plan: Tested the SSR fixture.

Currently this case is using the stack renderer.
In our tests this normally happens because ReactDOM.js injects them into
the shared module, but when Fiber is enabled or this is its own flat
bundle, that doesn't happen.
ReactServer -> ReactDOMServerStream

This file is going to be the replacement for ReactDOMServer.

I mock ReactDOMServer and user ReactDOMServerStream when we have
the fiber flag enabled. I'm now also enabling this as the default for
distributions builds (react-dom/server on npm and
react-dom-server.production.min.js as umd bundle).

I'm using traverseStackChildren instead of traverseAllChildren because
traverseAllChildren is now only in the isomorphic package and we don't
want to build all of that that into the server package.

I also have to require lower case react for the builds to work.
expectTextNode(e.childNodes[0], '');
expectTextNode(e.childNodes[1], '');
expectTextNode(e.childNodes[2], '');
if (render === serverRender) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure @aickin is thrilled to see this check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor

@aickin aickin May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imagine a single tear streaming down my face... 😢

(but seriously: this PR is super, super, super exciting)

Copy link
Collaborator Author

@sebmarkbage sebmarkbage May 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once my other PR gets approved by reviewers I can start removing some of the differences (like comment tags). :)

@sebmarkbage sebmarkbage merged commit e5e874d into facebook:master May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants