diff --git a/.changeset/olive-cows-repeat.md b/.changeset/olive-cows-repeat.md new file mode 100644 index 00000000000..c34e9cafe36 --- /dev/null +++ b/.changeset/olive-cows-repeat.md @@ -0,0 +1,5 @@ +--- +'@tanstack/query-core': patch +--- + +Ignore a retained `pendingThenable` settlement callback invoked after the thenable has settled. Holding a reference to `resolve`/`reject` and calling it later used to overwrite `status` and `reason` even though the underlying promise had already settled, leaving the thenable advertising a state that disagreed with its value. diff --git a/packages/query-core/src/__tests__/thenable.test.tsx b/packages/query-core/src/__tests__/thenable.test.tsx new file mode 100644 index 00000000000..5540ea58cd1 --- /dev/null +++ b/packages/query-core/src/__tests__/thenable.test.tsx @@ -0,0 +1,126 @@ +import { describe, expect, it, vi } from 'vitest' +import { pendingThenable, tryResolveSync } from '../thenable' +import type { FulfilledThenable, RejectedThenable } from '../thenable' + +describe('pendingThenable', () => { + it('should start out pending with resolve and reject attached', () => { + const thenable = pendingThenable() + + expect(thenable.status).toBe('pending') + expect(typeof thenable.resolve).toBe('function') + expect(typeof thenable.reject).toBe('function') + }) + + it('should expose the value and drop the pending props once resolved', async () => { + const thenable = pendingThenable() + + thenable.resolve('data') + + expect(thenable.status).toBe('fulfilled') + expect((thenable as unknown as FulfilledThenable).value).toBe('data') + expect(thenable.resolve).toBeUndefined() + expect(thenable.reject).toBeUndefined() + await expect(thenable).resolves.toBe('data') + }) + + it('should expose the reason and drop the pending props once rejected', async () => { + const thenable = pendingThenable() + const reason = new Error('error') + + thenable.reject(reason) + + expect(thenable.status).toBe('rejected') + expect((thenable as unknown as RejectedThenable).reason).toBe(reason) + expect(thenable.resolve).toBeUndefined() + expect(thenable.reject).toBeUndefined() + await expect(thenable).rejects.toBe(reason) + }) + + it('should not reject a second time after being resolved', async () => { + const thenable = pendingThenable() + + thenable.resolve('data') + // `reject` is deleted on finalize, so a late caller cannot settle it twice + expect(thenable.reject).toBeUndefined() + + await expect(thenable).resolves.toBe('data') + }) + + it('should ignore a retained settlement callback invoked after it settled', async () => { + const thenable = pendingThenable() + // a caller can hold on to `reject` before the thenable settles + const retainedReject = thenable.reject + + thenable.resolve('data') + retainedReject(new Error('error')) + + expect(thenable.status).toBe('fulfilled') + expect((thenable as unknown as FulfilledThenable).value).toBe('data') + expect((thenable as unknown as RejectedThenable).reason).toBeUndefined() + await expect(thenable).resolves.toBe('data') + }) + + it('should not report an unhandled rejection when nobody awaits it', async () => { + const onUnhandledRejection = vi.fn() + process.on('unhandledRejection', onUnhandledRejection) + + pendingThenable().reject(new Error('error')) + // unhandled rejections are reported after the microtask queue drains + await new Promise((resolve) => setTimeout(resolve, 0)) + + process.off('unhandledRejection', onUnhandledRejection) + expect(onUnhandledRejection).not.toHaveBeenCalled() + }) +}) + +describe('tryResolveSync', () => { + it('should return undefined for an already resolved native promise', () => { + // `then` on a native promise always defers to a microtask, so the data is + // never synchronously available + expect(tryResolveSync(Promise.resolve('data'))).toBeUndefined() + }) + + it('should return the data of a thenable that resolves synchronously', () => { + const thenable = { + then: (onFulfilled: (value: string) => unknown) => { + onFulfilled('data') + return Promise.resolve('data') + }, + } as unknown as Promise + + expect(tryResolveSync(thenable)).toEqual({ data: 'data' }) + }) + + it('should support a synchronous thenable whose then() has no catch', () => { + // a React thenable is not always a full promise, so `then` may return + // something without a `catch` method + const thenable = { + then: (onFulfilled: (value: string) => unknown) => { + onFulfilled('data') + }, + } as unknown as Promise + + expect(tryResolveSync(thenable)).toEqual({ data: 'data' }) + }) + + it('should return undefined when a synchronous thenable resolves with undefined', () => { + const thenable = { + then: (onFulfilled: (value: undefined) => unknown) => { + onFulfilled(undefined) + }, + } as unknown as Promise + + expect(tryResolveSync(thenable)).toBeUndefined() + }) + + it('should return undefined for a rejected promise without leaving it unhandled', async () => { + const onUnhandledRejection = vi.fn() + process.on('unhandledRejection', onUnhandledRejection) + + expect(tryResolveSync(Promise.reject(new Error('error')))).toBeUndefined() + await new Promise((resolve) => setTimeout(resolve, 0)) + + process.off('unhandledRejection', onUnhandledRejection) + expect(onUnhandledRejection).not.toHaveBeenCalled() + }) +}) diff --git a/packages/query-core/src/thenable.ts b/packages/query-core/src/thenable.ts index 4b9a1ddaa17..034666d079f 100644 --- a/packages/query-core/src/thenable.ts +++ b/packages/query-core/src/thenable.ts @@ -56,6 +56,13 @@ export function pendingThenable(): PendingThenable { }) function finalize(data: Fulfilled | Rejected) { + if ((thenable as Thenable).status !== 'pending') { + // a caller that kept a reference to `resolve`/`reject` can still invoke it + // after the promise settled, which the underlying promise ignores. Applying + // it here would leave the status disagreeing with the settled value. + return + } + Object.assign(thenable, data) // clear pending props to avoid calling them twice