New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use single entry point for SSR via browser field #10362

Merged
merged 7 commits into from Aug 2, 2017

Conversation

Projects
None yet
3 participants
@gaearon
Member

gaearon commented Aug 2, 2017

Haven’t verified this works yet, will test tomorrow.
But that’s the basic idea.

@gaearon gaearon requested a review from sebmarkbage Aug 2, 2017

@@ -6,7 +6,7 @@
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactDOMServerEntry
* @providesModule ReactDOMServerBrowserEntry
*/

This comment has been minimized.

@gaearon

gaearon Aug 2, 2017

Member

We could add shims that throw for renderToStream() APIs here. Then it's less mysterious.

@gaearon

gaearon Aug 2, 2017

Member

We could add shims that throw for renderToStream() APIs here. Then it's less mysterious.

This comment has been minimized.

@sebmarkbage

sebmarkbage Aug 2, 2017

Member

Yes that would be nice.

@sebmarkbage

sebmarkbage Aug 2, 2017

Member

Yes that would be nice.

"test-utils.js",
"unstable-native-dependencies.js",
"cjs/",
"umd/"
],
"browser": {

This comment has been minimized.

@sebmarkbage

sebmarkbage Aug 2, 2017

Member

It might be nicer to do this with a more public browser entry point instead of the internal package details (personally I'd prefer not exposing those as separate files at all and just inline them).

With a public entry point it's at least possible to manually require the browser version if you need to. This encourages require to the internal implementation details instead.

@sebmarkbage

sebmarkbage Aug 2, 2017

Member

It might be nicer to do this with a more public browser entry point instead of the internal package details (personally I'd prefer not exposing those as separate files at all and just inline them).

With a public entry point it's at least possible to manually require the browser version if you need to. This encourages require to the internal implementation details instead.

@sebmarkbage

It might be nice to keep a public react-dom/server.browser.js that switches between dev/prod of the browser build. That way it's possible to work around the default by requiring this directly instead of deep linking. I'll let you decide tho.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 2, 2017

Member

It might be nice to keep a public react-dom/server.browser.js that switches between dev/prod of the browser build.

I wanted to do that but couldn’t decide on the naming. I like yours.

Member

gaearon commented Aug 2, 2017

It might be nice to keep a public react-dom/server.browser.js that switches between dev/prod of the browser build.

I wanted to do that but couldn’t decide on the naming. I like yours.

@sebmarkbage

Nice!

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 2, 2017

Member

Just tested in Node, webpack, and browserify, and it works as expected.

Member

gaearon commented Aug 2, 2017

Just tested in Node, webpack, and browserify, and it works as expected.

@gaearon gaearon merged commit fc86ef0 into facebook:master Aug 2, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@gaearon gaearon referenced this pull request Aug 2, 2017

Closed

React 16 Umbrella ☂️ (and 15.5) #8854

117 of 120 tasks complete
@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Aug 2, 2017

Member

Oops, sorry 😛
I'll fix

Member

gaearon commented Aug 2, 2017

Oops, sorry 😛
I'll fix

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon
Member

gaearon commented Aug 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment