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

RFC: Simplify middleware signature #1744

Closed
gaearon opened this issue May 18, 2016 · 7 comments
Closed

RFC: Simplify middleware signature #1744

gaearon opened this issue May 18, 2016 · 7 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented May 18, 2016

This was proposed numerous times before (#784, happypoulp/redux-tutorial#48), and I still hear about the Redux “obnoxious” middleware signature. I think the complaint comes from the fact that most middleware never uses store argument so it’s far from being clear why it’s required.

I’m not a big fan of changing things, but we have barely made any breaking changes since the release, and since we have an overhaul for store enhancers in #1702, I realized that if I was working on Redux 1.0 today, I’d keep the middleware signature simpler. I’m happy to release 4.0 alpha and keep it as next version for several months to let middleware authors catch up.

The signature I’m proposing is:

const logger = (next) => (action) => {
  // ...
}

Note that most middleware only care about the next argument so this is why it should be the first rather than store. You can then get store if you need to read the state or dispatch from the beginning of the chain—both are advanced use cases:

const thunk = (next, store) => (action) => {
  // ...
}

As a migration strategy, I suggest a compat library:

import { createStore, applyMiddleware } from 'redux'
import { wrapCompatMiddleware } from 'redux-compat'

const store = createStore(
  reducer,
  applyMiddleware(
    thunk, // new-style middleware
    wrapCompatMiddleware(logger), // middleware that hasn't been updated yet
  )
)

This will let consumers jump into 4.x right away but since they’d have to add a tiny compat package, that would put some pressure onto middleware authors to update their middleware for 4.x.

Finally, we can avoid doing this at all. However I don’t see another round of breaking changes like 4.x, so this is the only opportunity where I’m ready to do it. I’ve been telling people for months that “it’s just currying” but I can’t give a compelling reason for it other than “we just happened to choose this style”. And I do think this style is confusing to beginners, just like the original style of applying enhancers was. I think we improved it in 3.1.0, and I think we can improve this as well.

What do you think?

@gaearon gaearon added this to the 4.0 milestone May 18, 2016
@brycehanscomb
Copy link

brycehanscomb commented May 18, 2016

What would be the considerations for the long-term management of redux-compat?

Since the nature of compat libs like this is to help across versions, how would you manage this if wrapCompatMiddleware needs to behave differently for some later inter-version compatibility?

Maybe we have redux-compat-compat? :-P

@glenjamin
Copy link

I'm unconvinced. This would inconvenience "ordinary" users for a minor gain for middleware authors.

There are far more users than middleware authors, and this only comes up when you first create a new middleware.

@nickpresta
Copy link

I know this is only a single sample but we have 5 custom middlewares of various complexity (some for analytics, some for integrating with third party plugins that hijack window, etc) and we use the store argument in 4 of them.

I like that the signature is explicit and is really only "ugly" when using arrows, IMO.

@natenorberg
Copy link

Is the only reason for the change to make it less confusing to those that are new to redux?
Personally, I think it's more confusing to have an API change that makes tutorials inconsistent.

Right now you can just google currying or memorize that middleware starts with const middleware = store => next => action => {...}. Sorting out different versions of tutorials is much more confusing.

@acstll
Copy link

acstll commented May 18, 2016

I have played with this before (#651) too so I'm gonna share my 2 cents.

To avoid changing the signature but also allowing beginners (or people with a "personal dislike" for currying ;-)) to write simple middleware functions, you could use lodash curry (or something similar) to wrap every middleware function. (I haven't tested this now, but I remember doing it some time ago).

Sort of like:

import curry from 'lodash/curry'

// .. then

chain = middlewares.map(middleware => curry(middleware)(middlewareAPI))

(you could also check function arity (length) to wrap it or not, but I think wrapping anyways won't hurt)

Then users could keep authoring middleware with the current signature or just do

function myMiddleware (store, next, action) {
  // ..
}

Note this is practically the same as just wrapping it yourself when authoring, like:

import curry from 'lodash/curry'

function myMiddleware (store, next, action) {
  // ..
}

export default curry(myMiddleware)

@Silverwolf90
Copy link

On a pragmatic level, I think this change is not at all worth the requirement of migration.

On a more abstract level, the middleware signature feels consistent in the "spirit" of the library. It's just a curried function; there's nothing special or "obnoxious" about it. It may be confusing to beginners with no exposure to FP-style JS, but that's no different than people trying to learn all the popular JS libraries at once. Once thing at a time. It's up to the docs to point people in the right direction if they don't have the necessary information.

@gaearon
Copy link
Contributor Author

gaearon commented May 19, 2016

OK, seems like this is controversial so not worth it.

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

No branches or pull requests

8 participants