mobx-utils icon indicating copy to clipboard operation
mobx-utils copied to clipboard

fromPromise sets pending value to undefined if oldPromise is also pending

Open laszlopandy opened this issue 2 years ago • 0 comments

I found a race in code using fromPromise. When rapidly sending new requests in quick succession, created with fromPromise, the case method will temporarily return undefined until the latest response returns. Based on the documentation, I would expect it to always return stale data and never return undefined.

Example scenario

@observable
private dataResponse = undefined;

private loadData() {
    this.dataResponse = fromPromise(fetch(...), this.dataResponse);
}

getDataForReactUI() {
    return this.dataResponse?.case({
        fulfilled: (result) => result,
        pending: (stale) => stale,
    });
}

The race appears when the fetch response is slow, and the user triggers another load quickly. The sequence is:

  1. loadData() called for the first time, and this.dataResponse is initialized.
  2. Response returns and this.data resolves. User sees the returned data.
  3. User clicks a refresh button.
  4. loadData() is called a second time; fromPromise() is called with the resolved promise as the second argument.
  5. User clicks the refresh button again before the previous request returns.
  6. loadData() is called a third time; fromPromise() is called with the pending promise as the second argument.

What happens:

User sees data disappear for a few seconds, before seeing the new data appear.

What I expect

User sees stale data until new data is available.

Root cause

The cause seems to be this code inside fromPromise(): https://github.com/mobxjs/mobx-utils/blob/v5.6.1/src/from-promise.ts#L185

    const oldData =
        oldPromise && (oldPromise as any).state === FULFILLED
            ? (oldPromise as any).value
            : undefined

In short, if the oldPromise is still PENDING, fromPromise() will use undefined even if the oldPromise has a value for its pending state.

laszlopandy avatar May 06 '22 16:05 laszlopandy