redfin / react-server

:rocket: Blazing fast page load and seamless navigation.
https://react-server.io/
Apache License 2.0
3.89k stars 184 forks source link

Support koa #235

Open doug-wade opened 8 years ago

doug-wade commented 8 years ago

I would love to see a future version of this that uses Koa instead of Express.

We might consider allowing users to bring their own server (connect, hapi, etc) as well.

Generated from #50

doug-wade commented 8 years ago

I poked at this for a while today, and I wanted to put my findings somewhere.

The change in react-server-cli is actually fairly straightforward; i've attached it at the end of this comment. The real fun comes in when you get into renderMiddleware. React Server uses express-state to expose the React Server cache to the client side, which seems to be replaced with ctx.state in koa, but since koa expects you to use this to access ctx where express-state has you call res.expose(), the amount of refactoring required is pretty large. Unfortunately, its hard to do because the server just never starts, rather than throwing, so identifying the next step and taking it is pretty hard; we might want to look into our error handling in renderMiddleware to make sure we aren't swallowing any before digging further into this. The request object is already wrapped with an adapter, though, so adding a new wrapper around a koa request that has the same api shouldn't be too hard. I think we'll need to make a similar adapter for the servers themselves; even just to even out the different between express's

const httpServer = httpsOptions ? https.createServer(httpsOptions, server) : http.createServer(server);

and koa's

const httpServer = httpsOptions ? https.createServer(httpsOptions, server.callback()) : http.createServer(server.callback());

without an awful switch statement in react server. We may want to block on #33, however, so we can swap out startServer implementations, or add better documentation so that users' configure their own express/koa/hapi instance (so they can put their api server on the same server instance as their html or js server).

diff --git a/packages/react-server-cli/src/startServer.js b/packages/react-server-cli/src/startServer.js
index 474a295..21c72ae 100644
--- a/packages/react-server-cli/src/startServer.js
+++ b/packages/react-server-cli/src/startServer.js
@@ -1,7 +1,8 @@
 import reactServer, { logging } from "react-server"
 import http from "http"
 import https from "https"
-import express from "express"
+import koa from "koa"
+import serve from "koa-static"
 import path from "path"
 import pem from "pem"
 import compression from "compression"
@@ -117,29 +118,28 @@ const startImpl = ({
 // http://localhost:port/. returns an object with two properties, started and stop;
 // see the default function doc for explanation.
 const startHtmlServer = (serverRoutes, port, httpsOptions) => {
-   const server = express();
-   const httpServer = httpsOptions ? https.createServer(httpsOptions, server) : http.createServer(server);
+   const server = koa();
+   const httpServer = httpsOptions ? https.createServer(httpsOptions, server.callback()) : http.createServer(server.callback());
+   httpServer.on('error', (e) => {
+       console.error("Error starting up HTML server");
+       console.error(e);
+       reject(e);
+   });
    return {
        stop: serverToStopPromise(httpServer),
        started: new Promise((resolve, reject) => {
            logger.info("Starting HTML server...");

-           server.use(compression());
-           reactServer.middleware(server, require(serverRoutes));
-
-           httpServer.on('error', (e) => {
-               console.error("Error starting up HTML server");
-               console.error(e);
+           // server.use(compression());
+           try {
+               reactServer.middleware(server, require(serverRoutes));
+               httpServer.listen(port);
+           } catch (e) {
+               loger.error(e);
                reject(e);
-           });
-           httpServer.listen(port, (e) => {
-               if (e) {
-                   reject(e);
-                   return;
-               }
-               logger.info(`Started HTML server over ${httpsOptions ? "HTTPS" : "HTTP"} on port ${port}`);
-               resolve();
-           });
+           }
+           logger.info(`Started HTML server over ${httpsOptions ? "HTTPS" : "HTTP"} on port ${port}`);
+           resolve();
        }),
    };
 };
@@ -149,8 +149,12 @@ const startHtmlServer = (serverRoutes, port, httpsOptions) => {
 // static compiled JavaScript. returns an object with two properties, started and stop;
 // see the default function doc for explanation.
 const startStaticJsServer = (compiler, port, longTermCaching, httpsOptions) => {
-   const server = express();
-   const httpServer = httpsOptions ? https.createServer(httpsOptions, server) : http.createServer(server);
+   const server = koa();
+   // TODO: make this parameterized based on what is returned from compileClient
+   server.use(serve(`__clientTemp/build`, {
+       maxage: longTermCaching ? '365d' : '0s',
+   }));
+   const httpServer = httpsOptions ? https.createServer(httpsOptions, server.callback()) : http.createServer(server.callback());
    return {
        stop: serverToStopPromise(httpServer),
        started: new Promise((resolve, reject) => {
@@ -162,10 +166,6 @@ const startStaticJsServer = (compiler, port, longTermCaching, httpsOptions) => {
                }

                logger.debug("Successfully compiled static JavaScript.");
-               // TODO: make this parameterized based on what is returned from compileClient
-               server.use('/', compression(), express.static(`__clientTemp/build`, {
-                   maxage: longTermCaching ? '365d' : '0s',
-               }));
                logger.info("Starting static JavaScript server...");

                httpServer.on('error', (e) => {
roblg commented 8 years ago

re: express-state -- we actually don't do anything interesting with that. In my internal http2 prototype branch, I'm pretty sure I just replaced it with JSON.stringify.

Haven't thought about this rest of this deeply yet.