redux-toolkit icon indicating copy to clipboard operation
redux-toolkit copied to clipboard

Listeners cannot be immediately triggered upon addition

Open markerikson opened this issue 2 years ago • 3 comments

Listeners always run in response to a dispatched action, but this means there's no way to trigger one immediately. (Imagine a longer-running workflow, where we don't even care about what the triggering action might be, but just want to start running and doing other work as soon as the listener is defined.)

Perhaps we could add a runImmediately: boolean option to the startListening options? In that case you'd probably get either no action passed into the listener, or some kind of a dummy action.

The alternative is to write your listener in such a way that you end up dispatching an action just to trigger it. Which is a valid thing to do, but also annoying.

Example of that from https://codesandbox.io/s/listen-for-condition-1b1b6g?file=/src/index.ts :


const listenForCondition = <State>(
  actionType: string,
  predicate: AnyListenerPredicate<State>,
  timeout?: number
): ThunkAction<
  ReturnType<ConditionFunction<State>>,
  State,
  unknown,
  AnyAction
> => (dispatch) =>
  new Promise<boolean>((resolve, reject) => {
    dispatch(
      addListener({
        type: actionType,
        effect: (_, { condition, unsubscribe }) =>
          condition(predicate as AnyListenerPredicate<unknown>, timeout)
            .then(resolve)
            .catch(reject)
            .finally(unsubscribe),
      })
    );
    dispatch({ type: actionType });
  });

markerikson avatar Jun 21 '22 21:06 markerikson

Can't we expose a simple listenForCondition or something similar in reduxjs/toolkit instead?

I am afraid that this change is gong to complicate too much the TS types and the internal implementation.

FaberVitale avatar Jun 22 '22 10:06 FaberVitale

Let's skip this for now. Can always revisit later.

markerikson avatar Jul 03 '22 02:07 markerikson

FWIW I'm actually about to use this util in Replay :) This seems to have value, and we ought to at least add it to the docs. Reopening the issue to remind myself of that.

markerikson avatar Jul 14 '22 19:07 markerikson

Related issue: #2698 . Lenz had a suggestion over there:

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 avatar Jan 31 '23 18:01 markerikson

for what it's worth, i've come to the realisation that listenForCondition could be implemented without the need for the trigger:

/**
 * Attach a one-shot listener to the store, and resolves to `true` when `predicate` matches (or `false` if given `timeout` expires), then unsubscribes itself
 * @param predicate Condition to match - receives action, state, and originalState (before action was dispatched)
 * @param timeout Time in ms to wait before resolving promise to `false`.
 * @returns Promise to wait on.
 */
export const listenForCondition = <State>(
  predicate: AnyListenerPredicate<State>,
  timeout?: number
): ThunkAction<Promise<boolean>, State, unknown, AnyAction> => (dispatch) =>
  new Promise<boolean>((resolve) => {
    let timeoutId: ReturnType<typeof setTimeout> | undefined;
    const unsub = dispatch(
      (addListener as TypedAddListener<State>)({
        predicate,
        effect: (action, { unsubscribe }) => {
          unsubscribe();
          resolve(true);
          if (typeof timeoutId !== undefined) {
            clearTimeout(timeoutId);
          }
        },
      })
    );
    if (timeout) {
      timeoutId = setTimeout(() => {
        resolve(false);
        unsub();
      }, timeout);
    }
  });

EskiMojo14 avatar Mar 02 '23 11:03 EskiMojo14

Are you open to a PR that makes this clearer in the docs? I just spent a few hours down a rabbit hole wondering why nothing was working until I came to realise that .condition() and .take() only resolve after the next action is dispatched. Maybe adding a little note to the docs would make this clearer for future folks.

chmac avatar Apr 07 '23 15:04 chmac

@chmac yeah, docs PRs are always welcome!

Out of curiosity, what were you trying to accomplish, and what did you want to have happen? What does your code look like?

markerikson avatar Apr 07 '23 16:04 markerikson

@markerikson I'm loading encrypted files from disk. First, I need to load the decryption key into redux. Then I have a startup routine that reads files, decrypts them, puts their contents into redux. So I have a bunch of startup processes which all need to "wait" for the key to become available. My code is something like:

startAppListening({
  actionCreator: startup,
  effect: async (_, listenerApi) => {
    await listenerApi.condition((_, state) => typeof state.keys.privateKey === 'string')
    // do other stuff
  }
});

My expectation was that listenerApi.condition() would immediately resolve to true, or wait indefinitely until the predicate returned true, and then resolve. Took me a bit of head scratching to figure out that the .condition() was waiting for more actions. I found out by accident when I created a "dispatch nonsense action" button to check if my flipper config was working and suddenly my events were going! :-)

I'll work up a little PR to add a small note to the docs.

chmac avatar Apr 07 '23 16:04 chmac