[added] Router.renderRoutesToString

[karlmikko]

This PR allows the path to be given to as a prop.

<Routes initialPath="/some/path"> ... </Routes>

This only works server side. This patch also stops the URLStore and RoutesStore being started as componentWillMount is called but componentWillUnmount isn't call on server side. This would lead to a memory leak and potential async issue when multiple pages are being rendered at once.

When rendering on server side you should always make a new instance of <Routes> so if multiple pages are being rendered they don't change the routes in another page.

var App = React.createClass({
  render: function(){
    return <Routes initialPath={this.props.initialPath} location="history"> ... </Routes>
  }
});

//server side
React.renderComponentToString(<App initialPath="/some/path" />);

//client side
React.renderCompoent(<App />, domelement);

[ryanflorence]

Yeah, I think we will need this regardless of the final strategy.

Can you name it initialPath instead of path?

[karlmikko]

Done!

[karlmikko]

Now WIP with renderRoutesToString function calling getInitialActiveState and passing that state back to the route on render.

Please give me feedback. I am going to close the PR till finished.

What isn't working ATM is ActiveState for <Link/> component - I had it working at one point, just can't work it our right now.

Also for some reason with this setup React is re-rendering on client side as checksum is invalid. Even when there is only one routeHandler returning <div>Hi</div>. Maybe I am missing something there also.

TODO: serialise the initial data and pass to client to be passed to <Routes /> on initial render.

[karlmikko]

module to use from express

require('node-jsx').install({harmony: true});

var fs = require("fs");
var React = require("react");

var Router = require("react-router");

var app_router = require("../../client/app/app_router");
var router = require('express').Router({caseSensitive: true, strict: true});

//only read on startup
var template = fs.readFileSync(__dirname + "/../../client/app.html", {encoding:'utf8'});

//wildcard route to pass to react client app
router.get('*', function(req, res) {
  if(req.url == '/favicon.ico'){
    return res.status(404).end();
  }
  Router.renderRoutesToString(app_router, req.url, req.query).then(function(data){

    var html = template.replace(/\{\{body\}\}/, data.html);
    html = html.replace(/\{\{initialData\}\}/, JSON.stringify(data.initialData));
    res.send(html);

  }, function(err){
    console.error(err.stack);
    res.send("Error")
  });
});

module.exports = router;

[karlmikko]

Re-opened for review - this may need a bit of cleaning up.

And may also require the promise chain to be neater.

Will document and rebase after review incase the interface needs to change.

Do we want to create a middleware for express for react-router? Where you can pass in the <Routes /> and template, then let the middleware to the rest.

Resolves #57

Router.renderComponentToString(routes, url, query) returns a promise that will give an object with {html:string, initialData:object}

Probably should implement a way of returning a status also for rendering 404 with the correct http status pages from within the router.

---

I found another issues while I was in there.

React-Router will render to null first then on first dispatch the state is set with the path and matches. As first dispatch is async operation called after initial transition is resolved.

This caused issues when rendering from the server as it would destroy the dom to render null then re-create the dom.

[karlmikko]

To view a working app

https://github.com/karlmikko/bleeding-edge-sample-app/tree/react-router

[mjackson]

@karlmikko Thank you for taking this on! I don't have time to review ATM, but will try to find some time this weekend.

Also, would you please stick with existing coding conventions, particularly WRT whitespace? :D Thanks!

[karlmikko]

I need to write some tests - i will clean it all up also WRT to style - more wanted to make sure i was on the right track before spending all that time

[ryanflorence]

don't worry about style, better things to worry about right now :)

[karlmikko]

A special thanks to @petehunt and @rpflorence for taking the time to explain the particulars of the the problem and how this solution should work. Much appreciated!

[ryanflorence]

we shouldn't need query here. If you're in express just use req.originalUrl or plain node req.url

[karlmikko]

I can make that change!

[ryanflorence]

I've been thinking about the redirects, and not found.

Since we have <Redirect/> would could easily just fail the promise and give "redirect" as the reason, with the new url, and the server can then do whatever it wants for redirects.

I'd also like <NotFound/> routes. Same thing here, we just fail the promise and give the reason "not found" and the server can do what it needs. We'd also still provide the HTML so they can still render the app but provide a 500 status code.

[ryanflorence]

app.get('*', function(req, res) {
  Router.renderRoutesToString(routes, req.originalUrl).then(function(result) {
    res.send(result.html);
  }, function(result) {
    if (result.NOT_FOUND)
      res.send(404, result.html);
    else if (result.REDIRECT)
      res.redirect(result.redirectUrl);
    else
      res.send(500);
  });
});

[karlmikko]

I think should be a Route so the client side can display a 404 page when a route can't be displayed. Currently I am using this pattern https://github.com/karlmikko/bleeding-edge-sample-app/blob/react-router/client/app/app_router.js.

I have been thinking more on how we could implement this as Redirect and NotFound are not classical errors.

We could attach a prop of httpStatus 404 on the route. When renderComponentToString runs, it could check if any matches have a httpStatus and pass that back as part of the result object.

Redirect could do the same.

[ryanflorence]

Woops, meant 404, not 500, see my code samples from #142 (comment)

[ryanflorence]

we don't have to worry about redirect / not found just yet, lets get the happy path working and then follow up with discussion about handling these other cases.

[karlmikko]

The only issues with this PR now are Docs / TESTS and ActiveState isn't working properly for <Link/>

Other than that I think it is a goer.

[ryanflorence]

I'm pretty sure #169 will either help or really frustrate this PR

[karlmikko]

I don't think it would hinder this to much, not help to much - as there isn't any "history" in a http request - they are by nature stateless. We introduce state with cookies/session. However the fact the browser can change history and not notify the server means you have a conflict on which is the authority of truth on history.

I think it is best to not have history on the sever. As such back should probably render a 404 route as server side rendering is for "initial" page view and SEO.

[karlmikko]

@rpflorence what do you want me to do to land this PR?

I will need help writing the tests for this, especially with the async behaviour.

[mjackson]

I agree we should resolve #169 first. I'm going to take a look as soon as I can.

[mjackson]

This is what I thought we were talking about in #111

[robgraeber]

Does the initial path option still work? I'm getting "ReferenceError: navigator is not defined"

<Routes initialPath="/some/path" location="history"> ... </Routes>

[forestbelton]

is there any progress on this PR? we need it for production pretty badly

[appsforartists]

@forestbelton you can use it now (I am). It works great for static stuff, but anything that needs to be asynchronously fetched is blocked by #261. That will land, then @mjackson and @karlmikko will work on retrofitting this PR (and merging in all the changes since it was written).

What do you need it for? Curious how you ended up with a production site being blocked by the lack of an experimental feature. 😄

[mjackson]

All: I'm working on getting this functionality into the router as soon as possible. I recognize that this adds a ton of value for a lot of users, and it's at the top of my TODO list. Thanks for your patience. :)

[forestbelton]

cool stuff! i'll check out the PR and use it for the time being then. @appsforartists the reason we need it is because server-side rendering is a hard requirement for many pages in our application. right now those pages aren't using react-router, but i'd very much like them to be :) i appreciate all the work done on this stuff so far! we are getting a lot of use out of it.

[rileyjshaw]

@karlmikko you are "heaps good".. :)
Thanks for doing this.

[mjackson]

I just pushed 1b1a62b which adds server-side rendering to the architecture the router is currently using.

Huge thanks to @karlmikko and everyone else who has chimed in on this thread! You brought to the surface a lot of important issues that needed to be addressed in the core architecture of the router before server-side rendering would work at scale. Thank you!

[appsforartists]

Congratulations, @mjackson and @karlmikko!!

[karlmikko]

Thanks to everyone who helped us understand the issues in Server Side rendering!

@mjackson Thanks for the kind accolade and well done on the re-architecture! This was clearly a massive under taking!

[olance]

Congrats!! 👍

[nhunzaker]

Wow. This is amazing. Excellent work!

[hellojwilde]

This is awesomesauce! 👍

[robgraeber]

Awesome just tried it out and it works great! Btw since AsyncState has been removed, how do you do async server-side rendering now?

[tobice]

@robgraeber well apparently you can do some async work in willTransitionTo(). That works. But I'm not sure how it's intended to be used because this function is static. I can fetch data in this function but I have no idea how I am supposed to pass this data to a specific component instance.

[ryanflorence]

I worked on data loading all day and will continue this week until it's
done. VERY SOON.

On Monday, October 13, 2014, Tobiáš Potoček notifications@github.com
wrote:

@robgraeber https://github.com/robgraeber well apparently you can do
some async work in willTransitionTo(). That works. But I'm not sure how
it's intended to be used because this function is static. I can fetch data
in this function but I have no idea how I am supposed to pass this data to
a specific component instance.

—
Reply to this email directly or view it on GitHub
#181 (comment).

[vesparny]

@rpflorence any news about data loading?

[ryanflorence]

we did some work last night, hope to get stuff up this week :)

On Wed, Oct 29, 2014 at 9:29 AM, Alessandro Arnodo <notifications@github.com

wrote:

@rpflorence https://github.com/rpflorence any news about data loading?

—
Reply to this email directly or view it on GitHub
#181 (comment).

[vesparny]

cool, thanks

[tbo]

@rpflorence Is there a ticket or a branch where we can follow your progress?

[appsforartists]

@tbo Not hard to find. 😃

[tbo]

@appsforartists Thanks!