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

Make applyMiddleware optionally support a more "Nodeish" API #1932

Closed
dtothefp opened this issue Sep 6, 2016 · 3 comments
Closed

Make applyMiddleware optionally support a more "Nodeish" API #1932

dtothefp opened this issue Sep 6, 2016 · 3 comments

Comments

@dtothefp
Copy link

dtothefp commented Sep 6, 2016

Do you want to request a feature or report a bug?
feature

What is the current behavior?

function someMiddleware(opts) {
  return ({ dispatch, getState }) => next => action => //do stuff
}

would be nice and easier to understand for those used to Node/Express/Koa to do

function someMiddleware(opts) {
  return ({ dispatch, getState }) => (action, next) => //do stuff & call `next()`
}

Would be trivial to do and could be made backwards compatible for current middleware style by checking middleware function argument length

const [last] = chain.slice(-1);
const rest = chain.slice(0, -1);
const wrap = (fn, next) => {
  return fn.length === 2 ? (action) => fn(action, next) : fn(next);
};

const newDispatch = rest.reduceRight((next, fn) => wrap(fn, next), wrap(last, store.dispatch));

Sorry if similar issues have been previously posted (I didn't find any in my search) and wondering if you'd be open to a PR?

@markerikson
Copy link
Contributor

Yeah, variations on this have been discussed previously, and turned down. See #534 , #922, and #1744 for prior discussion.

@dtothefp
Copy link
Author

dtothefp commented Sep 6, 2016

😢 seems like being able to provide this API while at the same time supporting next => action through arguments check is best of both worlds ¯\_(ツ)_/¯

@jimbolla
Copy link
Contributor

jimbolla commented Sep 6, 2016

You could write your own applyMiddleware, as it's only about 20 lines of code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants