-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consolidate HTML + JS Servers #808
Conversation
…. Replace dependency on webpack-dev-server with webpack-dev-middleware.
…emove dependency on webpack-dev-server
…single HTTP server.
# Conflicts: # packages/react-server-cli/package.json
} | ||
|
||
let middlewareSetup = (server, rsMiddleware) => { | ||
server.use(compression()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if that's a problem but it looks like the compression()
middleware is used two times in non-hot mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! I fixed that.
# Conflicts: # packages/react-server-cli/src/commands/start.js # packages/react-server-cli/src/compileClient.js
…moved secondary use of `compression()` in non-hot mode (leftover from when this was used in a secondary Express instance).
# Conflicts: # packages/react-server-cli/src/commands/start.js
Closing this PR, pending further architecture discussion. |
This PR addresses #807 with the least controversial set of changes. See PR #791 for more discussion on this. Essentially, this PR does the following:
jsPort
)hot === true
,webpack-dev-server
is used as to create the HTTP/HTTPS server and Express application.react-server
piggy backs off of the Express application by injecting itself into the middleware chain afterwebpack-dev-middleware
is injected. No proxying is required.hot === false
,react-server
creates the HTTP/HTTPS server and Express application on its own and manually creates and runs a Webpack compiler for the client side code.In my opinion, this is not the cleanest path for
react-server
, but it is the least controversial set of changes to the overall infrastructure while still reducing complexity at runtime.Another method of solving this problem is to stop using
webpack-dev-server
and attachwebpack-dev-middleware
to the Express application thatreact-server
creates. This will more easily allow people to use alternatives to Express in the future and keepreact-server
operating the same in hot/not-hot modes. It does, however, move the client-side HMR mechanism away from something that the Webpack people maintain and over towebpack-hot-middleware
. This project supports multiple compilers (which will be useful later), Webpack 2, and seems to be well maintained.Checkout this patch to see what this code would look like.
As always, feedback is appreciated.