"enhancer" is actually a middleware, not an enhancer

[hedgepigdaniel]

I think there is a misunderstanding in Reductive about what a Redux enhancer is. From the redux docs:

• A middleware is a function from getState (returns the current state of the store), dispatch (dispatches an action to the store), and next (dispatches an action to the next middleware in the chain). A middleware intercepts actions dispatched to the store and can block, change, or delay them or dispatch other actions as a result.
• An enhancer is a function that composes a storeCreator to return a new storeCreator. Enhancers wrap the entire API of the store.

Everything that a middleware can do can be done by an enhancer (as evidenced by the fact that the applyMiddlewares function from redux returns an enhancer), but the opposite is not true! Some things that enhancers can do that middlewares cannot:

• Change the initial state of the store without dispatching any actions.
• Change the reducer before the store is created
• Dispatch actions in response to external events (e.g. navigation actions), without waiting for any other actions to be dispatched first.

A middleware cannot do these things because it is only called when an action is dispatched to the store. An enhancer wraps the storeCreator, so it can change the state before the store is created and dispatch actions before other actions are dispatched.

I propose the following alternative:

• The enhancer argument to Store.create be removed
• The docs be updated to say that an enhancer is a function from a storeCreator to a storeCreator (many of which can be composed into another store enhancer)
  • An applyMiddleware function, that returns an enhancer that applies the (possibly composed) middleware that is passed to it.

This way it is clear how a store enhancer in the Redux sense can be applied, and the user controls the order in which enhancers and middlewares are applied.

Some types:

type store('action, 'state) = Reductive.Store.t('action, 'state);

type dispatch('action, 'state) = store('action, 'state) => 'action => unit;

type getState('action, 'state) = store('action, 'state) => 'state;

type reducer('action, 'state) = 'state => 'action => 'state;

type middleware('action, 'state) = store('action, 'state) => ('action => unit) => 'action => unit;

type storeCreator('action, 'state) = reducer('action, 'state) => 'state => store('action, 'state);

type enhancer('action, 'state) = storeCreator('action, 'state) => storeCreator('action, 'state);

type applyMiddleware('action, 'state) = middleware('action, 'state) => enhancer('action, 'state);

[ambientlight]

@rickyvetter, @hedgepigdaniel: It would be great to have some progress on this.

I personally don't think enhancer labeled argument need to be removed, we can just rename it to middleware. We don't need applyMiddleware function in this way, since reductive store's customDispatcher does what applyMiddleware when we chain the middleware with @@. It is great to have middleware available out of the box in reductive as it is now.

We can then have those types mentioned above available in reductive:

type store('action, 'state) = Store.t('action, 'state);
type reducer('action, 'state) = ('state, 'action) => 'state;

type middleware('action, 'state) =
  (store('action, 'state), 'action => unit, 'action) => unit;

type storeCreator('action, 'state) =
  (
    ~reducer: reducer('action, 'state),
    ~preloadedState: 'state,
    ~middleware: middleware('action, 'state)=?,
    unit
  ) =>
  store('action, 'state);

type storeEnhancer('action, 'state) =
  storeCreator('action, 'state) => storeCreator('action, 'state);

so we can define store enhancers with:

let logIt: storeEnhancer('action, 'state) = (storeCreator: storeCreator('action, 'state)) => (~reducer, ~preloadedState, ~middleware=?, ()) => {
  let someEffect = anything => Js.log(anything)

  let store = {
    ...storeCreator(~reducer, ~preloadedState, ~middleware?, ()),
    customDispatcher: Some((store, next, action) => {
      action |> someEffect;
      next(action);
      Store.getState(store) |> someEffect;  
    })
  };

  Store.getState(store) |> someEffect;
  store
};

and apply in the following way:

let storeCreator = Reductive.Enhancers.logIt @@ Reductive.Store.create;
let store = storeCreator(
  ~reducer=appReducer,
  ~preloadedState={counter: 0, content: ""},
  (),
);

Those renaming enhancer to middleware will result in breaking change. Is there anything like @ocaml.deprecated to show the warning for a deprecated parameter? Maybe we can just add another labeled parameter middleware and mark enhancer as deprecated? Alternatively we have two function for creating a store (let's say Reductive.Store.new with middleware labeled parameter) with current one Reductive.Store.create marked as deprecated so we avoid the breaking change. What do you think?

[rickyvetter]

Yeah I'm hesitant to do anything here because it would be a breaking change. I agree that enhancer probably isn't the best name because it doesn't match Redux, but it's still descriptive. @ambientlight makes good points that there really isn't a need for a dedicated applyMiddleware function since it's just function application. So we'd basically be making a breaking change from enhancer to middleware without actually using the enhancer name anywhere. It seems wasteful to introduce this churn without good reason.

[ambientlight]

@rickyvetter: so we can technically introduce another storeCreator function with middleware argument and mark Reductive.Store.create as deprecated. Calls will be highlighted with warning and we can keep the existing Reductive.Store.create infinitely long, just having the new storeCreator will remove this ambiguity. Or you think it's not worth it? Then maybe we should introduce the types above which will hint users about middleware and also mention it in docs to decrease this ambiguity?

[rickyvetter]

Definitely on board with mentioning in docs and adding types that might help clarify things.

As far as making a breaking change I'm more hesitant. Do you feel like there is enough confusion here to warrant a breaking change?

[ambientlight]

@rickyvetter: let's then maybe start with the pull requests with these types, docs and examples.

I would personally just attach deprecation notice that will emit warning to existing storeCreator and give an alternative storeCreator.

I am not super familiar, do we have enough editor/compiler support to give smart error messages here: the parameter was renamed?, then there will be less friction if we introduce the breaking change, and we might bump up the major version in this case so that CIs will not break (I assume mostly user's package.json will have ^1.0.0 form)