Skip to content
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

Serverside rendering and Async Routes #477

Closed
kevinpanxc opened this issue Jul 14, 2016 · 20 comments
Closed

Serverside rendering and Async Routes #477

kevinpanxc opened this issue Jul 14, 2016 · 20 comments

Comments

@kevinpanxc
Copy link

kevinpanxc commented Jul 14, 2016

I've been trying to get React-router's dynamic routing to work with ReactOnRails and found that it's impossible right now with server-side rendering turned on. The problem is that the client-generated markup and the server-generated markup are different. This is expected as code splitting implies the client needs to make ajax requests to fetch new code to render React components. Client markup will certainly be different from server markup until the requests are complete and the new components are rendered.

React-router has a solution for this and they outline it here: React-router async routes.

Specifically, I would like to do this:

match({ history, routes }, (error, redirectLocation, renderProps) => {
  render(<Router {...renderProps} />, mountNode)
})

As mentioned in the previous link, we will now be waiting for all async behaviour to be resolved before comparing client/server markups. This allows for the client to load the split code and render any missing React components before the markup comparison and React will not complain of a mismatch.

However, as I see it from clientStartup.js, the render method is highly inflexible and it is impossible to place a wrapper around it.

Will there be greater support for code splitting (specifically client/server markup parity) in ReactOnRails in the future?

Thanks!
Other than this, it's been a pleasure to work with ReactOnRails so far.

@justin808
Copy link
Member

@kevinpanxc We can hack on clientStartup.js and put together a PR. What do you want it to do?

@kevinpanxc
Copy link
Author

kevinpanxc commented Jul 14, 2016

@justin808 I'm not sure what would be the best way to do it, but perhaps an API to allow me to specify a wrapper function around ReactDom.render in clientStartup.js so that we can do the match thing that React-router does.

Main point is to have greater support for the dynamic routing feature of React-router.

@kevinpanxc
Copy link
Author

One thing I can see that would be iffy is how we pass the history and routes objects to React-router's match function.

Any ideas on how we can cleanly do it?

@kevinpanxc
Copy link
Author

kevinpanxc commented Jul 14, 2016

Wait, maybe a good way would be to adapt ComponentRegistry.js to take in promises, then we wouldn't have to deal with passing esoteric values to clientStartup.js. We would create a promise in AppClient.jsx (as in ReactOnRails.register({ AppClient })) that returns the top level react component on complete and clientStartup.js can call ReactDom.render at this point.

@justin808
Copy link
Member

@kevinpanxc Please create a demo app and a PR that demonstrates what you'd like to do.

@robwise
Copy link
Contributor

robwise commented Jul 22, 2016

@kevinpanxc are you sure you can't do this with how ReactOnRails is currently written? I'm doing completely different things when server rendering versus client rendering (including using match) with ReactOnRails. The key is using a let closure for the match callback (credit: @alexfedoseev). See code sample:

import ReactOnRails from 'react-on-rails';
import React from 'react';
import { browserHistory, RouterContext, Router, Route, match } from 'react-router';

import routes from './routes'; // custom to your app

const getClientMyComponent = (props, railsContext) => {
   // feel free to use props/railsContext here
  return <Router history={history}>{routes}</Router>;
};

const getServerMyComponent = (props, railsContext) => {
   // feel free to use props/railsContext here
  const { location } = railsContext;
  let retVal;

  const matchCallback = (error, redirectLocation, renderProps) => {
    retVal = (error || redirectLocation)
       ? { error, redirectLocation }
       : retVal = <RouterContext {...renderProps} />;
  };

  match({ routes, location }, matchCallback);

  return retVal;
};

const MyComponent = (props, railsContext) => {
  const isServerSide = typeof window === 'undefined';

  return isServerSide
    ? getServerMyComponent(props, railsContext)
    : getClientMyComponent(props, railsContext);
};

ReactOnRails.register({ MyComponent });

If you use my code sample, but convert the getClientMyComponent function to do something similar to the getServerMyComponent function, but using <Router> instead of <RouterContext>, would that work? I mean something like this:

const getClientMyComponent = (props, railsContext) => {
   // feel free to use props/railsContext here
  const { location } = railsContext;
  let retVal;

  const matchCallback = (error, redirectLocation, renderProps) => {
    retVal = (error || redirectLocation)
       ? { error, redirectLocation }
       : retVal = <Router {...renderProps} />;
  };

  match({ routes, location }, matchCallback);

  return retVal;
};

@kevinpanxc
Copy link
Author

@robwise I will try that and report the results in a couple days... caught up with school work right now. Thanks for the heads up! 👍

@alex35mil
Copy link
Member

I'm pretty sure it's not possible b/c of ExecJS.
On Sun, 24 Jul 2016 at 10:24, Kevin Pan [email protected] wrote:

@robwise https://github.com/robwise I will try that and report the
results in a couple days... caught up with school work right now. Thanks
for the heads up! 👍


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#477 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEDDG84evk-Vhg38Fidxxeo1EyDKOZBzks5qYszggaJpZM4JMFqI
.

Regards, Alex Fedoseev
alexfedoseev.com · https://twitter.com/alexfedoseev

@justin808
Copy link
Member

@alexfedoseev Should we close this one as basically impossible b/c we're not using a separate Node server for rendering? Or maybe this is something a Node server could handle?

@alex35mil
Copy link
Member

Should we close this one as basically impossible b/c we're not using a separate Node server for rendering? Or maybe this is something a Node server could handle?

@justin808 Yeah, it's impossible w/o separate Node server which accepts http requests and send http responses. In the node renderer implemented by @alleycat-at-git we simply eval js string synchronously, so all async stuff won't be handled correctly.

@justin808
Copy link
Member

justin808 commented Aug 9, 2016

@kevinpanxc Any compelling use to case to suggest that we need to support "Serverside rendering and Async Routes"?

@jtibbertsma
Copy link
Contributor

jtibbertsma commented Oct 18, 2016

@justin808 I'm in the exact same situation as OP. I'm using server rendering and webpack code splitting, and when I request a page that has a chunk that needs to be requested asynchronously, React gives me an error saying that the client generated code doesn't match the server generated code. This happens because ReactRouter renders a comment while it waits for the webpackJsonP request to be completed. The solution is to wait until the chunk is fetched before doing the initial render.

Right now, I've solved this problem by not registering the component that contains the router, and rendering it myself. It works fine, but it's kind of an ugly solution, because now ReactOnRails throws an error complaining that I haven't registered the component.

What would be better is if ReactOnRails provided one more registering API, something like registerRenderer. The function you pass would accept three arguments: props, railsContext, and domNodeId, and would be responsible for calling ReactDOM.render. That way I could wait and render after the chunk is fetched from the server. Here's an example of what I want:

page.html.erb

<%= redux_store_hydration_data %>
<%= react_component("NavigationApp", prerender: true) %>
<%= react_component("RouterApp", prerender: true) %>

clientRegistration.js

import ReactOnRails from 'react-on-rails';
import NavigationApp from './NavigationApp';
import RouterApp from './renderRouterApp';
import applicationStore from '../store/applicationStore';

ReactOnRails.registerStore({applicationStore});
ReactOnRails.register({NavigationApp});
ReactOnRails.registerRenderer({RouterApp});

renderRouterApp.jsx

import ReactOnRails from 'react-on-rails';
import React from 'react';
import ReactDOM from 'react-dom';
import Router from 'react-router/lib/Router';
import match from 'react-router/lib/match';
import browserHistory from 'react-router/lib/browserHistory';
import { Provider } from 'react-redux';

import routes from '../routes/routes';


const renderRouterApp = (props, railsContext, domNodeId) => {
  const store = ReactOnRails.getStore('applicationStore');

  match({ history: browserHistory, routes }, (error, redirectionLocation, renderProps) => {
    if (error) {
      throw error;
    }

    const component = (
      <Provider store={store}>
        <Router {...renderProps} />
      </Provider>
    );

    ReactDOM.render(component, document.getElementById(domNodeId));
  });
};

export default renderRouterApp;

I plan on implementing this myself and submitting a pull request in the next week or so. Any feedback?

@alex35mil
Copy link
Member

@kevinpanxc How do you plan to "wait" on server in ExecJS env?

@jtibbertsma
Copy link
Contributor

There's no need to wait on the server side. You would just call register like normal.

@justin808
Copy link
Member

@jtibbertsma I look forward to seeing your PR.

@alex35mil
Copy link
Member

Right now, I've solved this problem by not registering the component that contains the router, and rendering it myself.

@jtibbertsma From this part I assume you're just doing router's work manually on the server, removing the router related parts from the bundle.

@jtibbertsma
Copy link
Contributor

Right now, I've solved this problem by not registering the component that contains the router, and rendering it myself.

@jtibbertsma From this part I assume you're just doing router's work manually on the server, removing the router related parts from the bundle.

No, I'm using the router on the server as well. On the client side, I don't register the component and render it myself. On the server side, I register it like normal and let react on rails do the rendering.

@alex35mil
Copy link
Member

@jtibbertsma I don't get then. If you're using code splitting on server you are calling require.ensure/System.import to fetch the bundle in your routes components. Those methods are callback / promise based. How do you handle this on the synchronous server?

@jtibbertsma
Copy link
Contributor

Oh, I see what your asking. I've actually got separate route files for server and client. In the client routes I use System.import and in the server routes I import directly. So no code splitting on server side. There's really no benefit to using code splitting for the server bundle.

@alex35mil
Copy link
Member

I see, ok.

jtibbertsma added a commit to jtibbertsma/react_on_rails that referenced this issue Oct 25, 2016
The line doc/additional-reading/code-splitting.md should be
changed to a url when/if this proposal gets accepted.

This proposal was originally desribed here:
shakacode#477
jtibbertsma added a commit to jtibbertsma/react_on_rails that referenced this issue Nov 30, 2016
Made the following changes to the node package:

* ComponentRegistry.js: Modified register to detect generator functions
  with three arguments. Set the isRenderer key to true for these
  functions.
* clientStartup.js: Added logic to delegate to renderer functions.
* createReactElement.js: Now accepts an object instead of a component
  name.
* serverRenderReactComponent.js: Throws an error if attempting to
  render a renderer function.
* ReactOnRails.js: Change render function to call createReactElement
  with the component object.

Doc changes:

* README.md: Added section about renderer function under the section
  on generator functions. Moved the section on generator functions
  from the 'ReactOnRails View Helpers API' section to the 'Globally
  Exposing Your React Components' section.
* Added a file code-splitting.md that describes how to use renderer
  functions to do code splitting with server rendering.

Tests:

* ComponentRegistry.test.js: Modified existing test cases to expect
  the isRenderer key to be false. Added a few test cases related to
  renderer functions.
* serverRenderReactComponent.test.js: Show that an error gets thrown
  if trying to server render with a renderer function.
* spec/dummy: Added two examples using rendering functions, one of
  which implements code splitting. Added three test to
  integration_spec.rb.

Resolves: shakacode#477
jtibbertsma added a commit to jtibbertsma/react_on_rails that referenced this issue Nov 30, 2016
Made the following changes to the node package:

* ComponentRegistry.js: Modified register to detect generator functions
  with three arguments. Set the isRenderer key to true for these
  functions.
* clientStartup.js: Added logic to delegate to renderer functions.
* createReactElement.js: Now accepts an object instead of a component
  name.
* serverRenderReactComponent.js: Throws an error if attempting to
  render a renderer function.
* ReactOnRails.js: Change render function to call createReactElement
  with the component object.

Doc changes:

* README.md: Added section about renderer function under the section
  on generator functions. Moved the section on generator functions
  from the 'ReactOnRails View Helpers API' section to the 'Globally
  Exposing Your React Components' section.
* Added a file code-splitting.md that describes how to use renderer
  functions to do code splitting with server rendering.

Tests:

* ComponentRegistry.test.js: Modified existing test cases to expect
  the isRenderer key to be false. Added a few test cases related to
  renderer functions.
* serverRenderReactComponent.test.js: Show that an error gets thrown
  if trying to server render with a renderer function.
* spec/dummy: Added two examples using rendering functions, one of
  which implements code splitting. Added three test to
  integration_spec.rb.

Resolves: shakacode#477
jtibbertsma added a commit to jtibbertsma/react_on_rails that referenced this issue Nov 30, 2016
Made the following changes to the node package:

* ComponentRegistry.js: Modified register to detect generator functions
  with three arguments. Set the isRenderer key to true for these
  functions.
* clientStartup.js: Added logic to delegate to renderer functions.
* createReactElement.js: Now accepts an object instead of a component
  name.
* serverRenderReactComponent.js: Throws an error if attempting to
  render a renderer function.
* ReactOnRails.js: Change render function to call createReactElement
  with the component object.

Doc changes:

* README.md: Added section about renderer function under the section
  on generator functions. Moved the section on generator functions
  from the 'ReactOnRails View Helpers API' section to the 'Globally
  Exposing Your React Components' section.
* Added a file code-splitting.md that describes how to use renderer
  functions to do code splitting with server rendering.

Tests:

* ComponentRegistry.test.js: Modified existing test cases to expect
  the isRenderer key to be false. Added a few test cases related to
  renderer functions.
* serverRenderReactComponent.test.js: Show that an error gets thrown
  if trying to server render with a renderer function.
* spec/dummy: Added two examples using rendering functions, one of
  which implements code splitting. Added three test to
  integration_spec.rb.

Resolves: shakacode#477
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants