Tighten applyMiddleware types

[Methuselah96]

---

name: "Tighten applyMiddleware types"
about: More correctly types applyMiddleware

PR Type

Does this PR add a new feature, or fix a bug?

Fixes types

Why should this PR be included?

The applyMiddleware enhancer should not be allowed to accept any arguments, since it is only guaranteed to accept preloadedState? as its second argument.

Also the createStore parameter should not be StoreCreator since it doesn't allow for enhancers. It should match the parameter type for StoreEnhancor.

Checklist

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Is there an existing issue for this PR?
  • Simple enough that it probably doesn't need an issue.
• Have the files been linted and formatted?
• Have the docs been updated to match the changes in the PR?
• Have the tests been updated to match the changes in the PR?
• Have you run the tests locally to confirm they pass?

Bug Fixes

What is the current behavior, and the steps to reproduce the issue?

It's implied that more than 2 arguments are accepted by this enhancer, when it should it only be called with 2 arguments.

What is the expected behavior?

TypeScript should produce an error if trying to pass more than the two defined arguments.

How does this PR fix the problem?

It tightens the arguments' types.

[netlify]

Deploy preview for redux-docs ready!

Built with commit febd0e1

https://deploy-preview-3766--redux-docs.netlify.app

[Methuselah96]

This was the only usage of StoreCreator within Redux. Do we still want to keep that type or can it be removed?

[timdorr]

Can we leave the args spread? There's historical reason for that in #2201.

[Methuselah96]

Wow, that's a lot of historical context to dig through. However, I still think we should get rid of the args spread.

I'll start with #1302 since that seems like the most authoritative reasoning. The first problem is that in the example given, the enhancers are applied in the wrong order and should actually be flipped to be like this instead:

const finalCreateStore = applyMiddleware(thunk, logger)(createStore)
const store = finalCreateStore(
  reducer,
  initialState,
  DevTools.instrument()
)

This will apply DevTools.instrument() first and the applyMiddleware() afterward which will make it work correctly (I reproduced the example not working here: https://codesandbox.io/embed/optimistic-torvalds-vl1qm?fontsize=14&hidenavigation=1&theme=dark). Do we really expect people to mix the two different styles of enhancers and be able to apply them in the right order?

The main issue I have is that enhancers are not expected to pass along an enhancer as their third argument. The above example is the only pattern that will work when mixing enhancer creation styles (i.e., applyMiddleware is used to create a finalCreateStore and then enhancers are passed into it as a third argument). If any other enhancer was substituted for applyMiddleware or if applyMiddleware was passed into a finalCreateStore created with a different enhancer it would not work. No user can depend on an enhancer passing along a third enhancer argument in its createStore function and there's no reason why applyMiddleware should support it. If we don't expect other enhancers to pass along a third enhancer argument, why do we expect applyMiddleware to do so?

I think the other issues mentioned actually bolster my claim. All of the issues that were linked to in the description of the issue you linked to (#2028, #2128, #2131, #2200) all agree with this change and would not have been created if this change were applied.

As a newcomer to the code I found the signature of applyMiddleware confusing, and it seems obvious that others have thought so as well, since it seems incorrect and/or unnecessary. I think it would help clear up confusion to have applyMiddleware just support the intended function signature for enhancers instead of supporting an arcane use case that only works in very few specific cases and relies on knowing implementation and historical details.

[markerikson]

Given that we're looking at doing this all in a v5.0 major anyway, I'd be fine with dropping whatever vague bits of support exist for that legacy syntax.

[timdorr]

Yeah, let's just do this.