Coverage remained the same at 82.987% when pulling 63416d2 on jtibbertsma:add-new-api-register-renderer into 5e5b92e on shakacode:master.

@jtibbertsma Very intriguing!

1. Can you modify the example in spec/dummy to demonstrate this? Or make spec/dummy2?
2. Alternately a PR to https://github.com/shakacode/react-webpack-rails-tutorial/ would be great to demonstrate this. The PR can refer to your fork.

@robwise @alexfedoseev We'll have to give this a close look in the coming weeks. However, please stay focused on F&G v2! We can potentially leverage this when we make a big push for performance after v2 is completed!

@justin808 Sure, I'll work on a pr for react-webpack-rails-tutorial this weekend.

@jtibbertsma I'm looking forward to seeing the changes!

@jtibbertsma Any update?

@justin808 I've been a bit busy this week. I should have some time to work on it today.

Coverage remained the same at 82.987% when pulling 79d2e87 on jtibbertsma:add-new-api-register-renderer into 7fecce1 on shakacode:master.

@jtibbertsma Could have a simple example in /spec/dummy along with some simple tests of this? It's critical to keep really good test coverage to avoid regressions.

@justin808 Sure, I'll work on it this week

@jtibbertsma Keep me posted!

Coverage remained the same at 82.987% when pulling 9b98494 on jtibbertsma:add-new-api-register-renderer into 60d9f7f on shakacode:master.

Coverage remained the same at 82.987% when pulling 6bcf72f on jtibbertsma:add-new-api-register-renderer into 1174ab5 on shakacode:master.

@justin808 I added a simple integration test that shows that the renderer function gets called correctly.

@jtibbertsma REALLY AWESOME. Let's address the comments!

Reviewed 8 of 12 files at r1, 8 of 8 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

CHANGELOG.md, line 8 at r5 (raw file):

## [Unreleased]
##### Added
- New API registerRenderer which allows a user to manually render their app to the DOM by [jtibbertsma](https://github.com/jtibbertsma)

reference the PR, as done below for other changes

you can use the release 6.2.0

and change the bottom part showing the diff

README.md, line 456 at r5 (raw file):

1. [React on Rails docs for react-router](docs/additional-reading/react-router.md)
1. Examples in [spec/dummy/app/views/react_router](spec/dummy/app/views/react_router) and follow to the JavaScript code in the [spec/dummy/client/app/startup/ServerRouterApp.jsx](spec/dummy/client/app/startup/ServerRouterApp.jsx).

we should mention the registerRenderer here, or maybe in the docs for react-router.

README.md, line 476 at r5 (raw file):

  + [Turbolinks](docs/additional-reading/turbolinks.md)

+ **Javascript**

we need to refer to the page on code splitting here

docs/additional-reading/code-splitting.md, line 15 at r5 (raw file):

 (client) <!-- react-empty: 1 -
 (server) <div data-reactroot="

this is formatting so you have to scroll sideways a ton

please see if you can change that

docs/additional-reading/code-splitting.md, line 18 at r5 (raw file):

<!--This comment is here because the comment beginning on line 13 messes up Sublime's markdown parsing-->

Different markup is generated on the client than on the server. Why does this happen? When you register a component with `ReactOnRails.register`, react on rails will render the component as soon as the page loads. However, react-router renders a comment while waiting for the code chunk to be fetched from the server. This means that react will tear all of the server rendered code out of the DOM, and then rerender it a moment later once the code chunk arrives from the server, defeating most of the purpose of server rendering.

I'm a bit confused about the interaction between deferred rendering and react-router. Is the idea that a page is bookmarked, and react-router's deferred rendering counts as a first render, so that the server render and what's in react-router's deferred renders should be the same?

It would be great to have some graphic or other explanation of this.

What's not obvious is that we can server render what the deferred rendering will be.

docs/additional-reading/code-splitting.md, line 26 at r5 (raw file):

Here's an example of how you might use this in practice:

page.html.erb

Maybe use #### to format the file names.

docs/additional-reading/code-splitting.md, line 42 at r5 (raw file):

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

We need to be very clear that RouterApp will handle calling ReactDOM to create the react element, attached to a DOM node.

Should we be showing some serverRegistration.js page?

docs/additional-reading/code-splitting.md, line 85 at r5 (raw file):

### Caveats

If you're going to try to do code splitting with server rendered routes, it's important that you have seperate webpack configurations for client and server. The code splitting happens for the client, but the server should one big file.

should one big file

maybe:

should be one file containing all the JavaScript code

docs/api/javascript-api.md, line 64 at r5 (raw file):

   * Allows registration of renderers. A renderer is a function that accept three args:
   * props, railsContext, and domNodeId, and is responsible for rendering a component
   * to the DOM. Not available on the server. For one possible use case see:

We might be able to throw an error if this is called on the server since we'll know we're server rendering with the rails context.

node_package/src/ReactOnRails.js, line 114 at r5 (raw file):

    ComponentRegistry.registerRenderer(renderers);
  },

Please move this next to the definition immediately below registerStore

spec/dummy/client/app/startup/ManualRenderAppRenderer.jsx, line 12 at r5 (raw file):

  );

  ReactDOM.render(reactElement, document.getElementById(domNodeId));

I feel that we need the example to show react-router.

We use react-router in the dummy apps.

Another consideration is support for react-router v4.

Comments from Reviewable

I have one comment below.

And also one more comment on the global API: I'm not a big fan of owning key deps (as ReactDOM) w/o strong reason (is there any?), so leaving it in user land by default is IMO a better design decision, as we had already few issues w/ it.

Review status: all files reviewed at latest revision, 12 unresolved discussions.

docs/additional-reading/code-splitting.md, line 85 at r5 (raw file):

### Caveats

If you're going to try to do code splitting with server rendered routes, it's important that you have seperate webpack configurations for client and server. The code splitting happens for the client, but the server should one big file.

I don't think the main point here is different webpack configs (as client/server configs are usually different regardless of whether or not code splitting is used), but the fact that it requires different routes files for the client and the server (w/ same routes, but different components resolvers), which is most uncommon & untransparent part here.

Comments from Reviewable

Review status: 13 of 18 files reviewed at latest revision, 12 unresolved discussions.

docs/additional-reading/code-splitting.md, line 18 at r5 (raw file):

The server render matches the deferred render because the server bundle is a single file, and so it doesn't need to wait for anything to be fetched.

Comments from Reviewable

Review status: 13 of 18 files reviewed at latest revision, 12 unresolved discussions.

docs/api/javascript-api.md, line 64 at r5 (raw file):

Comments from Reviewable

Coverage remained the same at 82.987% when pulling 174a3ea on jtibbertsma:add-new-api-register-renderer into 1174ab5 on shakacode:master.

Review status: 13 of 18 files reviewed at latest revision, 12 unresolved discussions.

spec/dummy/client/app/startup/ManualRenderAppRenderer.jsx, line 12 at r5 (raw file):

Comments from Reviewable

Looking good!

Reviewed 5 of 5 files at r6.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

docs/additional-reading/code-splitting.md, line 18 at r5 (raw file):

docs/additional-reading/code-splitting.md, line 85 at r5 (raw file):

docs/additional-reading/code-splitting.md, line 58 at r6 (raw file):

});

Note that you should not use registerRenderer on the server. Instead, use register like normal. For an example of how to set up an app for server rendering, see the react router docs.

We'll know if we're server rendering. Could we allow the same registration code for client and server, and just ensure that we don't do the deferred render thing on the server?

Needing this to be different for the server rendering seems like a big gotcha.

Comments from Reviewable

@jtibbertsma I'd like to get this in the next release, maybe this weekend!

Review status: all files reviewed at latest revision, 5 unresolved discussions.

docs/additional-reading/code-splitting.md, line 85 at r5 (raw file):

docs/additional-reading/code-splitting.md, line 58 at r6 (raw file):

Comments from Reviewable

Review status: 17 of 18 files reviewed at latest revision, 5 unresolved discussions.

docs/additional-reading/code-splitting.md, line 18 at r5 (raw file):

Comments from Reviewable

Review status: 17 of 18 files reviewed at latest revision, 5 unresolved discussions.

docs/additional-reading/code-splitting.md, line 58 at r6 (raw file):

This is an important feature that we'll probably use.

The key question for me is if we should have separate registerRenderer and register APIs.

Since this is an API change, I want to make sure we get this right.

Possibly we'll implement this as a beta release to get some community feedback?

Comments from Reviewable

Reviewed 6 of 6 files at r17.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

Comments from Reviewable