Why are listeners notified before middleware is finished?

[georeith]

I have a bug in my application that I am trying to work out how to fix and the issue goes as follows:

• An action is triggered
• I have middleware which records state before and after the action is fired (either side of next(action))
• I have some React components with a useLayoutEffect() hook that sometimes dispatch actions
• Some actions seem to trigger immediate renders when calling next() in middleware, which triggers an immediate dispatch and the middleware is not getting the state after the result of the action but its getting the state after the result of the 2nd action, that is the dispatch from useLayoutEffect is occurring in the middle of next(action) and not after the middleware has run.

I am trying to reproduce this in a simplified test case and having a hard time in doing so, what I have determined is that for some actions my application renders immediately when listener() is invoked inside dispatch and for other actions the re-render occurs after the listener and I haven't yet worked out how to reproduce the first case where the re-render is immediate despite trying here: https://codesandbox.io/s/immutable-architecture-2535n?file=/src/App.tsx

I can't seem to see any difference in the next(action) call of either action:

Regardless my question is why are listeners notified at all until after middleware is finished? Otherwise isn't there always the risk that a listener dispatches another action and the middleware is unable to act on the finished state of the first action?

https://github.com/reduxjs/redux/blob/b5d07e04364da74cc348eb8398185b304dc3ceb9/src/createStore.ts#L251-L255

[timdorr]

When calling next, it should be treated like calling dispatch. That is, you don't know if it's dispatch that's next or not, but you can assume it will behave like calling it. dispatch is a function that executes synchronously, including calling its subscribers. We have no way of knowing if a middleware is "done" or even exists outside of that function, so we can't delay any execution of code (not that we would anyways; this violates how Redux is supposed to behave).

My guess is your use of useLayoutEffect is causing the issue. Unless you have a very specific purpose to use that, you should be using useEffect to ensure the timing is correct. You shouldn't be dispatching actions right as things render, because that's almost guaranteed to make that render a wasted effort.

[georeith]

I do have a very specific use case and not using useLayoutEffect is not an alternative. I am using layout information and need to trigger a re-render because of it synchronously. That is not the issue.

As it stands there is no way to guarantee you are getting the state for that action after next(action) is called in middleware and that is bad.

I don't really feel like my question was answered. Why does the code after next() in my synchronous middleware not run before invoking listeners to prevent this issue? Or rather why would I want other side effects to be allowed to happen before middleware has had a chance to engage with the state?

@timdorr You absolutely can tell when middleware is done, I am not talking about asynchronously queued events I'm talking about waiting for a synchronous function to complete. Unless JS is no longer event loop driven this is predictable.

[markerikson]

@georeith : middleware + dispatch form a literal call stack. If I have 3 middleware, a->b->c->dispatch, the execution sequence will be:

• begin a
  • begin b
  • begin c
    • store.dispatch
    • reducer(state, action)
    • notify listeners
      • subscriber1()
      • useSelector checks, etc
      • subscriber2()
      • useSelector checks, etc
  • unwind c
  • unwind b
• unwind a

All of this is entirely synchronous by default.

So, it's literally impossible for, say, "unwind b to occur before any of the subscribers are notified, because the subscription notification happens inside of store.dispatch, and middleware are a wrapper around store.dispatch.

[georeith]

@markerikson I understand the current way the code is written you can't reorder it without some kind of refactoring, but I wouldn't say it's impossible to refactor into a way it doesn't.

Whatever calls A could notify instead after the function completes.

My question is there a design decision for calling listeners first which then means you are not guaranteed to access the correct state in middleware?

For context, I am building a design tool, I can insert text and the size of that text is non-deterministic when its in auto sizing mode, I need layout to run and the browser to tell me the size it has given it.

I also have other UI that needs to size itself based on this text so I useLayoutEffect to update the state with the size of the text, this has to be synchronous.

This design tool also allows real time collaboration, each action you fire stores the change it makes and middleware sends those changes up to the server. If that middleware can not access the state before another action is dispatched by a subscriber to the initial I am unable to access the original change.

My question is that I am struggling to understand any use case where you'd want listeners to be notified before middleware is complete and is not what I would consider true middleware, not using useLayoutEffect isn't a solution. I would like to understand at least before I find another state management method why this one opts for that behaviour.

[markerikson]

is there a design decision for calling listeners first which then means you are not guaranteed to access the correct state in middleware?

Yes, in the sense that "that is how the code currently works" - ie, it's a fallout of the design properties and semantics that were chosen for listeners, which just happens to mean that a middleware might not be able to see these intermediate states you're describing.

Remember that a Redux store has to work on its own without any middleware. So, the entire dispatch+listeners cycle has to be complete and standalone, even if there are no middleware involved.

Middleware add on and wrap around the Redux store. So, they are external to the entire internal dispatching process, and thus can never see any intermediate steps in the middle of dispatching and notifying listeners.

Generally, it seems like you have a very specific edge case, and it may be that the properties and behavior of the Redux store are simply not a match for your specific use case.

It's hypothetically possible that you might be able to write a custom store enhancer that overrides the dispatching behavior to be more suitable for your use case, but I don't immediately know what the relevant logic for that would look like.

[markerikson]

Hmm. My other thought here is that in theory, a dispatch while in a subscription callback should end up with two copies of middlewareA in the call stack. Since a middleware is a nested closure, I suppose you might be able to add a flag in the outermost closure to track if you are "currently dispatching" or not, and get the "intermediate" state then? I really haven't ever tried to dispatch from a subscription callback, so I haven't played with that scenario, but my understanding of the data flow says that might work.

Something like:

const myMiddleware = storeApi => {
  let isDispatching = false;
  let intermediateState = null;

  return next => action => {
    if (isDispatching) {
      intermediateState = getState()
    } else {
      isDispatching = true
    }

    const result = next(action);
    isDispatching = false;

    // do something with intermediateState somewhere in here, maybe?
    return result
  }
}

[georeith]

@markerikson thanks for your response and trying to find a workaround, agreed it is an edge case, as in I don't think this would break many apps we just have hit an unlucky series of requirements that have triggered it, as @timdorr said initially most people won't even be doing synchronous dispatches from listeners.

What I am wondering around the "design decision" aspect is that what you said here:

Yes, in the sense that "that is how the code currently works" - ie, it's a fallout of the design properties and semantics that were chosen for listeners, which just happens to mean that a middleware might not be able to see these intermediate states you're describing.

Remember that a Redux store has to work on its own without any middleware. So, the entire dispatch+listeners cycle has to be complete and standalone, even if there are no middleware involved.

I totally appreciate that, I guess what I'm asking is would you be opposed to it working the other way if I offered up a PR which changes the order and doesn't compromise Redux's simple case (no middleware) in doing so. Or would it be rejected because you don't want Redux to work in that fashion at all / are depending on the current behaviour for some reason or another?

I appreciate its an edge case and not high on your priority and wouldn't expect a fix from yourselves but this is a pretty big stick in the mud for my use case and Redux works well otherwise.

Hmm. My other thought here is that in theory, a dispatch while in a subscription callback should end up with two copies of middlewareA in the call stack

That is the case you do end up with two 👍

[markerikson]

I'm saying that off the top of my head, I don't see any possible way to modify Redux's logic to get the kind of behavior you seem to be describing.

Or would it be rejected because you don't want Redux to work in that fashion at all / are depending on the current behaviour for some reason or another?

One of the things I've learned as a maintainer is Hyrum's Law, which states "With a sufficient number of users of an API, it does not matter what you promise in the contract: all observable behaviors of your system will be depended on by somebody.". This totally applies to everything we've done with Redux :)

Admittedly, I don't think we have much in the way of tests for some of the subscriber-related semantics (but I also haven't looked at the core lib tests in a while).

If you want to fiddle with it and can come up with something that might actually resolve your use case, we can at least discuss it, but I can't guarantee we'll accept any changes.

But, at the same time, this is part of the reason why store enhancers exist - to let you override core store functionality like subscriber notifications, and replace it with your own custom logic. So, at a minimum you should probably see if you can get this working as an enhancer first. (Of course, I will admit that our likely response here is "see, you just proved you can get this working in userland, no core code changes needed" :) )

[georeith]

@markerikson thanks as long as its open to discussion that works for me. I appreciate the fine line you have to tread to keep everyone happy and was why I made this issue to determine whether it was intentional or not and can go from there.

Also appreciate regardless of the original intent it could be seen as a breaking change because someone may be depending on the inverse (I can't think of the use case but I'm sure they wouldn't think of mine off the cuff either).

I will take a stab at both approaches and report back.

[markerikson]

Sure. If you can manage to come up with any additional unit tests that capture the exact flow of the current behavior, we'd be interested in those even if we don't end up accepting any changes to the core logic, so we at least have something that would visibly indicate that hypothetical future logic changes are actually changing that flow.

Also note that the current master branch is an as-yet-unreleased TS conversion of the library. You may want to pick up with the 4.x branch, which is still plain JS. Code can get ported either way, of course.