From 4d1e45f2583e742214917b10f8b5a75881da08f7 Mon Sep 17 00:00:00 2001 From: Marco Rieger Date: Tue, 3 Nov 2026 08:17:25 +0100 Subject: [PATCH 1/2] fix: call the same hooks for every ghost chain `.use()` called one query hook per method call in the chain and skipped it when the target of a call was empty. The number of hooks therefore depended on the chain and on intermediate results. React aborted with "Rendered more hooks than during the previous render." when, between two renders, the target of a chain was set, an `undefined` result in the chain became available or a ghost with a different chain was passed (e.g. a plain value from `asGhostProps()` vs. a ghost). All queries of a chain now go through a single `useSuspenseQueries()`, whose hooks do not depend on the number of queries. The chain is walked once against the query cache to find the queries needed so far and once against the hook results to compute the value. Query keys, sequential fetching, options, error boundaries and target auto invalidation behave as before. Transforms are memoized via a ref instead of one `useMemo` per chain item. --- .../react-ghostmaker/src/makeGhost.test.tsx | 69 ++++- packages/react-ghostmaker/src/types.ts | 4 - .../react-ghostmaker/src/useGhostChain.ts | 250 ++++++++++++------ 3 files changed, 242 insertions(+), 81 deletions(-) diff --git a/packages/react-ghostmaker/src/makeGhost.test.tsx b/packages/react-ghostmaker/src/makeGhost.test.tsx index 49a949d..8c53209 100644 --- a/packages/react-ghostmaker/src/makeGhost.test.tsx +++ b/packages/react-ghostmaker/src/makeGhost.test.tsx @@ -15,7 +15,8 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { invalidateGhosts } from "./invalidate.ts"; import { makeGhost } from "./makeGhost.ts"; import { cleanupTargetHashes } from "./useGhostChain.ts"; -import { Component, Suspense, type ReactNode } from "react"; +import type { ReactGhost } from "./types.ts"; +import { Component, Suspense, useState, type ReactNode } from "react"; class FailingService { public async fail(): Promise { @@ -404,6 +405,72 @@ describe("Hooks", () => { }); }); + describe("Hook order", () => { + const settle = async () => { + for (let i = 0; i < 3; i++) { + await act(() => vitest.runOnlyPendingTimersAsync()); + } + }; + + test("stays the same when the target becomes available", async () => { + let project: Project | undefined; + + const { result, rerender } = await renderHookWithSuspense(() => { + useState(); + return makeGhost(project).getDetailed().name.use(); + }); + expect(result.current).toBeUndefined(); + + project = new Project("Project A"); + rerender(); + await settle(); + expect(result.current).toBe("Project Project A"); + + project = undefined; + rerender(); + expect(result.current).toBeUndefined(); + }); + + test("stays the same when a ghost with a different chain is used", async () => { + let projectGhost: ReactGhost = makeGhost( + new ProjectDetailed("Project A", "Plain project", "C1"), + ); + + const { result, rerender } = await renderHookWithSuspense(() => { + useState(); + return projectGhost.name.use(); + }); + expect(result.current).toBe("Plain project"); + + projectGhost = ProjectGhost.ofId("Project A").getDetailed(); + rerender(); + await settle(); + expect(result.current).toBe("Project Project A"); + }); + + test("stays the same when an undefined result in the chain becomes available", async () => { + const ghost = ProjectGhost.ofId("Project A") + .findDetailed() + .customer.getDetailed() + .getName(); + + const { result } = await renderHookWithSuspense(() => { + useState(); + return ghost.use(); + }); + expect(result.current).toBeUndefined(); + + projectMocks.findDetailed = vitest + .fn() + .mockImplementation( + (id) => new ProjectDetailed(id, `Project ${id}`, "C1"), + ); + ghost.invalidate(queryClient); + await settle(); + expect(result.current).toBe("Customer C1"); + }); + }); + describe("Reset", () => { test("useGhost.reset() triggers re-execution of all async methods", async () => { const { result } = await renderHookWithSuspense(() => diff --git a/packages/react-ghostmaker/src/types.ts b/packages/react-ghostmaker/src/types.ts index cf1fede..ee296c3 100644 --- a/packages/react-ghostmaker/src/types.ts +++ b/packages/react-ghostmaker/src/types.ts @@ -66,10 +66,6 @@ export type QueryKey = readonly unknown[]; export type InvalidateGhostFn = () => Promise; export type ResetGhostFn = () => Promise; -export interface GhostChainContext { - queryKey: QueryKey; -} - export interface UseGhostReturn { value: T; invalidate: InvalidateGhostFn; diff --git a/packages/react-ghostmaker/src/useGhostChain.ts b/packages/react-ghostmaker/src/useGhostChain.ts index fd2482e..17d577a 100644 --- a/packages/react-ghostmaker/src/useGhostChain.ts +++ b/packages/react-ghostmaker/src/useGhostChain.ts @@ -1,38 +1,81 @@ import is, { assert } from "@sindresorhus/is"; -import { useEffect, useEffectEvent, useMemo } from "react"; +import { useEffect, useRef } from "react"; import { transformFnProp, type GhostChain, - type GhostChainContext, type GhostChainItem, type QueryKey, type UseGhostOptions, type UseGhostReturn, type UseQueryReturnType, } from "./types.ts"; -import { useQueryClient, useSuspenseQuery } from "@tanstack/react-query"; +import { + useQueryClient, + useSuspenseQueries, + type QueryClient, + type UseSuspenseQueryOptions, +} from "@tanstack/react-query"; import { queries } from "./queries.ts"; import { ghostFnContext } from "./context.ts"; import { invalidateGhosts } from "./invalidate.ts"; import { hashObject } from "./hash.ts"; import { resetGhosts } from "./reset.ts"; +type ChainQueryOptions = UseSuspenseQueryOptions>; + +type GetChainQueryResult = ( + queryOptions: ChainQueryOptions, + index: number, +) => UseQueryReturnType | undefined; + +interface ChainCall { + queryKey: QueryKey; + target: object; +} + +interface TransformMemo { + dependencies: unknown[]; + value: unknown; +} + +type TransformMemos = Map; + +/** + * Resolves a ghost chain with a fixed set of hooks, regardless of the chain and + * its target. All queries of the chain go through a single + * `useSuspenseQueries()`, so React sees the same hooks when the target is + * empty, an intermediate result is undefined or a different ghost is passed. + */ export const useGhostChain = ( target: unknown, chain: GhostChain, - options?: UseGhostOptions, + options: UseGhostOptions = {}, ): UseGhostReturn => { const queryClient = useQueryClient(); + const transformMemos = useRef(new Map()); + + const walk = (getResult: GetChainQueryResult) => + walkGhostChain( + target, + chain, + options as UseGhostOptions, + transformMemos.current, + getResult, + ); - const context: GhostChainContext = { - queryKey: queries.target(target), - }; + const { queryOptions } = walk((queryOptions) => + getCachedQueryResult(queryClient, queryOptions), + ); + const results = useSuspenseQueries({ queries: queryOptions }); + + const failedQuery = results.find((result) => result.error); + if (failedQuery) { + throw failedQuery.error; + } + + const { value, calls } = walk((ignored, index) => results[index]?.data); - const value = chain.reduce( - (currentTarget, item) => - useGhostChainItem(currentTarget, item, options, context), - target as unknown, - ) as T; + useTargetAutoInvalidate(calls); const invalidate = () => invalidateGhosts(queryClient, queries.ghostChain(target, chain)); @@ -41,96 +84,151 @@ export const useGhostChain = ( resetGhosts(queryClient, queries.ghostChain(target, chain)); return { - value, + value: value as T, invalidate, reset, }; }; -const useGhostChainItem = ( +/** + * Walks the chain as far as `getResult` provides results. Returns the queries + * needed so far: up to and including the first one without a result. + */ +const walkGhostChain = ( target: unknown, - item: GhostChainItem, - options: UseGhostOptions = {}, - context: GhostChainContext, -): unknown => { - const { propName, args } = item; - - if (propName === transformFnProp) { - assert.array(args); - const transformFn = args[0]; - assert.function(transformFn, "transform requires a mapping function"); - - const transformDependencies = args[1] ?? []; - assert.array(transformDependencies); - return useMemo( - () => transformFn(target), - [target, ...transformDependencies], - ); - } - - if (is.nullOrUndefined(target)) { - return target; - } - assert.object(target); - - const targetPropertyName = propName as keyof typeof target; - const targetProperty = target[targetPropertyName]; - context.queryKey = queries.chainItem(context.queryKey, item); + chain: GhostChain, + options: UseGhostOptions, + transformMemos: TransformMemos, + getResult: GetChainQueryResult, +) => { + const queryOptions: ChainQueryOptions[] = []; + const calls: ChainCall[] = []; + let queryKey: QueryKey = queries.target(target); + let value = target; + + for (const [index, item] of chain.entries()) { + const { propName, args } = item; + + if (propName === transformFnProp) { + value = transform(value, item, transformMemos, index); + continue; + } - if (args === undefined) { - // no function call, just a property access - return targetProperty; - } + if (is.nullOrUndefined(value)) { + continue; + } + assert.object(value); - assert.function(targetProperty); - const targetFn = targetProperty.bind(target); + const targetProperty = value[propName as keyof typeof value]; + queryKey = queries.chainItem(queryKey, item); - const queryKey = [...context.queryKey]; + if (args === undefined) { + // no function call, just a property access + value = targetProperty; + continue; + } - const query = useSuspenseQuery>({ - ...options, + assert.function(targetProperty); + const targetFn = targetProperty.bind(value); + + const callOptions: ChainQueryOptions = { + ...(options as Omit), + queryKey, + queryFn: async (ctx) => { + const targetFnWithContext = ghostFnContext.bind( + { query: ctx }, + targetFn, + ); + return { result: await targetFnWithContext(...args) }; + }, + }; + + calls.push({ queryKey, target: value }); + queryOptions.push(callOptions); + + const result = getResult(callOptions, queryOptions.length - 1); + if (result === undefined) { + return { queryOptions, calls, value: undefined }; + } + value = result.result; + } - queryKey, - queryFn: async (ctx) => { - const targetFnWithContext = ghostFnContext.bind({ query: ctx }, targetFn); - return { result: await targetFnWithContext(...args) }; - }, - }); + return { queryOptions, calls, value }; +}; - useTargetAutoInvalidate(target, queryKey); +const getCachedQueryResult = ( + queryClient: QueryClient, + queryOptions: ChainQueryOptions, +) => { + const { queryHash } = queryClient.defaultQueryOptions(queryOptions); + return queryClient.getQueryCache().get>(queryHash) + ?.state.data; +}; - if (query.error) { - throw query.error; +const transform = ( + target: unknown, + item: GhostChainItem, + transformMemos: TransformMemos, + index: number, +) => { + const { args } = item; + assert.array(args); + const transformFn = args[0]; + assert.function(transformFn, "transform requires a mapping function"); + + const transformDependencies = args[1] ?? []; + assert.array(transformDependencies); + + const dependencies = [target, ...transformDependencies]; + const memo = transformMemos.get(index); + + if (memo && areDependenciesEqual(memo.dependencies, dependencies)) { + return memo.value; } - return query.data.result; + const value = transformFn(target); + transformMemos.set(index, { dependencies, value }); + return value; }; +const areDependenciesEqual = (a: unknown[], b: unknown[]) => + a.length === b.length && + a.every((dependency, i) => Object.is(dependency, b[i])); + const targetHashes = new Map(); -const useTargetAutoInvalidate = (target: object, queryKey: QueryKey) => { +const useTargetAutoInvalidate = (calls: ChainCall[]) => { const queryClient = useQueryClient(); + const previousTargets = useRef(new Map()); + + useEffect(() => { + const currentTargets = new Map(); + + for (const { queryKey, target } of calls) { + const joinedQueryKey = queryKey.join("."); + currentTargets.set(joinedQueryKey, target); - const onTargetChangeEvent = useEffectEvent(() => { - const joinedQueryKey = queryKey.join("."); - const targetHash = hashObject(target); - const prevTargetHash = targetHashes.get(joinedQueryKey); + if (previousTargets.current.get(joinedQueryKey) === target) { + continue; + } - const needsRefresh = - prevTargetHash !== undefined && prevTargetHash !== targetHash; + const targetHash = hashObject(target); + const prevTargetHash = targetHashes.get(joinedQueryKey); - targetHashes.set(joinedQueryKey, targetHash); + const needsRefresh = + prevTargetHash !== undefined && prevTargetHash !== targetHash; - if (needsRefresh) { - queryClient.invalidateQueries({ - queryKey, - }); + targetHashes.set(joinedQueryKey, targetHash); + + if (needsRefresh) { + queryClient.invalidateQueries({ + queryKey, + }); + } } - }); - useEffect(() => { - onTargetChangeEvent(); - }, [target]); + previousTargets.current = currentTargets; + }); }; /** @internal */ From 63bc5a98fad0aa07a6413a70e615b66ea5a03ac5 Mon Sep 17 00:00:00 2001 From: Marco Rieger Date: Tue, 3 Nov 2026 08:17:25 +0100 Subject: [PATCH 2/2] feat: add optionalGhost() for ghosts that may be missing `ghost?.use()` skips the hook while the ghost is missing, before ghostmaker is involved. `optionalGhost(ghost).use()` always calls the same hooks and resolves to `undefined` if no ghost is given. --- README.md | 38 +++++++++ packages/react-ghostmaker/src/index.ts | 1 + .../src/optionalGhost.test-d.ts | 23 ++++++ .../src/optionalGhost.test.tsx | 82 +++++++++++++++++++ .../react-ghostmaker/src/optionalGhost.ts | 13 +++ 5 files changed, 157 insertions(+) create mode 100644 packages/react-ghostmaker/src/optionalGhost.test-d.ts create mode 100644 packages/react-ghostmaker/src/optionalGhost.test.tsx create mode 100644 packages/react-ghostmaker/src/optionalGhost.ts diff --git a/README.md b/README.md index 6a785d9..c610ac3 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ making your code cleaner and more declarative. - [🔄 Error Handling & Loading States](#-error-handling--loading-states) - [🏗️ Working with Domain Models](#️-working-with-domain-models) - [🎭 Flexible Component Props with MaybeReactGhost](#-flexible-component-props-with-maybereactghost) +- [🫥 Optional Ghosts](#-optional-ghosts) - [🔧 Cache Management](#-cache-management) - [📚 Complete Example](#-complete-example) - [🔍 API Reference](#-api-reference) @@ -633,6 +634,41 @@ Don't use it when: - You always know the data will be resolved (use the plain type `T`) - Simple components that don't need this flexibility +## 🫥 Optional Ghosts + +`.use()` is a React hook, so it must be called on every render. Optional +chaining like `projectGhost?.use()` skips the call while the ghost is missing. +If the ghost is set on a later render, React fails with "Rendered more hooks +than during the previous render." + +Wrap optional ghosts with `optionalGhost()` instead. It always calls the hook +and resolves to `undefined` as long as no ghost is given: + +```tsx +import { optionalGhost, type ReactGhost } from "@mittwald/react-ghostmaker"; + +function ProjectInfo(props: { projectGhost?: ReactGhost }) { + // ❌ skips the hook while projectGhost is undefined + // const project = props.projectGhost?.use(); + + // ✅ always calls the same hooks + const project = optionalGhost(props.projectGhost).use(); + + return {project?.name ?? "No project"}; +} +``` + +This also works with `asGhostProps()`, which leaves missing props `undefined`: + +```tsx +const { projectGhost } = asGhostProps(props, ["project"]); +const project = optionalGhost(projectGhost).use(); +``` + +`.use()` calls the same hooks for every ghost: regardless of the length of its +chain, an empty target or an `undefined` result within the chain. A component +can therefore receive a different ghost on the next render. + ## 🔧 Cache Management ### Invalidate Queries @@ -866,6 +902,8 @@ Creates a ghost proxy from any object or class instance. ### Utility Functions - **`asGhostProps(props)`** - Converts props to ghost-compatible format +- **`optionalGhost(ghost)`** - Returns a ghost that resolves to `undefined` if + `ghost` is missing; use it instead of `ghost?.use()` - **`GhostMakerModel(options)`** - Decorator to define model name and ID for automatic query key generation - **`invalidateGhostsById(id)`** - Globally invalidate cached data by ID - **`getGhostId(ghost)`** - Get the unique ID of a ghost diff --git a/packages/react-ghostmaker/src/index.ts b/packages/react-ghostmaker/src/index.ts index a2c7933..70ae72b 100644 --- a/packages/react-ghostmaker/src/index.ts +++ b/packages/react-ghostmaker/src/index.ts @@ -1,4 +1,5 @@ export { makeGhost } from "./makeGhost.ts"; +export { optionalGhost } from "./optionalGhost.ts"; export { registerModelIdentifier } from "./modelIdentifier.ts"; export { type ReactGhost, UseGhostReturn } from "./types.ts"; export { getQueryContext, forwardQueryContext } from "./context.ts"; diff --git a/packages/react-ghostmaker/src/optionalGhost.test-d.ts b/packages/react-ghostmaker/src/optionalGhost.test-d.ts new file mode 100644 index 0000000..785eb17 --- /dev/null +++ b/packages/react-ghostmaker/src/optionalGhost.test-d.ts @@ -0,0 +1,23 @@ +import { expectTypeOf, test } from "vitest"; +import type { ProjectDetailed } from "./testMocks.ts"; +import { ProjectGhost } from "./testMocks.ts"; +import type { ReactGhost } from "./types.ts"; +import { optionalGhost } from "./optionalGhost.ts"; + +test("optionalGhost() makes the value optional", () => { + const projectGhost = ProjectGhost.ofId("Project A").getDetailed() as + | ReactGhost + | undefined; + + const optionalProjectGhost = optionalGhost(projectGhost); + + expectTypeOf(optionalProjectGhost).toEqualTypeOf< + ReactGhost + >(); + expectTypeOf(optionalProjectGhost.use()).toEqualTypeOf< + ProjectDetailed | undefined + >(); + expectTypeOf(optionalProjectGhost.name.use()).toEqualTypeOf< + string | undefined + >(); +}); diff --git a/packages/react-ghostmaker/src/optionalGhost.test.tsx b/packages/react-ghostmaker/src/optionalGhost.test.tsx new file mode 100644 index 0000000..32ae7f0 --- /dev/null +++ b/packages/react-ghostmaker/src/optionalGhost.test.tsx @@ -0,0 +1,82 @@ +import { act, cleanup, render } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { Suspense, useState } from "react"; +import { afterEach, beforeEach, expect, test, vitest } from "vitest"; +import { optionalGhost } from "./optionalGhost.ts"; +import { ProjectGhost } from "./testMocks.ts"; +import type { ReactGhost } from "./types.ts"; +import { cleanupTargetHashes } from "./useGhostChain.ts"; + +const queryClient = new QueryClient(); + +beforeEach(() => { + vitest.useFakeTimers(); + cleanupTargetHashes(); + queryClient.clear(); +}); + +afterEach(() => { + cleanup(); + vitest.runOnlyPendingTimers(); + vitest.useRealTimers(); +}); + +type NameGhost = ReactGhost | null | undefined; + +const ProjectName = (props: { nameGhost: NameGhost }) => { + useState(); + return <>{optionalGhost(props.nameGhost).use() ?? "no project"}; +}; + +const renderProjectName = (nameGhost: NameGhost) => { + const element = (nameGhost: NameGhost) => ( + + + + + + ); + + const ui = render(element(nameGhost)); + + return { + ui, + rerender: (nameGhost: NameGhost) => ui.rerender(element(nameGhost)), + }; +}; + +const settle = async () => { + for (let i = 0; i < 3; i++) { + await act(() => vitest.runOnlyPendingTimersAsync()); + } +}; + +test("resolves to undefined without suspending if the ghost is missing", () => { + expect(renderProjectName(undefined).ui.container.textContent).toBe( + "no project", + ); + cleanup(); + expect(renderProjectName(null).ui.container.textContent).toBe("no project"); +}); + +test("resolves the given ghost", async () => { + const { ui } = renderProjectName( + ProjectGhost.ofId("Project A").getDetailed().name, + ); + expect(ui.container.textContent).toBe("loading"); + + await settle(); + expect(ui.container.textContent).toBe("Project Project A"); +}); + +test("keeps the hook order when the ghost is set or removed", async () => { + const { ui, rerender } = renderProjectName(undefined); + expect(ui.container.textContent).toBe("no project"); + + rerender(ProjectGhost.ofId("Project A").getDetailed().name); + await settle(); + expect(ui.container.textContent).toBe("Project Project A"); + + rerender(undefined); + expect(ui.container.textContent).toBe("no project"); +}); diff --git a/packages/react-ghostmaker/src/optionalGhost.ts b/packages/react-ghostmaker/src/optionalGhost.ts new file mode 100644 index 0000000..c6ebaeb --- /dev/null +++ b/packages/react-ghostmaker/src/optionalGhost.ts @@ -0,0 +1,13 @@ +import { makeGhost } from "./makeGhost.ts"; +import type { ReactGhost } from "./types.ts"; + +/** + * Use instead of `ghost?.use()`: optional chaining skips the hook entirely when + * the ghost is missing. The returned ghost always calls the same hooks and + * resolves to `undefined` if no ghost is given. + */ +export function optionalGhost( + ghost: ReactGhost | null | undefined, +): ReactGhost { + return (ghost ?? makeGhost(undefined)) as ReactGhost; +}