diff --git a/jsr.json b/jsr.json index ac3c21a0700fdbdddf9efe9f082a49176a1a568d..97ae6d43d77e074576a9e0ace43b10140e01d3db 100644 --- a/jsr.json +++ b/jsr.json @@ -1,6 +1,6 @@ { "name": "@clo/react-mutation", - "version": "3.0.0", + "version": "3.0.1", "exports": { ".": "./src/mod.ts", "./tanstack-query": "./src/tanstack-query.ts", diff --git a/package.json b/package.json index 514be0ad76fa73d53ce78249051152f2e4677560..ff737c7d90896cfe4ba0f4142a552f6da8ebc4e1 100644 --- a/package.json +++ b/package.json @@ -1,11 +1,8 @@ { "name": "@clo/react-mutation", "private": true, - "description": "", "license": "ISC", - "author": "", "type": "module", - "main": "index.js", "scripts": { "dev": "vitest dev", "test": "vitest run", diff --git a/readme.changes.md b/readme.changes.md index 3405cb0e4af03a236a614a1757c7069f8cfae7ee..672088958f0e22e8bfbb908e93d3ac37ccbb523a 100644 --- a/readme.changes.md +++ b/readme.changes.md @@ -1,5 +1,13 @@ # notable changes in React Mutation +## v3.0.1 + +### bugfixes + +- resolve an SSR memory leak in `useMutate` +- resolve `isAllowed`/`isDisabled` no longer updating after StrictMode's + simulated remount permanently dropped the subscription + ## v3 ### breaking diff --git a/src/client.ts b/src/client.ts index 2ccb96dfbadedd32b442f7740ea18ea7ed25b474..46c9bf6fae16475cf89520177366206c7e571681 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1,5 +1,34 @@ import { BlockingMutation, type MutationOptions } from "./mutation.ts"; -import type { Json, Mutation, MutationAction } from "./types.ts"; +import type { Mutation, MutationAction, Serializable, SerializableValue } from "./types.ts"; + +/** + * Every element of a fixed argument tuple passes the {@link Serializable} check. + * + * The recursion deliberately walks head/tail and treats the non-tuple tail as + * valid: any construct that inspects the whole tuple instead (`keyof`, `length`, + * a mapped type, or `[...infer A]` reconstruction) forces resolution of the + * `NoInfer`-wrapped argument type and misfires on the `async function` mutate + * form, which is common. The cost is that a purely variadic mutate signature + * (`mutate: (...xs: NonSerializable[])`) reaches the `true` base case unchecked; + * fixed and leading-fixed parameters are still validated. + */ +type AllSerializable = Args extends [infer Head, ...infer Tail] + ? [Head] extends [Serializable] ? AllSerializable : false + : true; + +/** + * Compile-time guard for the serializable-argument rule, expressed as `define`'s + * trailing rest parameter: an empty tuple (nothing to pass) when the rule holds, + * a one-element tuple otherwise, so a non-serializable `auth: true` call fails on + * argument count at the call site. It rides a separate parameter rather than an + * intersection on the options object, which is the sole inference source for + * `Args` (via `mutate`) and collapses under intersection; and rather than a + * type-parameter bound, which silently goes permissive against inferred + * arguments. Scoped and unauthenticated mutations are unconstrained. + */ +type SerializableArgsGuard = Auth extends true ? AllSerializable extends true ? [] + : [error: "auth:true mutation arguments must be serializable"] + : []; export interface MutationClientConfig { context: {}; @@ -61,7 +90,7 @@ export interface MutationClientOptions< handleUnauthenticated?: ( this: { context: Context }, action: MutationAction, - mutation: Mutation, + mutation: Mutation, ) => void; } @@ -84,13 +113,13 @@ export class MutationClient< context: Context; userContext?: Reactive; authScopes?: { [scope: string]: Reactive }; - handleUnauthenticated?: (action: MutationAction, mutation: Mutation) => void; + handleUnauthenticated?: (action: MutationAction, mutation: Mutation) => void; getOptimisticHelpers: (event: OptimisticEvents) => OptimisticHelpers; reportError: (message: string, error: unknown) => void; reportSuccess?: (message: string) => void; deepEquals: (a: unknown, b: unknown) => boolean; enabled: boolean; - #mutations: Map> = new Map(); + #mutations: Map> = new Map(); constructor(options: MutationClientOptions) { this.context = options.context; @@ -106,14 +135,27 @@ export class MutationClient< /** * Define a standard mutation. + * + * Only an `auth: true` mutation attempted without a user builds a serializable + * {@link MutationAction} for replay after sign-in, so only its arguments must + * be serializable, enforced by {@link SerializableArgsGuard}. That check is a + * {@link Serializable} mapped type rather than a concrete value type so + * `interface` arguments type-check, and the leaf set stays extensible via + * {@link SerializableLeaves}. Scoped (`auth: ""`) and unauthenticated + * mutations never produce an action and leave their arguments unconstrained. */ - define( + define< + const Args extends unknown[], + Result, + const Auth extends boolean | AuthScopes = false, + >( options: MutationOptions< Args, Result, Auth, { context: Context; optimisticHelpers: OptimisticHelpers; userContext: UserContext; authScopes: AuthScopes } >, + ..._serializable: SerializableArgsGuard, NoInfer> ): Mutation { if (typeof options.auth === "string" && !this.authScopes?.[options.auth]) { throw new Error(`Unknown auth scope "${options.auth}".`); @@ -138,7 +180,7 @@ export class MutationClient< this, options, ); - this.#mutations.set(options.id, mutation as unknown as Mutation); + this.#mutations.set(options.id, mutation as unknown as Mutation); return mutation; } diff --git a/src/mod.ts b/src/mod.ts index 08eb4f7df483d22f76ceb161c8ad982cea1c6939..33159dff52687a8c579a405eb8a27eed5ebee34a 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -19,4 +19,12 @@ export { type UseMutateResultBase, type UseMutateSuccess, } from "./react.ts"; -export type { Json, Mutation, MutationAction, MutationEvent } from "./types.ts"; +export type { + Json, + Mutation, + MutationAction, + MutationEvent, + Serializable, + SerializableLeaves, + SerializableValue, +} from "./types.ts"; diff --git a/src/mutation.ts b/src/mutation.ts index 9ca4e703ccb31fc9cf2bf440fa8e3dc8367ba887..ff3089a53c74ab193fbec0dbfe54da6b3b3aada6 100644 --- a/src/mutation.ts +++ b/src/mutation.ts @@ -1,7 +1,7 @@ import { message as errMessage } from "@clo/lib/error.ts"; import type { MutationClientFromConfig, Reactive } from "./client.ts"; import type { MutationClientConfig } from "./client.ts"; -import type { Json, Mutation, MutationEvent, RunOptions } from "./types.ts"; +import type { Mutation, MutationEvent, RunOptions, SerializableValue } from "./types.ts"; /** * Argument to `defineBlocking`. @@ -10,7 +10,7 @@ import type { Json, Mutation, MutationEvent, RunOptions } from "./types.ts"; * @template Config - global values and helpers from `MutationContext` */ export interface MutationOptions< - Args extends Json[], + Args extends unknown[], Result, Auth extends boolean | string, Config extends MutationClientConfig, @@ -182,7 +182,7 @@ interface Item { } export class BlockingMutation< - Args extends Json[], + Args extends unknown[], Result, Auth extends boolean | string, Config extends MutationClientConfig, @@ -219,8 +219,10 @@ export class BlockingMutation< const handler = options.onUnauthenticated ?? this.#client.handleUnauthenticated; handler?.call( this.#client, - { id: this.#options.id, description: this.describe(...args), args }, - this as unknown as Mutation, + // Reached only for `auth: true`, whose arguments the define-time guard + // constrains to be serializable. + { id: this.#options.id, description: this.describe(...args), args: args as SerializableValue[] }, + this as unknown as Mutation, ); } diff --git a/src/react.ts b/src/react.ts index 75d8fe224185abec4c5db9062b0a709ae93edfbc..36a7b629b846909aac6b47adc3d582d466309285 100644 --- a/src/react.ts +++ b/src/react.ts @@ -7,13 +7,15 @@ import { type ReactNode, useCallback, useEffect, + useMemo, useRef, useState, + useSyncExternalStore, } from "react"; import { jsx } from "react/jsx-runtime"; import type { MutationClient, MutationClientConfig, MutationClientFromConfig } from "./client.ts"; import { BlockingMutation, formatFriendlyError } from "./mutation.ts"; -import type { Json, Mutation, RunOptions } from "./types.ts"; +import type { Mutation, RunOptions, SerializableValue } from "./types.ts"; /** * Subscribe to a mutation's status, as well as accessing a local `run` method. @@ -27,12 +29,26 @@ export function useMutate< ): UseMutateResult { const [_, setRerender] = useState(0); const [observer] = useState(() => new Observer(setRerender)); - useEffect(() => () => void observer.reset(), []); if (mutation !== observer.mutation) { observer.mutation = mutation; observer.reset(); - observer.subscribeAllowed(); } + // The allowed subscription lives in useSyncExternalStore: the server never + // subscribes, and StrictMode's simulated remount resubscribes. + const subscribeAllowed = useMemo(() => (onChange: () => void) => { + if (!mutation) return () => {}; + const unsubscribeMutation = mutation.subscribeAllowed(onChange); + const unsubscribeClient = mutation.client.userContext?.sub(onChange) ?? (() => {}); + return () => { + unsubscribeMutation(); + unsubscribeClient(); + }; + }, [mutation]); + // Everything the subscription can change about a render, as one comparable value. + const allowedSnapshot = () => + mutation === null ? -1 : (mutation.isAllowed() ? 1 : 0) | (mutation.isUnauthenticated() ? 2 : 0); + useSyncExternalStore(subscribeAllowed, allowedSnapshot, allowedSnapshot); + useEffect(() => () => void observer.reset(), []); return observer.binding; } @@ -62,11 +78,11 @@ export function bindAsyncCallback( live.current = { callback, options }; // A stable, unregistered mutation with no optimistic state; every field // reads the ref so it tracks the latest callback and options. Args are - // never serialized here (no id, no auth replay), so the `Json[]` bound is - // cast away locally. Empty describe/describeResult fall through to the + // never serialized here (no id, no auth replay), so the serializable bound + // is cast away locally. Empty describe/describeResult fall through to the // generic error message and no success toast, respectively. const [mutation] = useState(() => - new BlockingMutation( + new BlockingMutation( client as unknown as MutationClientFromConfig, { id: "", @@ -196,7 +212,6 @@ class Observer { setRerender: (fn: number) => void; mutation: Mutation | null = null; unsubscribe: (() => void) | null = null; - unsubscribeAllowed: (() => void) | null = null; currentKey: string | null = null; pendingTimer: Timer | null = null; debounced: boolean = false; @@ -223,30 +238,12 @@ class Observer { reset() { this.unsubscribe?.(); - this.unsubscribeAllowed?.(); this.unsubscribe = null; - this.unsubscribeAllowed = null; this.currentKey = null; this.state = initialState(); this.debounced = false; } - subscribeAllowed() { - const mutation = this.mutation; - if (!mutation) return; - const rerenderIfWatched = () => { - if (this.watched.has("isAllowed") || this.watched.has("isDisabled")) { - this.setRerender(Math.random()); - } - }; - const unsubscribeMutation = mutation.subscribeAllowed(rerenderIfWatched); - const unsubscribeClient = mutation.client.userContext?.sub(rerenderIfWatched) ?? (() => {}); - this.unsubscribeAllowed = () => { - unsubscribeMutation(); - unsubscribeClient(); - }; - } - resetPending() { this.setState({ isPending: false }); if (this.pendingTimer) clearTimeout(this.pendingTimer); @@ -441,11 +438,9 @@ class Observer { return self.state.isMutating; }, get isAllowed() { - self.watched.add("isAllowed"); return self.mutation?.isAllowed() ?? false; }, get isDisabled() { - self.watched.add("isDisabled"); self.watched.add("isMutating"); const mutation = self.mutation; if (!mutation || (self.state.isMutating && !self.debounced)) return true; diff --git a/src/types.ts b/src/types.ts index fd2d8326c42fc5cecc3630055822920749dc85b8..62a14ec45731b7c5c5ea7437dba9777d3b7f9d42 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,8 +1,8 @@ import type { MutationClient } from "./client.ts"; /** - * Mutation arguments must be JSON serializable so a call can be stored and - * replayed later, such as re-running a mutation after the user signs in. + * Plain JSON value. The default serializable leaf set, still exported for + * consumers that want the exact JSON shape. */ export type Json = | string @@ -12,6 +12,44 @@ export type Json = | Json[] | { [key: string]: Json }; +/** + * Registry of serializable leaf types. Empty by default, which reduces + * {@link SerializableValue} to plain JSON. Downstream consumers whose transport + * handles a superset of JSON augment this to register those types: + * + * ```ts + * declare module "@clo/react-mutation" { + * interface SerializableLeaves { file: File | Blob } + * } + * ``` + */ +export interface SerializableLeaves {} + +type Leaf = string | number | boolean | null | undefined | SerializableLeaves[keyof SerializableLeaves]; + +/** + * Validates that `T` is serializable: every leaf is a registered {@link Leaf} + * and nested objects/arrays recurse. Enforced at `define` for `auth: true` + * arguments; unlike a concrete value type it accepts `interface` arguments, + * because the object case is a mapped type over `keyof T` and so needs no index + * signature the way `{ [k: string]: ... }` would. Any non-serializable member + * (a function/method, an unregistered class) collapses to `never`. + */ +export type Serializable = [T] extends [Leaf] ? T + : T extends (...args: never[]) => unknown ? never + : T extends readonly unknown[] ? { [I in keyof T]: Serializable } + : T extends object ? { [K in keyof T]: Serializable } + : never; + +/** + * A concrete serializable value, driven by the {@link SerializableLeaves} + * registry. Stored in a {@link MutationAction} for replay after sign-in. + */ +export type SerializableValue = + | Leaf + | SerializableValue[] + | { [key: string]: SerializableValue }; + /** * A serializable record of a mutation call. Produced when an authenticated * mutation is attempted without a user; pass it to `MutationClient.run` @@ -20,7 +58,7 @@ export type Json = export interface MutationAction { id: string; description: string; - args: Json[]; + args: SerializableValue[]; } export interface Mutation { @@ -84,7 +122,7 @@ export interface RunOptions { * available. The action is JSON serializable and can be passed to * `MutationClient.run` after sign-in. */ - onUnauthenticated?: (action: MutationAction, mutation: Mutation) => void; + onUnauthenticated?: (action: MutationAction, mutation: Mutation) => void; } export interface MutationEvent { diff --git a/test/auth.test.tsx b/test/auth.test.tsx index 65bc60d2c7c800be986d932d63fda61ecfd27b7e..a9ec661249048206773035afceb0ac12fee765ad 100644 --- a/test/auth.test.tsx +++ b/test/auth.test.tsx @@ -566,3 +566,52 @@ test("authenticated mutation isAllowed receives user context", () => { currentUser = null; assertEquals(mutation.isAllowed(), false); }); + +// Type-level: the serializable-argument constraint applies only to `auth: true` +// mutations, which are the only ones captured into a replayable MutationAction. +// Guarded by the package's `tsc --noEmit`; never executed. +// deno-lint-ignore no-unused-vars +function _serializableArgsConstraint() { + const { client } = createAuthClient(() => null, false, { get: () => true, sub: () => () => {} }); + const base = { optimistic: () => {}, describe: "x", describeResult: null } as const; + + // Non-auth mutations accept non-serializable arguments (File, interfaces). + client.define({ id: "t1", mutate: async (_file: File) => {}, ...base }); + + // Scoped mutations never serialize either, so they are unconstrained. + client.define({ id: "t2", auth: "admin", mutate: async (_file: File) => {}, ...base }); + + // `auth: true` with serializable arguments is allowed. + client.define({ id: "t3", auth: true, mutate: async (_id: string) => {}, ...base }); + + // `auth: true` with `interface`-typed arguments is allowed. A concrete `Json` + // value type would reject these for lacking an index signature; the mapped + // `Serializable` check accepts them. + interface Payload { + name: string; + nested: { count: number; tags: string[] }; + } + client.define({ id: "t4", auth: true, mutate: async (_p: Payload) => {}, ...base }); + + // `auth: true` with a non-serializable argument is rejected. A function is + // never a registered leaf, so this holds regardless of augmentation below. + // @ts-expect-error a function is not serializable + client.define({ id: "t5", auth: true, mutate: async (_fn: () => void) => {}, ...base }); + + // Leading fixed arguments are checked even alongside a rest parameter. + // @ts-expect-error a non-serializable leading argument is rejected + client.define({ id: "t7", auth: true, mutate: async (_fn: () => void, ..._rest: number[]) => {}, ...base }); + + // A `File` argument is serializable here only because the `SerializableLeaves` + // augmentation below registers it; the library does not hard-code it. + client.define({ id: "t6", auth: true, mutate: async (_file: File) => {}, ...base }); +} + +// Type-level: consumers extend the serializable leaf set through the +// `SerializableLeaves` registry, so a superset value (here `File`) becomes a +// valid `auth: true` argument without the library forcing the type. +declare module "../src/types.ts" { + interface SerializableLeaves { + file: File; + } +} diff --git a/test/useMutate.test.tsx b/test/useMutate.test.tsx index b1e6a8816cf22ece19af5b2316a8effd4e1f01d8..a880a2b68a4a8e58b1e514f022d37792b35d48dd 100644 --- a/test/useMutate.test.tsx +++ b/test/useMutate.test.tsx @@ -1,8 +1,10 @@ import { assertEquals } from "@std/assert"; import { act, render, screen } from "@testing-library/react"; import { userEvent } from "@testing-library/user-event"; -import type { FC, MouseEventHandler, ReactNode } from "react"; +import { type FC, type MouseEventHandler, type ReactNode, StrictMode } from "react"; +import { renderToString } from "react-dom/server"; import { test, vi } from "vitest"; +import { MutationClient } from "../src/client.ts"; import { createMutationButton, useMutate } from "../src/react.ts"; import { createTestMutationClient, IterableStream } from "./share.ts"; @@ -321,3 +323,67 @@ test("MutationButton should allow args={null} to disable mutation runs", async ( assertEquals(mutate.mock.calls, []); }); + +function countedAllowedMutation() { + let activeSubscriptions = 0; + const subscribers = new Set<() => void>(); + let allowed = true; + const sub = (cb: () => void) => { + activeSubscriptions++; + subscribers.add(cb); + return () => { + activeSubscriptions--; + subscribers.delete(cb); + }; + }; + const client = new MutationClient({ + context: {}, + userContext: { get: () => null, sub }, + getOptimisticHelpers: () => ({}), + reportError: () => {}, + }); + const mutTest = client.define({ + id: "test-allowed-subscription", + mutate: async () => "ok", + describe: "Test", + describeResult: null, + optimistic: () => {}, + isAllowed: { get: () => allowed, sub }, + }); + const setAllowed = (next: boolean) => { + allowed = next; + subscribers.forEach((cb) => cb()); + }; + return { mutTest, setAllowed, activeSubscriptions: () => activeSubscriptions }; +} + +test("server rendering never subscribes to allowed state", () => { + const { mutTest, activeSubscriptions } = countedAllowedMutation(); + function App() { + useMutate(mutTest); + return null; + } + for (let i = 0; i < 3; i++) renderToString(); + assertEquals(activeSubscriptions(), 0); +}); + +test("StrictMode keeps the allowed subscription alive", () => { + const { mutTest, setAllowed, activeSubscriptions } = countedAllowedMutation(); + let lastIsAllowed: boolean | null = null; + function App() { + lastIsAllowed = useMutate(mutTest).isAllowed; + return null; + } + const view = render( + + + , + ); + assertEquals(lastIsAllowed, true); + + act(() => setAllowed(false)); + assertEquals(lastIsAllowed, false); + + view.unmount(); + assertEquals(activeSubscriptions(), 0); +});