Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Listeners cannot be immediately triggered upon addition #2435

Open
markerikson opened this issue Jun 21, 2022 · 8 comments
Open

Listeners cannot be immediately triggered upon addition #2435

markerikson opened this issue Jun 21, 2022 · 8 comments
Labels
Milestone

Comments

@markerikson
Copy link
Collaborator

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 });
  });
@FaberVitale
Copy link
Contributor

FaberVitale commented Jun 22, 2022

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.

@markerikson
Copy link
Collaborator Author

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

@markerikson markerikson closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2022
@markerikson
Copy link
Collaborator Author

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 markerikson reopened this Jul 14, 2022
@markerikson markerikson added this to the 1.9 milestone Jul 14, 2022
@markerikson markerikson removed this from the 1.9 milestone Aug 11, 2022
@markerikson
Copy link
Collaborator Author

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?

@EskiMojo14
Copy link
Collaborator

EskiMojo14 commented Mar 2, 2023

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);
    }
  });

@chmac
Copy link
Contributor

chmac commented Apr 7, 2023

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.

@markerikson
Copy link
Collaborator Author

@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?

@chmac
Copy link
Contributor

chmac commented Apr 7, 2023

@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 added a commit to chmac/redux-toolkit that referenced this issue Apr 7, 2023
@markerikson markerikson added this to the 1.9.x milestone Aug 16, 2023
silte pushed a commit to silte/redux-toolkit that referenced this issue Aug 21, 2023
@markerikson markerikson modified the milestones: 1.9.x, 2.x bugfixes Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants