diff --git a/src/mutation.ts b/src/mutation.ts index 949f2f5a10ba88c0bfe842d6a1e4121dbe7416e8..48745d618069b7842cb144c9da17017c4addf3c1 100644 --- a/src/mutation.ts +++ b/src/mutation.ts @@ -63,6 +63,12 @@ export interface MutationOptions< * All pending promises resolve with the final result. */ debounceMs?: number; + /** + * Called before and after optimistic updates to detect no-op mutations. + * If the snapshots are equal (using deepEquals), the mutation is cancelled. + * Only `onSettled` callbacks fire, not `onSuccess` or global handlers. + */ + snapshot?: (context: Config["context"] & { args: Args }) => unknown; } export type OptimisticContext< @@ -92,11 +98,13 @@ interface PendingDebouncedState { }>; /** Success callbacks from the most recent call */ onSuccess: Array<(result: Result) => void>; + /** Initial snapshot before any debounced calls (for no-op detection) */ + initialSnapshot?: unknown; } interface Channel { listeners: Set<(update: MutationEvent) => void>; - status: "idle" | "waiting" | "mutating" | "refetching"; + status: "idle" | "waiting" | "mutating" | "refetching" | "skipped"; rollbacks: Array<() => void>; refetches: Array<() => Promise>; queue: Array>; @@ -242,7 +250,7 @@ export class BlockingMutation< const suppressGlobalSuccess = onSuccess !== undefined; const suppressGlobalError = onError !== undefined; - const promise = this.#runAsPromiseWithOptions(args, { onRestore }); + const promise = this.#runWithOptions(args, { onRestore }); promise.then((result) => { // Call user handlers onSuccess?.(result); @@ -268,12 +276,7 @@ export class BlockingMutation< }); } - /** Calls the mutation, treating the errors as promise rejection. */ - runAsPromise(...args: Args): Promise { - return this.#runAsPromiseWithOptions(args, {}); - } - - #runAsPromiseWithOptions( + #runWithOptions( args: Args, { onRestore: userOnRestore }: Pick, "onRestore">, ): Promise { @@ -290,6 +293,11 @@ export class BlockingMutation< return this.#runDebouncedAndReturn(args, key, channel, userOnRestore, true); } + // Take snapshot before optimistic update (if snapshot function defined) + const beforeSnapshot = this.#options.snapshot + ? this.#options.snapshot.call(this.#client.context, { args }) + : undefined; + // Create shared optimistic helpers instance for the channel if it doesn't exist if (channel.helpers === null) { const onRefetch = (cb: () => Promise) => { @@ -357,6 +365,29 @@ export class BlockingMutation< } expired = true; + // Take snapshot after optimistic update and check for no-op + if (beforeSnapshot !== undefined) { + const afterSnapshot = this.#options.snapshot!.call( + this.#client.context, + { args }, + ); + const isNoOp = this.#client.deepEquals(beforeSnapshot, afterSnapshot); + + if (isNoOp) { + let next; + while ( + next = channel.rollbacks.splice(channel.rollbacks.length - rollbacks, 1)[0] + ) { + next(); + } + + // Notify listeners of skipped status + this.#notify(channel, "skipped"); + + return new Promise(() => {}); + } + } + const { promise, resolve, reject } = Promise.withResolvers(); channel.queue.push({ args, @@ -460,6 +491,13 @@ export class BlockingMutation< userOnRestore?: () => void, fromRunWithOptions = false, ): Promise { + // Capture initial snapshot before first debounced call + const isFirstDebouncedCall = channel.pendingDebounced === null; + let initialSnapshot: unknown; + if (isFirstDebouncedCall && this.#options.snapshot) { + initialSnapshot = this.#options.snapshot.call(this.#client.context, { args }); + } + // If there's a pending debounced call, roll it back if (channel.pendingDebounced) { this.#rollbackPendingDebounced(channel); @@ -533,6 +571,45 @@ export class BlockingMutation< } expired = true; + // Check for no-op by comparing to initial snapshot + if (this.#options.snapshot) { + const currentSnapshot = this.#options.snapshot.call( + this.#client.context, + { args }, + ); + const snapshotToCompare = isFirstDebouncedCall + ? initialSnapshot! + : channel.pendingDebounced?.initialSnapshot; + + if ( + snapshotToCompare !== undefined + && this.#client.deepEquals(snapshotToCompare, currentSnapshot) + ) { + // No-op detected - rollback optimistic update + let next; + while ( + next = channel.rollbacks.splice(channel.rollbacks.length - rollbacks, 1)[0] + ) { + next(); + } + + // Clear debounce timer + if (channel.debounceTimer !== null) { + clearTimeout(channel.debounceTimer); + channel.debounceTimer = null; + } + + // Clear pending debounced state + channel.pendingDebounced = null; + + // Notify listeners of skipped status + this.#notify(channel, "skipped"); + + // Return resolved promise + return new Promise(() => {}); + } + } + // Create promise for this call const { promise, resolve, reject } = Promise.withResolvers(); @@ -544,6 +621,7 @@ export class BlockingMutation< rollbackCount: rollbacks, pending: [{ resolve, reject }], onSuccess, + initialSnapshot: isFirstDebouncedCall ? initialSnapshot : undefined, }; // Set status to waiting @@ -555,6 +633,7 @@ export class BlockingMutation< channel.pendingDebounced.rollbackCount = rollbacks; channel.pendingDebounced.pending.push({ resolve, reject }); channel.pendingDebounced.onSuccess = onSuccess; + // Keep the initial snapshot from the first call // Status stays "waiting" } diff --git a/src/react.ts b/src/react.ts index d4c9dc778e19b92576d0cf6be026a3e3bc499db2..c324a2196c7a54b88484cfc2fabe1b8c0eea2b43 100644 --- a/src/react.ts +++ b/src/react.ts @@ -253,20 +253,13 @@ class Observer { || this.watched.has("error") || this.watched.has("errorMessage"); const watchesSuccess = this.watched.has("isSuccess") || this.watched.has("result"); - const promise = mutation.runAsPromise(...args) - .then((result) => { - if (!watchesSuccess && mutation.describeResult) { - const message = mutation.describeResult(args, result); - if (message && mutation.client.reportSuccess) { - mutation.client.reportSuccess(message); - } - } - }); - promise.catch((err) => { - if (!watchesError) { - mutation.client.reportError(formatFriendlyError(mutation.describe(...args), errMessage(err)), err); - } - }); + const promise = mutation.runWithOptions( + ...args, + { + onSuccess: watchesSuccess ? () => {} : undefined, + onError: watchesError ? () => {} : undefined, + } satisfies RunOptions, + ); return promise; } diff --git a/src/types.ts b/src/types.ts index 89f3d15415d8b5f01b84d8573dd76078a520ce52..572ee00743f0edc62166717f4f99b8db95565a80 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,8 +5,6 @@ export interface Mutation { run(...args: Args): void; /** Calls the mutation with custom handlers that can suppress global handlers. */ runWithOptions(...args: [...args: Args, options: RunOptions]): void; - /** Calling the mutation. Errors are thrown in the promise. */ - runAsPromise(...args: Args): Promise; /** Returns the concurrency key used for a given set of arguments */ key(args: Args): string; @@ -38,7 +36,7 @@ export interface RunOptions { } export interface MutationEvent { - status: "idle" | "waiting" | "mutating" | "refetching"; + status: "idle" | "waiting" | "mutating" | "refetching" | "skipped"; result: Result | null; error: unknown; } diff --git a/test/cases/runWithOptions.test.tsx b/test/cases/runWithOptions.test.tsx index df47c81b80d554f1ade0c2b0ff6690d3a4311fa4..74c8de639e2a683149be993394834d64b51fdbc9 100644 --- a/test/cases/runWithOptions.test.tsx +++ b/test/cases/runWithOptions.test.tsx @@ -16,10 +16,12 @@ test("runWithOptions should allow react hook to do local handling", async () => mutate: async () => { return (await s.next()).value; }, + optimistic: ({ onSuccess }) => { + onSuccess(() => {}); + }, + refetchOnSuccess: false, describe: "Test the action", describeResult: "Tested the action", - optimistic: () => {}, - refetchOnSuccess: false, }); let renders: Array<{ status: string; result: string | undefined }> = []; @@ -58,4 +60,9 @@ test("runWithOptions should allow react hook to do local handling", async () => assertEquals(errorMessages, []); assertEquals(renders, [{ status: "success", result: "ok" }]); renders = []; + await act(async () => { + vi.advanceTimersByTime(10000); + }); + assertEquals(renders, []); + renders = []; }); diff --git a/test/cases/snapshot.test.tsx b/test/cases/snapshot.test.tsx new file mode 100644 index 0000000000000000000000000000000000000000..a71627aabc99e2020f33e0b0f1f81d94101a9443 --- /dev/null +++ b/test/cases/snapshot.test.tsx @@ -0,0 +1,231 @@ +import { useMutate } from "@clo/react-mutation"; +import { assert, assertEquals } from "@std/assert"; +import { act, render, screen } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { test, vi } from "vitest"; +import { createTestMutationClient, IterableStream } from "../share.ts"; + +test("snapshot should skip no-op mutation", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const user = userEvent.setup({ delay: null }); + + const { client, successMessages, errorMessages } = createTestMutationClient(); + const s = new IterableStream(); + + let state = { value: "initial" }; + let failed = false; + const mutUpdate = client.define({ + mutate: async (newValue: string) => { + failed = true; + }, + optimistic: ({ args: [newValue] }) => { + state.value = newValue; + }, + snapshot: () => state.value, + describe: "Update value", + describeResult: "Updated value", + }); + + let renders: Array<{ status: string; isOptimisticData: boolean }> = []; + function TestComponent() { + const { run, status, isOptimisticData } = useMutate(mutUpdate); + renders.push({ status, isOptimisticData }); + return ( + + ); + } + + render(); + assertEquals(renders, [{ status: "idle", isOptimisticData: false }]); + renders = []; + + // Click to run mutation with same value (no-op) + await act(() => user.click(screen.getByTestId("btn"))); + vi.runAllTimers(); + + // Should skip the mutation and return to idle without calling API + assert(!failed); + assertEquals(state.value, "initial"); + assertEquals(renders, []); // nothing changed + assertEquals(successMessages, []); // nothing happened + assertEquals(errorMessages, []); +}); + +test("snapshot should allow mutation when value changes", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const user = userEvent.setup({ delay: null }); + + const { client, successMessages, errorMessages } = createTestMutationClient(); + const s = new IterableStream(); + + let state = { value: "initial" }; + + const mutUpdate = client.define({ + mutate: async (newValue: string) => { + return (await s.next()).value; + }, + optimistic: ({ args: [newValue] }) => { + state.value = newValue; + }, + snapshot: () => state.value, + describe: "Update value", + describeResult: "Updated value", + }); + + let renders: Array<{ status: string; isOptimisticData: boolean }> = []; + function TestComponent() { + const { run, status, isOptimisticData } = useMutate(mutUpdate); + renders.push({ status, isOptimisticData }); + return ( + + ); + } + + render(); + renders = []; + + // Click to run mutation with different value + await act(() => user.click(screen.getByTestId("btn"))); + assertEquals(state.value, "changed"); // Optimistic applied + assertEquals(renders, [{ status: "mutating", isOptimisticData: true }]); + renders = []; + + // Complete the mutation + await act(async () => { + s.push("ok"); + vi.advanceTimersByTime(100); + }); + + assertEquals(renders, [{ status: "success", isOptimisticData: false }]); + assertEquals(successMessages, ["Updated value"]); + assertEquals(errorMessages, []); +}); + +test("debounced snapshot should skip when final state equals initial", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const user = userEvent.setup({ delay: null }); + + const { client, successMessages, errorMessages } = createTestMutationClient(); + const s = new IterableStream(); + + let state = { value: "initial" }; + + const mutations: string[] = []; + const mutUpdate = client.define({ + mutate: async (newValue: string) => { + mutations.push(newValue); + }, + optimistic: ({ args: [newValue] }) => { + state.value = newValue; + }, + snapshot: () => state.value, + debounceMs: 500, + refetchOnSuccess: false, + describe: "Update value", + describeResult: "Updated value", + }); + + let renders: Array<{ status: string }> = []; + function TestComponent() { + const { run, status } = useMutate(mutUpdate); + renders.push({ status }); + return ( + <> + + + + ); + } + + render(); + assertEquals(renders, [{ status: "idle" }]); + renders = []; + + // First call - change value + await act(() => user.click(screen.getByTestId("a"))); + assertEquals(state.value, "changed"); + assertEquals(renders, []); // Debounced mutations don't show mutating, no update + assertEquals(mutations, []); + renders = []; + + // Second call - revert to initial (no-op overall) + await act(() => user.click(screen.getByTestId("b"))); + assertEquals(state.value, "initial"); // Should be rolled back to initial + assertEquals(renders, []); // Still idle + assertEquals(successMessages, []); + assertEquals(errorMessages, []); + assertEquals(mutations, []); + + await act(() => vi.runAllTimers()); + assertEquals(successMessages, []); + assertEquals(errorMessages, []); + assertEquals(mutations, []); +}); + +test("debounced snapshot should mutate when final differs from initial", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + const user = userEvent.setup({ delay: null }); + + const { client, successMessages, errorMessages } = createTestMutationClient(); + const s = new IterableStream(); + + let state = { value: "initial" }; + + const mutUpdate = client.define({ + mutate: async (newValue: string) => { + return (await s.next()).value; + }, + optimistic: ({ args: [newValue] }) => { + state.value = newValue; + }, + snapshot: () => state.value, + debounceMs: 500, + refetchOnSuccess: false, + describe: "Update value", + describeResult: "Updated value", + }); + + let renders: Array<{ status: string; result: string | undefined }> = []; + function TestComponent() { + const { run, status, result } = useMutate(mutUpdate); + renders.push({ status, result }); + return ( + <> + + + + ); + } + + render(); + renders = []; + + // First call + await act(() => user.click(screen.getByTestId("a"))); + assertEquals(state.value, "temp"); + renders = []; + + // Second call - different from initial + await act(() => user.click(screen.getByTestId("b"))); + assertEquals(state.value, "final"); + renders = []; + + // Wait for debounce + await act(async () => { + vi.advanceTimersByTime(500); + }); + assertEquals(renders, [{ status: "mutating", result: undefined }]); + renders = []; + + // Complete mutation + await act(async () => { + s.push("ok"); + vi.advanceTimersByTime(100); + }); + assertEquals(renders, [{ status: "success", result: "ok" }]); + assertEquals(successMessages, []); +}); diff --git a/test/mutations.test.ts b/test/mutations.test.ts index f8843fbff0260fa79553a070041b06d1763f44ab..9953a339a9e96bef3d95010234561507f6bbdd50 100644 --- a/test/mutations.test.ts +++ b/test/mutations.test.ts @@ -1,6 +1,48 @@ -import { test } from "vitest"; +import { delay } from "@clo/lib/async.ts"; +import { MutationClient } from "@clo/react-mutation"; +import { assertEquals } from "@std/assert"; +import { test, vi } from "vitest"; import { createTestMutationClient } from "./share.ts"; -test("apply optimistic update, refetch when done", () => { - const { client } = createTestMutationClient(); +test("run executes callbacks in correct order", async () => { + vi.useFakeTimers(); + const { client, errorMessages, successMessages } = createTestMutationClient(); + const calls: string[] = []; + const mutTest = client.define({ + mutate: async () => { + calls.push("mutate"); + await delay(100); + return "success"; + }, + describe: () => { + calls.push("describe"); + return "Test the action"; + }, + describeResult: () => { + calls.push("describeResult"); + return "Tested the action"; + }, + optimistic: ({ onRefetch, onRestore, onSuccess }) => { + onRefetch(async () => void calls.push("refetch")); + onSuccess(async () => void calls.push("success")); + onRestore(async () => void calls.push("restore")); + calls.push("optimistic"); + }, + }); + mutTest.run(); + await vi.advanceTimersByTimeAsync(50); + assertEquals(calls, [ + "optimistic", + "mutate", + ]); + await vi.advanceTimersByTimeAsync(100); + assertEquals(calls, [ + "optimistic", + "mutate", + "success", + "refetch", + "describeResult", + ]); + assertEquals(errorMessages, []); + assertEquals(successMessages, ["Tested the action"]); });