fix: Should always reset to the original data after mutating with opt…

…imistic data (#1982)

* fix _o bug

* fix race conditions

_internal/utils/mutate.ts

@@ -42,8 +42,8 @@ export const internalMutate = async <Data>(
   const [get, set] = createCacheHelper<
     Data,
     State<Data, any> & {
-      // The original data.
-      _o?: Data
+      // The previously committed data.
+      _c?: Data
     }
   >(cache, key)
   const [EVENT_REVALIDATORS, MUTATION, FETCH] = SWRGlobalState.get(
@@ -80,21 +80,27 @@ export const internalMutate = async <Data>(
 
   const hasOptimisticData = !isUndefined(optimisticData)
   const state = get()
-  const currentData = state.data
-  const originalData = isUndefined(state._o) ? currentData : state._o
+
+  // `displayedData` is the current value on screen. It could be the optimistic value
+  // that is going to be overridden by a `committedData`, or get reverted back.
+  // `committedData` is the validated value that comes from a fetch or mutation.
+  const displayedData = state.data
+  const committedData = isUndefined(state._c) ? displayedData : state._c
 
   // Do optimistic data update.
   if (hasOptimisticData) {
     optimisticData = isFunction(optimisticData)
-      ? optimisticData(originalData)
+      ? optimisticData(committedData)
       : optimisticData
-    set({ data: optimisticData, _o: originalData })
+
+    // When we set optimistic data, backup the current committedData data in `_c`.
+    set({ data: optimisticData, _c: committedData })
   }
 
   if (isFunction(data)) {
     // `data` is a function, call it passing current cache value.
     try {
-      data = (data as MutatorCallback<Data>)(originalData)
+      data = (data as MutatorCallback<Data>)(committedData)
     } catch (err) {
       // If it throws an error synchronously, we shouldn't update the cache.
       error = err
@@ -119,8 +125,10 @@ export const internalMutate = async <Data>(
       // Rollback. Always populate the cache in this case but without
       // transforming the data.
       populateCache = true
-      data = originalData
-      set({ data: originalData })
+      data = committedData
+
+      // Reset data to be the latest committed data, and clear the `_c` value.
+      set({ data, _c: UNDEFINED })
     }
   }
 
@@ -129,15 +137,15 @@ export const internalMutate = async <Data>(
     if (!error) {
       // Transform the result into data.
       if (isFunction(populateCache)) {
-        data = populateCache(data, originalData)
+        data = populateCache(data, committedData)
       }
 
       // Only update cached data if there's no error. Data can be `undefined` here.
-      set({ data })
+      set({ data, _c: UNDEFINED })
     }
 
-    // Always update or reset the error and original data field.
-    set({ error, _o: UNDEFINED })
+    // Always update error and original data here.
+    set({ error })
   }
 
   // Reset the timestamp to mark the mutation has ended.
@@ -146,6 +154,10 @@ export const internalMutate = async <Data>(
   // Update existing SWR Hooks' internal states:
   const res = await startRevalidate()
 
+  // The mutation and revalidation are ended, we can clear it since the data is
+  // not an optimistic value anymore.
+  set({ _c: UNDEFINED })
+
   // Throw error or return data
   if (error) throw error
   return populateCache ? res : data

mutation/index.ts

@@ -61,6 +61,7 @@ const mutation = (<Data, Error>() =>
         try {
           const data = await mutate<Data>(
             serializedKey,
+            // FIXME: Error shouldn't be broadcasted here.
             (fetcher as any)(resolvedKey, { arg }),
             options
           )

test/use-swr-local-mutation.test.tsx

@@ -1229,6 +1229,120 @@ describe('useSWR - local mutation', () => {
     expect(previousValue2).toBe(0)
   })
 
+  it('should rollback to the original value after multiple mutations', async () => {
+    const key = createKey()
+    const renderedData = []
+    let mutate
+    let serverData = 'foo'
+
+    function Page() {
+      const { data, mutate: boundMutate } = useSWR(key, () =>
+        createResponse(serverData, { delay: 20 })
+      )
+      mutate = boundMutate
+      if (
+        !renderedData.length ||
+        renderedData[renderedData.length - 1] !== data
+      ) {
+        renderedData.push(data)
+      }
+      return <div>data: {String(data)}</div>
+    }
+
+    // data == "foo"
+    renderWithConfig(<Page />)
+    await screen.findByText('data: foo')
+
+    // data == "bar"
+    await executeWithoutBatching(async () => {
+      await mutate(
+        createResponse('bar', { delay: 20 }).then(r => (serverData = r)),
+        {
+          optimisticData: 'bar',
+          populateCache: false
+        }
+      )
+    })
+
+    try {
+      // data == "baz", then reverted back to "bar"
+      await executeWithoutBatching(() =>
+        mutate(createResponse(new Error(), { delay: 20 }), {
+          optimisticData: 'baz',
+          revalidate: false
+        })
+      )
+    } catch (_) {
+      // Ignore
+    }
+
+    await sleep(30)
+    expect(renderedData).toEqual([undefined, 'foo', 'bar', 'baz', 'bar'])
+  })
+
+  it('should rollback to the original value after multiple mutations (2)', async () => {
+    const key = createKey()
+    const renderedData = []
+    let mutate
+    let serverData = 'foo'
+
+    function Page() {
+      const { data, mutate: boundMutate } = useSWR(key, () =>
+        createResponse(serverData, { delay: 20 })
+      )
+      mutate = boundMutate
+      if (
+        !renderedData.length ||
+        renderedData[renderedData.length - 1] !== data
+      ) {
+        renderedData.push(data)
+      }
+      return <div>data: {String(data)}</div>
+    }
+
+    // data == "foo"
+    renderWithConfig(<Page />)
+    await screen.findByText('data: foo')
+
+    // Here m1 and m2 have overlap and m1 will be discarded.
+    await executeWithoutBatching(async () => {
+      const m1 = mutate(
+        createResponse('bar', { delay: 30 }).then(r => (serverData = r)),
+        {
+          optimisticData: 'bar',
+          populateCache: false
+        }
+      )
+
+      await sleep(10)
+
+      const m2 = mutate(
+        createResponse('baz', { delay: 30 }).then(r => (serverData = r))
+      )
+
+      await m1
+      await m2
+    })
+
+    try {
+      // data == "qux", then reverted back to "baz"
+      await executeWithoutBatching(() =>
+        mutate(createResponse(new Error(), { delay: 20 }), {
+          optimisticData: 'qux',
+          revalidate: false
+        })
+      )
+    } catch (_) {
+      // Ignore
+    }
+
+    // data: "foo" -> "bar" -> "baz" -> "qux" -> "baz"
+    //                 ^ optimistic      ^ error
+
+    await sleep(30)
+    expect(renderedData).toEqual([undefined, 'foo', 'bar', 'baz', 'qux', 'baz'])
+  })
+
   it('should not rollback optimistic updates if `rollbackOnError`', async () => {
     const key = createKey()
     const renderedData = []