koa support about universal-redux

[bartolkaruza]

I currently use koa under universal-redux, simply wrapping the express middleware; https://github.com/vkurchatkin/koa-connect

I'm having a look at making the renderer more dynamic, so it could work with either express or koa middleware. This part is tripping me up though;

if (config.redux.middleware) {
      const customMiddleware = require(path.resolve(config.redux.middleware)).default;
      each(customMiddleware, (customMiddlewareToAdd) => {
        if (typeof customMiddlewareToAdd === 'function') {
          middleware.push(customMiddlewareToAdd(req, res));
        }
      });
    }

The custom middleware I've worked with so far were not dependent on the express req, res objects, but I can imagine others might use it in some other way. Can you tell me what the intention is for passing the req and res into the custom middleware? Is there an example?

[bdefore]

@bartolkaruza I believe you are correct that this is unconventional, and was related to migrating the ApiClient.js from the react-redux-universal-hot-example project. That library needed access to the request object in order to retrieve and set cookies for authentication purposes. The modified version is in use in the JWT example here: https://github.com/bdefore/react-redux-universal-hot-example/blob/react-router-2/src/redux/middleware/fetcher.js

That said, I think it's more appropriate to follow Redux middleware conventions, and if need be, find another way to implement that fetcher. Would you agree that the appropriate way to assign the middleware should be without checking for function, nor executing it? Like so:

    const middleware = [];
    if (config.redux.middleware) {
      const customMiddleware = require(path.resolve(config.redux.middleware)).default;
      each(customMiddleware, (customMiddlewareToAdd) => {
        middleware.push(customMiddlewareToAdd);
      });
    }

Hit me up on Reactiflux if you'd like to chat more about it.

[bdefore]

You'll also want to similarly modify client.js. In fact now that req and res need not be applied, these should be pulled into a helper.

[bdefore]

Well, hey, even more terse: const middleware = config.redux.middleware ? require(path.resolve(config.redux.middleware)).default : [];

[bdefore]

@bartolkaruza I've released 3.0.0-rc3 based on these changes

[bartolkaruza]

I've made a first attempt, untested;
bartolkaruza@ec50f67

I'm curious what you think of this approach.

[bdefore]

@bartolkaruza that actually looks about like what i was thinking too. i like how you've managed to contain the original logic into dynamicMiddleware. would be great to see this in a PR with an addition to examples that's working with koa.

one thing I'm wondering: would it be possible to glean whether the server is koa or express based on the incoming arguments rather than require configuration? does the response object already come with the identifier by the time it hits this middleware?

[bdefore]

@bartolkaruza ping!

[bartolkaruza]

@bdefore I saw you merged the PR. Great stuff. Close this issue?

[bdefore]

@bartolkaruza yup, thanks!