Revisit server-side API and isolate into subdir

[mjackson]

I've seen a few people do weird things like try to use match on the client side. We should probably follow react-dom's lead and isolate our server-side API into a react-router/server directory in our build. This would make it obvious to people using it that it should only ever be used on the server and never on the client.

Potential API could be:

import { format as formatURL } from 'url'
import { renderToString } from 'react-dom/server'
import { match, createLocationFromNodeRequest } from 'react-router/server'
import { RouterContext } from 'react-router'

http.createServer((req, res) => {
  const location = createLocationFromNodeRequest(req)

  match(location, routes, (error, nextLocation, state) => {
    if (error) {
      res.status(500).send('Internal Server Error')
    } else if (nextLocation) {
      res.redirect(formatURL(nextLocation))
    } else {
      renderToString(...)
    }
  })
})

This would also hopefully make it more clear to people that they shouldn't be using a memory history on the server.

[taion]

match on the client side is required when using async routes with server-side rendering. This is because the client needs to delay calling ReactDOM.render until all async chunks are loaded; otherwise, there's a markup mismatch.

[mjackson]

Ah, you're right. Man, now I really want to kill getChildRoutes. :D

I'll update the OP. We can still have a createLocationFromNodeRequest method so people don't use history objects on the server.

[taion]

It's not just getChildRoutes – it's getComponent and getComponents as well, along with any async onEnter hooks. It was a huge pain to get everything wired up to work properly here.

Also, SSR still requires a history object or something implementing the same API for createHref to render <a href> on <Link>s.

[mjackson]

It was a huge pain to get everything wired up to work properly here.

You mean as a consumer of the router? Or are you talking about some work you did in the router itself?

[taion]

I mean in adding proper support for this in the router. #2965 was required to fully support this for async onEnter hooks, and it was a huge PITA.

[taion]

Hmm, I remember that code being a lot more painful to write when I first wrote it. Looking back at the diffs, it doesn't look as bad as I remembered.

[mjackson]

Gotcha.

The ideal situation for a client-side app is to not do any async matching at all on the initial render, but that's a limitation of our current code splitting approach. In other words, the initial match operation would be synchronous but all subsequent ones would be async.

Also, webpack 2 makes all calls to require.ensure async, even if the bundle is already loaded. So a synchronous match is impossible if we tell people to use require.ensure in their route configs.

Synchronous match on the initial render would be great though. Maybe we could prescribe a build step...

[taion]

It's not just about code splitting. #2965 was to fix issues where the user did something like have an async call in onEnter, like to an external API or something. This can't be solved by any amount of require.ensure caching (and in fact async from just routes worked before #2965).

[taion]

Sorry, tired – what I mean is, users can do arbitrary async things in onEnter, which means that route matching can never be assumed to be synchronous with the current API, even with a build step for the code splitting.

[mjackson]

Ya, you're right. Still, it's kind of annoying that people can't opt-in to a synchronous match on the initial render client-side when using code splitting. The whole reason we use callbacks instead of promises in the router for async work is to support server-side rendering, so we can render on the initial turn of the event loop if you're not doing anything async in your onEnter hooks. require.ensure ruins that.

[mjackson]

This also begs the question as to what on earth you're rendering while we do async work in match. It's just not right. We've got to figure this out.

[taion]

The browser is rendering the initial server-sent HTML. No client-side React rendering will yet have run. It won't have any event handlers or whatever wired up, so mostly the page won't respond in any JS-enabled way (though if you click on a <Link> or an <a>, the href will do what it should).

In other words, the same as what you get with SSR normally; it's just that you don't start loading the extra chunks until after you've finished parsing the entry chunk and running match.

[taion]

With some care, it's possible to shim the require.ensure calls on the server to track the required chunks, then send them to the client as part of the initial server-side payload, BTW. This will reduce waterfall issues with the round trips for the page to initially become responsive, but IMO it's not a super big deal, because you'll still have the server-rendered markup no matter what.

[mjackson]

When using the router to render server-side, the HTML from the server is generated by the router. Either the server or the client should be responsible for the match, but not both.

[ryanflorence]

Like jimmy said, you either call match on the client to tickle the code
splitting or have a smarter server that sends the chunks (not sure how to
do that with webpack).

The use case of code splitting and server rendering together is low
priority to me. In fact, server rendering generally is.
On Tue, Apr 12, 2016 at 11:44 PM Thriller notifications@github.com wrote:

When using the router to render server-side, the HTML from the server is
generated by the router. Either the server or the client should be
responsible for the match, but not both.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3290 (comment)

[timdorr]

It's super important to a product I'm about to start work on next month. I need the slimmest possible entry bundle for fast load times and server side rendering for SEO purposes.

Unfortunately, outside of going back to Router.run, React's lack of support for async rendering means you have to go outside of ReactDOM.render to make this happen. You need match (really "syncify") in order to wait for anything async. The good part is that it's optional, so you don't have to use it until you want to get into SSR and async routing behaviors.

[taion]

Without getting too deep into the weeds here, I think that's a matter of semantics – isomorphism requires both the client and the server to run an initial router match, and most likely if it's async on the server, the router will have to also support doing it in an async manner on the client (when using match() on the client, the <Router> skips its initial match and uses the result from match()).

Can you clarify what the action item here is? It's not ideal to discourage users from using match() on the client right now, given that it's required for most use cases involving server-side rendering and async routes (from either code splitting or async onEnter() hooks).

[taion]

On second thought, there is something here – the client and server APIs are not the same.

The client API expects a history but no location, while the server API expects usually just a location (sometimes it has to be a history if the user has custom history enhancers).

We've seen people get confused by using the wrong form of the match function on the client and ending up with markup mismatches. Maybe it'd be best to split these up.

[ryanflorence]

We need histories on the server because they know how to createHref, isActive and friends, so we either live with that or, figure out a way to separate path stuff. Today it looks like this:

// server
const history = useNamedRoutes(createMemoryHistory)({ routes })
match({ history, location, routes })

// client
const history = useNamedRoutes(createBrowserHistory)({ routes })
<Router history={history}/>

The only difference between the client and server API is where you pass the history. If we figured out some separation between the history and the path creator/active checker thing it becomes something like:

// server
const pathThing = createNamedRoutesPathThing({ routes })
match({ pathThing, location, routes })

// client
const history = useNamedRoutes(createBrowserHistory)({ routes })
<Router history={history}/>

The client doesn't change, but the server API is now even farther from the client API and router extensions, like use-named-routes, have to provide two APIs, before they only needed to provide one.

In the case of code-splitting + server rendering, we either be okay with using match for its async side-effects, or we make a new function for the client to tickle the right places of a route config but not do everything else:

match({ routes, history, window.location }, cb)
// becomes
tickle(routes, cb)

No matter what you've got to deal the statefulness of the render lifecycle on the client and the one-shot-get-everything-we-need-to-render of the server (and, indirectly, the checksum match on the client w/ async route config behaviors).

We tried to make it "the same" in both places with Router.run() pre-1.0, but that didn't pan out.

But here's what I really think:

It's ok. @jordwalke figured it out. You can fork the UI and then rebase the resolved promise on top of it. Working on it.

https://twitter.com/sebmarkbage/status/720389596261953537

1. We have an idea for how we could do forked behavior of components where they could live in two states at once and then rebase. This would allow a component to update itself (remain responsive) while a pending update is still being processed.

facebook/react#6481 (comment)

We should probably just keep doing what we're doing and wait to see what business @jordwalke and @sebmarkbage are cooking up.

[taion]

I agree that we can't change the guts of how things work. I think there's room for a small DX affordance, though.

• For server rendering, you always want to pass in location, but only sometimes want to pass in history (sometimes the default memory history with options is good enough)
• For client rendering when hydrating async routes, you always want to pass in history, but you should never pass in location

These two versions of match() do almost the same thing, but not quite. If we split them up (e.g. put the former under react-router/server or something), it might make it harder for users to accidentally mess up.

[taion]

Err, I think I might have just said the same thing as @ryanflorence's tickle() proposal above.

[ryanflorence]

I'm not sure I can 👍 it w/o being named tickle

[mjackson]

We need histories on the server because they know how to createHref, isActive and friends

Will everyone please stop reminding me of how code I wrote works? :) Thanks.

I obviously overlooked the server-side use case when working on the history library and coupled things too closely (my bad). My hope is that we can figure out how to split them apart so that we don't tell people to use history objects on the server, which makes no sense IMO.

If I could break down responsibilities we need on either side, it would probably look something like this:

Stuff we need on the server:

• createLocationFromNodeRequest
• createHref (for <Link>s)
• query string handling

Stuff we need on the client:

• getCurrentLocation
• listen (for location updates)
• transitions (listenBefore and onLeave hooks)
• beforeunload support
• basename support
• createHref (for <Link>s)
• query string handling

These responsibilities are different enough that I think it justifies us using a different API server-side than we use on the client, akin to how React itself uses renderToString (stateless) on the server vs. render (stateful) on the client. That's all I'm trying to say.

[ryanflorence]

Will everyone please stop reminding me of how code I wrote works? :) Thanks.

Only brought it up because your example code in the OP doesn't account for any of that and other people read these issues.

[taion]

I brought this up on remix-run/history#231 (comment), but what do we think of making a createDummyHistory or something that's like createMemoryHistory, except doesn't have listen, push, replace, &c. methods?

Then use that for server-side rendering. I think it gets us most of what we want with "not having a history on the server" and making that API fully stateless, but doesn't require rethinking the history enhancer thing.

[denvned]

I hope it is ok to continue the discussion (the part related to <Router> on the server) started in facebook/react#6232 here.

facebook/react#6232 (comment)

You may use , but on the server side this is duplicate behavior. It will result in us creating duplicate subscribers and internal objects, which is wasteful in a server app.

@timdorr, I guess it is not the case, because <Router> reuses internal objects produced by match since #2965. The only actual problem I found so far is that <Router> does not call this._unlisten on the server. But I think there should be better solution to that than forcing users to use another component (<RouterContext>) on the server when using more familiar <Router> could be enough. And it automatically solves the problem with applyRouterMiddleware on the server.

[taion]

There's no real difference between the two components in terms of API, though. The server-side and client-side match APIs are intended to be different, though – the form of the API that accepts a location is intended as server-only, while a history should be required on the client side.

[taion]

Folded into #3611.