Skip to content
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

Hook for custom middleware #708

Merged
merged 5 commits into from
Nov 17, 2016

Conversation

sresant
Copy link
Contributor

@sresant sresant commented Oct 11, 2016

Relates to #694.

@sresant sresant added the enhancement New functionality. label Oct 11, 2016
@@ -114,9 +114,22 @@ export default (webpackConfig) => {

In the `.reactserverrc` file add an option for `webpack-config` that points to that function file and when React Server is setting up Webpack it will call your function with the result of the built in Webpack options, allowing you to make any modifications needed.

### Use Custom Express Middleware
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gigabo i am the worst at documentation so let me know what you think

Copy link
Collaborator

@doug-wade doug-wade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Express Middleware is order-dependent, which makes me think that custom middleware loading will need to be more intricate than this. In particular, it seems like users will want to have middleware run before compression, but after cookies and body are parsed, or to replace the cookie or body parsing middleware. I think a better approach would be to load no middleware other than react-server if custom middleware are specified, and provide the current behavior as a default to custom middleware stack.

@gigabo
Copy link
Contributor

gigabo commented Oct 11, 2016

Hi @doug-wade! It looks like @sresant has actually accounted for that. The default behavior remains to apply the various other middlewares (compression, etc) in a certain order and then apply the React Server middleware. But if the new hook is used the caller becomes responsible for applying all middleware, including compression, etc and React Server's, in whatever order is desired.

server.use(bodyParser.urlencoded({ extended: false }));
server.use(bodyParser.json());
server.use(session(options));
rsMiddleware(); // Must be called once or server will not start
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match the parameter name above (reactServerMiddleware there).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!



In the `.reactserverrc` file add an option for `custom-middleware-path` that points to that function file and when React Server is setting up the server it will call your function for setup rather then the default middlewares mentioned above. On the command line the option is `--custom-middleware-path`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we've generally used the camelCase variants in the config file? Like customMiddlewarePath?

Not sure if either will work, or even if I'm remembering right. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, i was just tired when writing up the documentation good catch!

@doug-wade doug-wade merged commit c431cfc into redfin:master Nov 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants