listenerApi.condition not working as expected

[skavan]

Hi,

Forgive any poor terminology, I'm relatively new at redux.

I have created a test reducer (uselessReducer), a listenerMiddleware, and my general appMiddleware. uselessReducer has an action, updateStatus. the listenerMiddlewarehas an actionCreatoron updateStatus.

In my general appMiddleware, if the useless/init case is triggered, the updateStatus action is dispatched and my listener is triggered, its condition is met (connected: true) and it executes as expected. All good.

    return function (next) {
        return function (action) {
            switch (action.type) {
                case "useless/init":
                    store.dispatch(updateStatus({ connected: true, status: "connected" }));
                    return
                default:
                    break;
            }
            return next(action);
        };
    };

Here's the listener:

listenerMiddleware.startListening({
  actionCreator: updateStatus,
  effect: async (action, listenerApi) => {
        // Can cancel other running instances
        listenerApi.cancelActiveListeners()  
        console.log("LISTENER RUNNING")

        if (await listenerApi.condition((action, currentState) =>
          currentState.useless.connected === true
        )){
          console.log("LISTENER TRIGGERED")
        }

  }
})

However, if I change the appMiddleware and dispatch the action inside the .then(..), then the action is correctly dispatched, the listener is run, but the condition doesn't work and the following code is never executed. I must be missing something pretty fundamental, but can't figure out what! What am I missing here?

case "useless/init":
       axios.get("http://somewebsite.xxx/").then((res) => {
               store.dispatch(updateStatus({ connected: true, status: "connected" }));                    
       });
       return

[markerikson]

Can you post a CodeSandbox that shows the actual code, and possibly a Replay ( https://replay.io/record-bugs ) that shows this happening? I don't see enough to understand what the setup is here.

[skavan]

OK - I tried! here you go: https://codesandbox.io/s/react-redux-listener-op4l7v?file=/src/App.js what's weird is that this time, lines 27,28,29 in listenerMiddleware never get called. Implies to me something about timing. To flip between a sync workflow and async, toggle lines 18 and 19 in App.js.

This is probably just me not understanding how this should work. I would have thought, that since the condition (lines 23, 24 in listenerMiddleware) is true, it should trigger the following code.

[phryneas]

The problem is that that await listenerApi.condition( will test on every future action dispatch if that condition is true - but you are not dispatching any more actions.

It's a bit awkward, because at the point in time where you do the await condition, the condition is actually already true, so it feels like it should execute immediately. But of course that is problematic as well, because condition does not only test for state, but also for actions - so it would need an action. Since there is no action, it cannot run immediately.

I think we might want to still execute that condition immediately anyways - with some fake action like { type: 'RTK__immediateConditionCheck' }. @markerikson what do you think?

[markerikson]

I don't think I understand what the intent is here.

The point of await condition() is to wait and check after future actions, not "right now".

If you want to check what the state is now, you should use getState() and compare some value inside.

Can you clarify what you're trying to accomplish?

[phryneas]

I'd assume it's a "wait for this state to be the case. It might already be the case or happen in the future".

I guess a

if (getState().someCondition || await listenerApi.condition((action, state) => state.someCondition) {
  ...
}

just feels like unneccessary repetition for something like that - so I get the wish for this to work.

[skavan]

I'd assume it's a "wait for this state to be the case. It might already be the case or happen in the future".

@phryneas -- exactly correct!

[markerikson]

Yeah, the listener middleware's implementation is explicitly "run checks after each action is dispatched", and condition() is implemented internally as yet another listener instance. So, it won't check right away - you would have to do dispatch some action to trigger an initial check, or do the check manually yourself.

I do get that's not ideal, but we'd have to do some major rethinking of how this works otherwise.

[skavan]

fair enough. the conditional approach above will work...

[phryneas]

I do get that's not ideal, but we'd have to do some major rethinking of how this works otherwise.

See my suggestion above - essentially

             condition: (
               predicate: AnyListenerPredicate<any>,
               timeout?: number
-            ) => take(predicate, timeout).then(Boolean),
+            ) =>
+              predicate(
+                { type: 'RTK__immediateConditionCheck' },
+                api.getState(),
+                getOriginalState()
+              )
+                ? Promise.resolve(true)
+                : take(predicate, timeout).then(Boolean),
             take,

WDYT?

[skavan]

But -- does that force an immediate ONLY check (or no?). Your original form, was an either/or which is what we want the outcome to be.

[phryneas]

It's a suggested change in https://github.com/reduxjs/redux-toolkit/blob/a60e79d9dd87f393f16d991d6fab9623ef719954/packages/toolkit/src/listenerMiddleware/index.ts#L385-L388 - which would immediately do the check and if that fails go back to the current behaviour.

[skavan]

By the way -- I dunno if this was implied -- by

you would have to do dispatch some action to trigger an initial check

what I would really like to do is init a listener, without triggering a dispatch from the app side of things. i.e a kind of autorun. I'm too inexperienced to figure out how to do that!

[skavan]

• which would immediately do the check and if that fails go back to the current behaviour.

perfect. but if its too much brain damage, I'm ok with the below, as its pretty self explanatory:

if (getState().someCondition || await listenerApi.condition((action, state) => state.someCondition) { ... }

and whatever the soln. probably good idea to update docs to emphazise the future nature of condition so peeps like me don't get tied up in knots...

[markerikson]

We have had a couple cases where it would be nice to trigger a listener right away.

One conceptual issue: addListener.predicate accepts an action, as does condition. If we trigger it immediately, then there isn't an action to pass in as the first arg, so what would we do? If your predicate code happens to check that, and it's null instead, it'll explode. (Alternately, if we were to change the type to be action: AnyAction | null, now you have to double-check that in every condition...)

edit

Oh. I see Lenz's point - pass in a dummy action with a fake type field, so it at least fulfills the minimum expected contents of an action object.

That could work, actually...

[skavan]

Oh. I see Lenz's point - pass in a dummy action with a fake type field, so it at least fulfills the minimum expected contents of an action object.

That could work, actually...

edit: wrong quote! But wouldn't that still require a triggering event of some action or other in order to hit the routine at all?

[skavan]

We have had a couple cases where it would be nice to trigger a listener right away.

edit What we kinda want is "when store/reducers/middleware loaded, run this listener definition(s)"