From b6d3ee71cf946a1c49660570a2f4e5246b86311b Mon Sep 17 00:00:00 2001 From: meh Date: Wed, 2 Sep 2026 16:20:16 +0700 Subject: [PATCH] fix(data)!: read query and mutation state as properties 2.12.0 exposed accessors -- `query.data()`. `solid-query`, which this replaces, returned a store, so every consumer that will be migrated onto it is written `query.data`: about 3300 such reads across these applications, 2097 of them `.data` alone. Keeping accessors would have made the migration touch every file that *reads* a query rather than the ~300 that define one, for no gain. These are getters over the same signals, so a read inside a tracked scope subscribes exactly as an accessor call did -- there is a test asserting the subscription rather than just the value. `isPending`, `isError` and `isSuccess` come along for the same reason, so carried-over call sites keep working. `isPending` deliberately does not mean what it meant: there it was "has no data", which stayed true forever for a query held back by `enabled: false`, so `if (isPending) return ` spun for the life of the page. Here it means a fetch is in flight. Breaking against 2.12.0, which no application consumes yet. --- src/hooks/data/createMutation.ts | 28 +++++++++----- src/hooks/data/createQuery.test.ts | 54 +++++++++++++++++++++----- src/hooks/data/createQuery.ts | 62 +++++++++++++++++++++++++----- 3 files changed, 115 insertions(+), 29 deletions(-) diff --git a/src/hooks/data/createMutation.ts b/src/hooks/data/createMutation.ts index 3983d5c9..498a950f 100644 --- a/src/hooks/data/createMutation.ts +++ b/src/hooks/data/createMutation.ts @@ -1,11 +1,10 @@ import { createSignal } from "solid-js"; -import type { Accessor } from "solid-js"; /** * A write, without a query library. The companion to `createQuery`. * * Replaces `useMutation`. The same rule applies as there: reading this never - * suspends and never throws. `mutate` reports failure through `error()`; + * suspends and never throws. `mutate` reports failure through `error`; * `mutateAsync` rejects, for a caller that wants to await and handle it. */ @@ -17,15 +16,17 @@ export interface CreateMutationOptions { onSettled?: () => void | Promise; } +/** Read as properties, for the same reason as `QueryResult`. */ export interface MutationResult { - /** Fire and forget. Failure lands on `error()` rather than as a rejection. */ + /** Fire and forget. Failure lands on `error` rather than as a rejection. */ mutate: (...args: TArgs) => void; /** Fire and await. Rejects on failure. */ mutateAsync: (...args: TArgs) => Promise; - isPending: Accessor; - error: Accessor; + readonly isPending: boolean; + readonly error: unknown; + readonly isError: boolean; /** The last successful result. */ - data: Accessor; + readonly data: TResult | undefined; /** Clear `error` and `data`. */ reset: () => void; } @@ -69,9 +70,18 @@ export const createMutation = ( void mutateAsync(...args).catch(() => {}); }, mutateAsync, - isPending, - error, - data, + get isPending() { + return isPending(); + }, + get error() { + return error(); + }, + get isError() { + return error() !== undefined; + }, + get data() { + return data(); + }, reset: () => { setError(undefined); setData(() => undefined); diff --git a/src/hooks/data/createQuery.test.ts b/src/hooks/data/createQuery.test.ts index b25781cc..f5f640c4 100644 --- a/src/hooks/data/createQuery.test.ts +++ b/src/hooks/data/createQuery.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { createRoot, flush } from "solid-js"; +import { createRenderEffect, createRoot, flush } from "solid-js"; // Run with `bun test --conditions=browser`, which the package script does. // Without it Bun resolves Solid's server build, where effects run once and @@ -27,9 +27,9 @@ describe("createQuery", () => { return "value"; }, })); - expect(q.isLoading()).toBe(false); - expect(q.data()).toBeUndefined(); - expect(q.isReady()).toBe(false); + expect(q.isLoading).toBe(false); + expect(q.data).toBeUndefined(); + expect(q.isReady).toBe(false); return d; }); await tick(); @@ -45,10 +45,10 @@ describe("createQuery", () => { fetcher: () => new Promise((r) => (resolveFetch = r)), })); flush(); - const whileLoading = { loading: q.isLoading(), ready: q.isReady() }; + const whileLoading = { loading: q.isLoading, ready: q.isReady }; resolveFetch?.("hello"); await tick(); - return { whileLoading, data: q.data(), ready: q.isReady(), dispose }; + return { whileLoading, data: q.data, ready: q.isReady, dispose }; }); expect(result.whileLoading).toEqual({ loading: true, ready: false }); expect(result.data).toBe("hello"); @@ -56,7 +56,7 @@ describe("createQuery", () => { result.dispose(); }); - test("a failure lands on error() rather than being thrown", async () => { + test("a failure lands on error rather than being thrown", async () => { const result = await createRoot(async (dispose) => { const q = createQuery(() => ({ key: ["bad"], @@ -66,7 +66,7 @@ describe("createQuery", () => { })); flush(); await tick(); - return { error: q.error(), loading: q.isLoading(), dispose }; + return { error: q.error, loading: q.isLoading, dispose }; }); expect((result.error as Error).message).toBe("nope"); expect(result.loading).toBe(false); @@ -123,6 +123,40 @@ describe("createQuery", () => { }); }); +describe("property reads stay reactive", () => { + test("a tracked scope re-runs when data lands", async () => { + // The whole reason these are getters rather than accessors: consumers + // carried over from solid-query are written `query.data`, thousands of + // times. A getter over a signal only earns that spelling if reading it + // inside a tracked scope still subscribes -- so assert the subscription, + // not just the value. + let resolveFetch: ((value: string) => void) | undefined; + const seen: (string | undefined)[] = []; + + const result = await createRoot(async (dispose) => { + const q = createQuery(() => ({ + key: ["reactive"], + fetcher: () => new Promise((r) => (resolveFetch = r)), + })); + createRenderEffect( + () => q.data, + (value) => { + seen.push(value); + }, + ); + flush(); + resolveFetch?.("arrived"); + await tick(); + flush(); + return { dispose }; + }); + + expect(seen[0]).toBeUndefined(); + expect(seen.at(-1)).toBe("arrived"); + result.dispose(); + }); +}); + describe("createMutation", () => { test("mutate reports failure without an unhandled rejection", async () => { const result = await createRoot(async (dispose) => { @@ -133,7 +167,7 @@ describe("createMutation", () => { })); m.mutate(); await tick(); - return { error: m.error(), pending: m.isPending(), dispose }; + return { error: m.error, pending: m.isPending, dispose }; }); expect((result.error as Error).message).toBe("write failed"); expect(result.pending).toBe(false); @@ -150,7 +184,7 @@ describe("createMutation", () => { }, })); const value = await m.mutateAsync("app"); - return { value, data: m.data(), dispose }; + return { value, data: m.data, dispose }; }); expect(result.value).toBe("made app"); expect(result.data).toBe("made app"); diff --git a/src/hooks/data/createQuery.ts b/src/hooks/data/createQuery.ts index f194ac65..0374e6c0 100644 --- a/src/hooks/data/createQuery.ts +++ b/src/hooks/data/createQuery.ts @@ -1,5 +1,4 @@ import { createRenderEffect, createSignal, onCleanup, untrack } from "solid-js"; -import type { Accessor } from "solid-js"; /** * Asynchronous reads, without a query library. @@ -18,7 +17,7 @@ import type { Accessor } from "solid-js"; * had nothing to print. That is how a support-chat button that had not * connected replaced an entire application with a blank error page. * - * Here, `data()` is `undefined` until there is data, `isLoading()` is true only + * Here, `data` is `undefined` until there is data, `isLoading` is true only * while a fetch is actually in flight, and neither ever throws. A caller that * wants to suspend can do so explicitly; a caller that forgets cannot take the * page down. @@ -36,15 +35,41 @@ export interface CreateQueryOptions { enabled?: boolean; } +/** + * Read as properties, not as accessors. + * + * `solid-query` returned a store, so every consumer of it is written + * `query.data`, `query.isLoading`, `query.isError` -- roughly three thousand + * such reads across these applications. Exposing accessors here would mean + * editing all of them to add `()`, turning a migration of the ~300 files that + * *define* queries into one that touches every file that reads one. + * + * These are getters over signals, so a read inside a tracked scope still + * subscribes exactly as an accessor call would. + */ export interface QueryResult { /** The last value read, or `undefined` before the first one arrives. */ - data: Accessor; + readonly data: T | undefined; /** The last failure, cleared by the next successful read. */ - error: Accessor; + readonly error: unknown; /** True only while a fetch is in flight. Never true for a disabled query. */ - isLoading: Accessor; + readonly isLoading: boolean; + /** + * Alias of `isLoading`, for call sites carried over from `solid-query`. + * + * Note the deliberate difference in meaning. There, `isPending` was "has no + * data", which stayed true forever for a query held back by `enabled: false` + * -- so `if (isPending) return ` spun for the life of the page. + * Here it means "a fetch is in flight", so a disabled query reads as not + * pending and its consumer renders instead of hanging. + */ + readonly isPending: boolean; + /** True when the last read failed. */ + readonly isError: boolean; + /** True when a value has arrived and the last read did not fail. */ + readonly isSuccess: boolean; /** True once a value has arrived at least once. */ - isReady: Accessor; + readonly isReady: boolean; /** Read again now, regardless of `enabled`. */ refetch: () => Promise; } @@ -126,10 +151,27 @@ export const createQuery = ( }); return { - data, - error, - isLoading, - isReady, + get data() { + return data(); + }, + get error() { + return error(); + }, + get isLoading() { + return isLoading(); + }, + get isPending() { + return isLoading(); + }, + get isError() { + return error() !== undefined; + }, + get isSuccess() { + return isReady() && error() === undefined; + }, + get isReady() { + return isReady(); + }, refetch: run, }; };