Fix race condition when calling mutate synchronously

[shuding]

Bug

We have this inconsistency issue in current version:

// we call `mutate` twice, synchronously, with a callback that reads the latest value
// let's assume the initial data is `0`

mutate('key', v => v + 1, false)
mutate('key', v => v + 1, false)

The data should be 2 after these 2 synchronous mutation calls. However if you test, it is actually 1 (here's a great bug reproduction from @derindutz).

The cause is that we are always updating the cache in async (as #619 tries to fix), so the argument v passed to the second mutation callback is still 0. To make it clear, here's a diagram addressing this bug:

Solution

We at first tried to make the mutate process synchronous (if the callback is not a promise or async function), but that brings another issue which is intermediate state:

mutate('key', v => v + 1, false)
mutate('key', v => v + 1, false)

Now it correctly renders 2 at the end, but there's a moment that 1 was rendered before that. Instead we should "batch"
synchronous mutations. It is because that the first mutation can't get deduplicated by the second one anymore. They both render the data.

To ensure the correctness of mutate, we need to make state broadcasting always asynchronous (kinda like React), but cache needs to be updated ASAP. So the latter mutations will always have the result from the former, but only the last mutation will get rendered:

The consequence of this fix is, if you mutate something and immediately check the result, it won't be updated. So in our test I added await 0 to delay the check in the next tick:

mutate('mutate-3', 'off', false)
await 0 // if no `await` here, it's gonna be wrong
expect(container.textContent).toMatchInlineSnapshot(`"off"`)

Fixes #566. Related to #619.

[shuding]

Great feedback @huozhi, thanks!

[huozhi]

LGTM! awesome work and explanation there 👍

[promer94]

we can keep the previous 'else if' flow here instead of reassign _data here ?

if (_data && typeof _data === 'function') {
    // `_data` is a function, call it passing current cache value
    try {
      data = _data(cache.get(key))
    } catch (err) {
      error = err
    }
  } else if (_data && typeof _data.then === 'function') {
    // `_data` is a promise
    isAsyncMutation = true
    try {
      data = await _data
    } catch (err) {
      error = err
    }
  } else {
    data = _data
  }

[shuding]

We can’t. If _data is an async function, calling it without await will get a promise returned, and we have to resolve it again.

[promer94]

On i see now 😅. I think i can add a test for mutate with async function. we don't have one now.

[shuding]

@promer94 that would be great, thanks!

[shuding]

Tested a bit, should be fine to ship this :)