This could be neatly integrated into the main setTimeout callback several lines below. Just don't run the fulfill function when isFulfilled returns true:

const isFulfilled =
  typeof resolver.isFulfilled === 'function' &&
  resolver.isFulfilled( state, ...args );
if ( ! isFulfilled ) {
  const action = resolver.fulfill( ...args );
  if ( action ) {
    await store.dispatch( action );
  }
}

It's the same thing, without code duplication, with just one difference: both startResolution and finishResolution are called, right after each other without any waiting. But that's fine, it's even welcome, because outside observers will see the same sequence of updates, no matter how the selector/resolver is internally implemented.

Another way to fix this is in the mapResolveSelector and mapSuspendSelector functions. These functions could perform the isFulfilled check and return a result right away. In that case, the startResolution/finishResolution actions would never be called when isFulfilled is true. But it's probably a worse solution than the first one, again for consistency reasons: sometimes, for the same selector, start/finishResolution would be called, sometimes not.

I agree with calling startResolution/finishResolution and being consistent. I'll wrap the call to isFulfilled into the main setTimeout. Thank you for the review.

@jsnajdr this is odd. The error is caught and the state of the resolver transitions to failed, but Jest fails the test without this line. I'm still researching the issue.

I've walked through with the debugger, wow there is a lot going on for a single resolution. Though after running jest.runAllTimers, promise has a rejected status and Error: isFulfilled error, even though it was caught in the try/catch and included in the FAIL_RESOLUTION action.

OH I got it, mapResolveSelector rejects it. So the test is correct, if this is the desired behavior.

Should the resolver resolution fail if isFulfilled throws an error?

Yes, resolveSelect detects the error and returns it.

If isFulfilled throws, that is certainly suprising and unexpected and the error should be surfaced. So yes, the resolution should fail.

This could be inlined into the isFulfilled call, we don't use the state anywhere else do we?

resolver.isFulfilled( store.getState(), ...args )

For the resolveSelect tests, we would be better off if we used jest.useRealTimers(). Then you don't need to run timers manually.

It will happen automatically if you move these tests to redux-store/test/index.js, where all other resolveSelect tests are. This registry.js suite has jest.useFakeTimers() at the start.

Yes, resolveSelect detects the error and returns it.

If isFulfilled throws, that is certainly suprising and unexpected and the error should be surfaced. So yes, the resolution should fail.