wdjungst / react-project

State of the art web development with React.js.
1.83k stars 136 forks source link

Add express middleware hooks #10

Open jaredpalmer opened 8 years ago

jaredpalmer commented 8 years ago
This would allow some optional server customization, but still keep everything 'battery-pack included'

// PublicServerAPI.js
export function createServer({ renderDocument, renderApp, routes, devMiddlewares, prodMiddlewares }) {
  const server = express()
  const webpackStats = getWebpackStats()

  if (process.env.NODE_ENV === 'production') {
    server.use(compression())
    prodMiddlewares(server)
    server.use(express.static(PUBLIC_DIR))
  } else {
    devMiddlewares(server)
  }
...
// Example middleware hooks (psuedo code)
// modules/server.js
...
import { createServer } from 'react-project/server'
..
import morgan from 'morgan'
import newrelic from 'newrelic'
...

function devMiddlewares(server) {
  server.use(morgan('dev'))
}

function prodMiddlewares(server) {
  server.use(morgan('combined'))
  server.locals.newrelic = newrelic
}

createServer({
  renderDocument,
  renderApp,
  routes,
  devMiddlewares,
  prodMiddlewares
}).start()
...
ryanflorence commented 8 years ago

This is interesting, but is there anything wrong with:

// Example middleware hooks (psuedo code)
// modules/server.js
...
import { createServer } from 'react-project/server'
..
import morgan from 'morgan'
import newrelic from 'newrelic'
...

const server = createServer({
  renderDocument,
  renderApp,
  routes
})

if (process.env.NODE_ENV === 'development') {
  server.use(morgan('dev'))
}

if (process.env.NODE_ENV === 'production') {
  server.use(morgan('combined'))
  server.locals.newrelic = newrelic
}

server.start()

I'm a little hesitant to become a wrapper over express.

jakedahm commented 8 years ago

@ryanflorence middleware needs to be defined before the routes are defined.

jaredpalmer commented 8 years ago

@ryanflorence 100% agree, but if we don't want to wrap express, then maybe we should do the following:

  1. Move all the great default express-specific middleware to the server.js blueprint file
  2. Keep the awesome rendering functions you wrote in the react-project server api
ryanflorence commented 8 years ago

^ how about that?

jaredpalmer commented 8 years ago

LGTM

ryanflorence commented 8 years ago

I actually want to just put morgan in by default

jaredpalmer commented 8 years ago

Pretty sure this is now solved with the createServer API.

// server.js
...

import morgan from 'morgan'

...

const server = createServer(getApp)
process.env.NODE_ENV !== 'production' ? server.use(morgan('dev')) : server.use(morgan('combined'))
server.start()
ryanflorence commented 8 years ago

I'm leaving this open because I am interested in having nicer hooks around things as we learn what is commonly added/replaced/removed.

jaredpalmer commented 8 years ago

fair enough.