fix: do not abort atoms on unmount (#2627)

src/vanilla/store.ts

@@ -362,8 +362,12 @@ export const createStore = (): Store => {
       registerCancelPromise(promise, (next) => {
         if (next) {
           continuePromise(next as Promise<Awaited<Value>>)
+
+          // Only abort promises that aren't user-facing. When next is set,
+          // we can replace the current promise with the next one, so we don't
+          // see any abort-related errors.
+          abortPromise?.()
         }
-        abortPromise?.()
       })
       return setAtomValue(atom, promise as Value, nextDependencies, true)
     }

src/vanilla/store2.ts

@@ -89,9 +89,13 @@ const createContinuablePromise = <T>(
           continuablePromiseMap.set(nextPromise, p)
           curr = nextPromise
           nextPromise.then(onFulfilled(nextPromise), onRejected(nextPromise))
+
+          // Only abort promises that aren't user-facing. When nextPromise is set,
+          // we can replace the current promise with the next one, so we don't
+          // see any abort-related errors.
+          abort()
+          abort = nextAbort
         }
-        abort()
-        abort = nextAbort
       }
     })
     p.status = PENDING

tests/react/abortable.test.tsx

@@ -119,7 +119,7 @@ describe('abortable atom test', () => {
     expect(abortedCount).toBe(1)
   })
 
-  it('can abort on unmount', async () => {
+  it('does not abort on unmount', async () => {
     const countAtom = atom(0)
     let abortedCount = 0
     const resolve: (() => void)[] = []
@@ -169,7 +169,7 @@ describe('abortable atom test', () => {
     await findByText('hidden')
 
     resolve.splice(0).forEach((fn) => fn())
-    await waitFor(() => expect(abortedCount).toBe(1))
+    await waitFor(() => expect(abortedCount).toBe(0))
   })
 
   it('throws aborted error (like fetch)', async () => {

tests/vanilla/store.test.tsx

@@ -440,3 +440,114 @@ describe('async atom with subtle timing', () => {
     expect(await bValue).toBe(2)
   })
 })
+
+describe('aborting atoms', () => {
+  // We can't use signal.throwIfAborted as it is not available
+  // in earlier versions of TS that this is tested on.
+  const throwIfAborted = (signal: AbortSignal) => {
+    if (signal.aborted) {
+      throw new Error('aborted')
+    }
+  }
+
+  it('should abort the signal when dependencies change', async () => {
+    const a = atom(1)
+    const callBeforeAbort = vi.fn()
+    const callAfterAbort = vi.fn()
+    let resolve = () => {}
+
+    const store = createStore()
+
+    const derivedAtom = atom(async (get, { signal }) => {
+      const aVal = get(a)
+
+      await new Promise<void>((r) => (resolve = r))
+
+      callBeforeAbort()
+
+      throwIfAborted(signal)
+
+      callAfterAbort()
+
+      return aVal + 1
+    })
+
+    const promise = store.get(derivedAtom)
+    const firstResolve = resolve
+    store.set(a, 3)
+
+    firstResolve()
+    resolve()
+    expect(await promise).toEqual(4)
+
+    expect(callBeforeAbort).toHaveBeenCalledTimes(2)
+    expect(callAfterAbort).toHaveBeenCalledTimes(1)
+  })
+
+  it('should abort the signal when dependencies change and the atom is mounted', async () => {
+    const a = atom(1)
+    const callBeforeAbort = vi.fn()
+    const callAfterAbort = vi.fn()
+    let resolve = () => {}
+
+    const store = createStore()
+
+    const derivedAtom = atom(async (get, { signal }) => {
+      const aVal = get(a)
+
+      await new Promise<void>((r) => (resolve = r))
+
+      callBeforeAbort()
+
+      throwIfAborted(signal)
+
+      callAfterAbort()
+
+      return aVal + 1
+    })
+
+    store.sub(derivedAtom, () => {})
+    const firstResolve = resolve
+    store.set(a, 3)
+
+    firstResolve()
+    resolve()
+
+    await new Promise(setImmediate)
+
+    expect(callBeforeAbort).toHaveBeenCalledTimes(2)
+    expect(callAfterAbort).toHaveBeenCalledTimes(1)
+  })
+
+  it('should not abort the signal when unsubscribed', async () => {
+    const a = atom(1)
+    const callBeforeAbort = vi.fn()
+    const callAfterAbort = vi.fn()
+    let resolve = () => {}
+
+    const store = createStore()
+
+    const derivedAtom = atom(async (get, { signal }) => {
+      const aVal = get(a)
+
+      await new Promise<void>((r) => (resolve = r))
+
+      callBeforeAbort()
+
+      throwIfAborted(signal)
+
+      callAfterAbort()
+
+      return aVal + 1
+    })
+
+    const unsub = store.sub(derivedAtom, () => {})
+
+    unsub()
+    resolve()
+
+    expect(await store.get(derivedAtom)).toEqual(2)
+    expect(callBeforeAbort).toHaveBeenCalledTimes(1)
+    expect(callAfterAbort).toHaveBeenCalledTimes(1)
+  })
+})