From 2418655b3ebd1f0ed338f9524b5477ddc85b6bb3 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 14:07:28 -0400 Subject: [PATCH 01/19] feat: add useEventCallback for stable access to the latest callback --- .../src/hooks/useEventCallback.test.ts | 39 +++++++++++++++++++ .../src/hooks/useEventCallback.ts | 20 ++++++++++ 2 files changed, 59 insertions(+) create mode 100644 packages/modules/imodel-browser/src/hooks/useEventCallback.test.ts create mode 100644 packages/modules/imodel-browser/src/hooks/useEventCallback.ts diff --git a/packages/modules/imodel-browser/src/hooks/useEventCallback.test.ts b/packages/modules/imodel-browser/src/hooks/useEventCallback.test.ts new file mode 100644 index 00000000..07f66625 --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useEventCallback.test.ts @@ -0,0 +1,39 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { renderHook } from "@testing-library/react-hooks"; + +import { useEventCallback } from "./useEventCallback"; + +describe("useEventCallback", () => { + const renderWith = (fn: (value: string) => string) => + renderHook<{ fn: (value: string) => string }, (value: string) => string>( + ({ fn: current }) => useEventCallback(current), + { initialProps: { fn } } + ); + + it("keeps one identity across renders", () => { + const { result, rerender } = renderWith(() => "first"); + const stable = result.current; + + rerender({ fn: () => "second" }); + + expect(result.current).toBe(stable); + }); + + it("calls the latest function", () => { + const { result, rerender } = renderWith(() => "first"); + expect(result.current("ignored")).toEqual("first"); + + rerender({ fn: () => "second" }); + + expect(result.current("ignored")).toEqual("second"); + }); + + it("forwards the arguments", () => { + const { result } = renderWith((value) => `got ${value}`); + + expect(result.current("payload")).toEqual("got payload"); + }); +}); diff --git a/packages/modules/imodel-browser/src/hooks/useEventCallback.ts b/packages/modules/imodel-browser/src/hooks/useEventCallback.ts new file mode 100644 index 00000000..e27b7655 --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useEventCallback.ts @@ -0,0 +1,20 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import React from "react"; + +/** + * A function with one identity for the life of the hook that always calls the latest `fn`. Lets a + * caller pass an inline closure where a stable dependency is needed. The standard `useEffectEvent` + * polyfill: an insertion effect writes the ref before any layout or passive effect can read it. + */ +export const useEventCallback = ( + fn: (...args: TArgs) => TResult +): ((...args: TArgs) => TResult) => { + const latestFn = React.useRef(fn); + React.useInsertionEffect(() => { + latestFn.current = fn; + }, [fn]); + return React.useCallback((...args: TArgs) => latestFn.current(...args), []); +}; From ea6c644ddc1a8444410a88d21053d47c7b5bec46 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 14:10:21 -0400 Subject: [PATCH 02/19] feat: add useReportChanges to report each distinct value once --- .../src/hooks/useReportChanges.test.ts | 71 +++++++++++++++++++ .../src/hooks/useReportChanges.ts | 26 +++++++ 2 files changed, 97 insertions(+) create mode 100644 packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts create mode 100644 packages/modules/imodel-browser/src/hooks/useReportChanges.ts diff --git a/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts b/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts new file mode 100644 index 00000000..75b9591e --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts @@ -0,0 +1,71 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { renderHook } from "@testing-library/react-hooks"; + +import { useReportChanges } from "./useReportChanges"; + +describe("useReportChanges", () => { + interface Props { + value: string | undefined; + report: (value: string | undefined) => void; + } + + const renderWith = (initialProps: Props) => + renderHook( + ({ value, report }) => useReportChanges(value, report), + { initialProps } + ); + + it("reports the first value", () => { + const report = jest.fn(); + + renderWith({ value: "first", report }); + + expect(report).toHaveBeenCalledTimes(1); + expect(report).toHaveBeenCalledWith("first"); + }); + + it("reports an initial undefined value", () => { + const report = jest.fn(); + + renderWith({ value: undefined, report }); + + expect(report).toHaveBeenCalledTimes(1); + expect(report).toHaveBeenCalledWith(undefined); + }); + + it("does not report the same value again when only the reporter changed", () => { + const report = jest.fn(); + const { rerender } = renderWith({ value: "same", report }); + + rerender({ value: "same", report: (value) => report(value) }); + rerender({ value: "same", report }); + + expect(report).toHaveBeenCalledTimes(1); + }); + + it("reports a new value once, through the latest reporter", () => { + const first = jest.fn(); + const second = jest.fn(); + const { rerender } = renderWith({ value: "one", report: first }); + + rerender({ value: "two", report: second }); + + expect(first).toHaveBeenCalledTimes(1); + expect(first).toHaveBeenCalledWith("one"); + expect(second).toHaveBeenCalledTimes(1); + expect(second).toHaveBeenCalledWith("two"); + }); + + it("reports a value that comes back, because only the last one is remembered", () => { + const report = jest.fn(); + const { rerender } = renderWith({ value: "one", report }); + + rerender({ value: "two", report }); + rerender({ value: "one", report }); + + expect(report).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/modules/imodel-browser/src/hooks/useReportChanges.ts b/packages/modules/imodel-browser/src/hooks/useReportChanges.ts new file mode 100644 index 00000000..d7fddb7b --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useReportChanges.ts @@ -0,0 +1,26 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import React from "react"; + +/** + * Calls `report(value)` once per distinct value, the first one included. Only this effect writes + * the ref, so `report` can stay a dependency without re-reporting and without a ref-sync effect. + */ +export const useReportChanges = ( + value: TValue, + report: (value: TValue) => void +) => { + const lastReported = React.useRef<{ value: TValue } | undefined>(undefined); + React.useEffect(() => { + const alreadyReported = + lastReported.current !== undefined && + Object.is(lastReported.current.value, value); + if (alreadyReported) { + return; + } + lastReported.current = { value }; + report(value); + }, [value, report]); +}; From 344f11ddfa5319b22e8121a611b0a75571778496 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 15:09:19 -0400 Subject: [PATCH 03/19] feat: add the pure infinite query state machine --- .../src/hooks/infiniteQueryReducer.test.ts | 324 ++++++++++++++++++ .../src/hooks/infiniteQueryReducer.ts | 225 ++++++++++++ 2 files changed, 549 insertions(+) create mode 100644 packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts create mode 100644 packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts new file mode 100644 index 00000000..99eb81c1 --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts @@ -0,0 +1,324 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { DataStatus } from "../types"; +import { + InfiniteQueryPolicy, + InfiniteQueryState, + initialUndecidedState, + reduceInfiniteQuery, +} from "./infiniteQueryReducer"; + +describe("infiniteQueryReducer", () => { + interface Query { + text: string; + scope: string; + } + type State = InfiniteQueryState; + + const fetching: InfiniteQueryPolicy = { + resolveLocally: () => undefined, + decideOnQueryChange: () => "restart", + }; + const keeping: InfiniteQueryPolicy = { + ...fetching, + decideOnQueryChange: () => "keep", + }; + + const query: Query = { text: "", scope: "all" }; + const other: Query = { text: "a", scope: "all" }; + + const reduce = ( + state: State, + action: Parameters>[1], + policy = fetching + ) => reduceInfiniteQuery(state, action, policy); + + const started = (policy = fetching) => + reduce( + initialUndecidedState(query), + { type: "start" }, + policy + ); + + /** Loads the page the state is waiting for, and says so loudly if it waits for none. */ + const loaded = (state: State, items: string[], hasMore: boolean) => { + const request = state.pendingRequest; + if (request === undefined) { + throw new Error("the state has no pending request to load"); + } + return reduce(state, { + type: "pageLoaded", + request, + page: { items, hasMore }, + }); + }; + + describe("initialUndecidedState", () => { + it("has no status and no request", () => { + expect(initialUndecidedState(query)).toEqual({ + query, + status: undefined, + items: [], + hasMore: true, + error: undefined, + totalCount: undefined, + lastRequestedPage: 0, + pendingRequest: undefined, + requestCount: 0, + }); + }); + }); + + describe("start", () => { + it("asks for the first page", () => { + expect(started()).toMatchObject({ + status: DataStatus.Fetching, + items: [], + hasMore: true, + pendingRequest: { id: 1, query, page: 0 }, + requestCount: 1, + }); + }); + + it("is idempotent, so a double dispatch asks once", () => { + const state = started(); + + expect(reduce(state, { type: "start" })).toBe(state); + }); + + it("resolves locally instead of requesting when the policy answers", () => { + const provided: InfiniteQueryPolicy = { + ...fetching, + resolveLocally: () => ({ + status: DataStatus.Complete, + items: ["given"], + }), + }; + + expect(started(provided)).toMatchObject({ + status: DataStatus.Complete, + items: ["given"], + hasMore: false, + pendingRequest: undefined, + requestCount: 0, + }); + }); + + it("resolves a missing precondition with no items", () => { + const noToken: InfiniteQueryPolicy = { + ...fetching, + resolveLocally: () => ({ status: DataStatus.TokenRequired }), + }; + + expect(started(noToken)).toMatchObject({ + status: DataStatus.TokenRequired, + items: [], + hasMore: false, + pendingRequest: undefined, + }); + }); + }); + + describe("queryChanged", () => { + it("only retargets while nothing has been decided", () => { + const undecided = initialUndecidedState(query); + + expect(reduce(undecided, { type: "queryChanged", query: other })).toEqual( + { + ...undecided, + query: other, + } + ); + }); + + it("returns the same state for the same query", () => { + const state = started(); + + expect(reduce(state, { type: "queryChanged", query })).toBe(state); + }); + + it("restarts when the policy says so", () => { + const complete = loaded(started(), ["one"], false); + + expect( + reduce(complete, { type: "queryChanged", query: other }) + ).toMatchObject({ + query: other, + status: DataStatus.Fetching, + items: [], + hasMore: true, + pendingRequest: { id: 2, query: other, page: 0 }, + }); + }); + + it("keeps the items and the pending request when the policy says keep", () => { + const complete = loaded(started(), ["one"], false); + + expect( + reduce(complete, { type: "queryChanged", query: other }, keeping) + ).toEqual({ ...complete, query: other }); + }); + }); + + describe("pageLoaded", () => { + it("replaces the items of the first page", () => { + expect(loaded(started(), ["one"], false)).toMatchObject({ + status: DataStatus.Complete, + items: ["one"], + hasMore: false, + lastRequestedPage: 0, + pendingRequest: undefined, + }); + }); + + it("appends a later page and keeps the earlier total count", () => { + const first = reduce(started(), { + type: "pageLoaded", + request: { id: 1, query, page: 0 }, + page: { items: ["one"], hasMore: true, totalCount: 7 }, + }); + const asking = reduce(first, { type: "fetchNextPage" }); + const second = reduce(asking, { + type: "pageLoaded", + request: { id: 2, query, page: 1 }, + page: { items: ["two"], hasMore: false }, + }); + + expect(second).toMatchObject({ + items: ["one", "two"], + hasMore: false, + totalCount: 7, + lastRequestedPage: 1, + }); + }); + + it("ignores a page that belongs to a superseded request", () => { + const state = started(); + + expect( + reduce(state, { + type: "pageLoaded", + request: { id: 99, query, page: 0 }, + page: { items: ["stale"], hasMore: false }, + }) + ).toBe(state); + }); + + it("clears an earlier error", () => { + const failed = reduce(started(), { + type: "pageFailed", + request: { id: 1, query, page: 0 }, + error: new Error("boom"), + }); + const retrying = reduce(failed, { type: "fetchNextPage" }); + + expect( + reduce(retrying, { + type: "pageLoaded", + request: { id: 2, query, page: 0 }, + page: { items: ["one"], hasMore: false }, + }) + ).toMatchObject({ status: DataStatus.Complete, error: undefined }); + }); + }); + + describe("pageFailed", () => { + it("clears the items of a failed first page and keeps hasMore", () => { + expect( + reduce(started(), { + type: "pageFailed", + request: { id: 1, query, page: 0 }, + error: "boom", + }) + ).toMatchObject({ + status: DataStatus.FetchFailed, + items: [], + hasMore: true, + error: "boom", + pendingRequest: undefined, + }); + }); + + it("keeps the items of earlier pages when a later page fails", () => { + const first = loaded(started(), ["one"], true); + const asking = reduce(first, { type: "fetchNextPage" }); + + expect( + reduce(asking, { + type: "pageFailed", + request: { id: 2, query, page: 1 }, + error: "boom", + }) + ).toMatchObject({ + status: DataStatus.FetchFailed, + items: ["one"], + lastRequestedPage: 1, + }); + }); + + it("ignores a failure that belongs to a superseded request", () => { + const state = started(); + + expect( + reduce(state, { + type: "pageFailed", + request: { id: 99, query, page: 0 }, + error: "boom", + }) + ).toBe(state); + }); + }); + + describe("fetchNextPage", () => { + it("asks for the page after the last one", () => { + const first = loaded(started(), ["one"], true); + + expect(reduce(first, { type: "fetchNextPage" })).toMatchObject({ + status: DataStatus.Complete, + pendingRequest: { id: 2, query, page: 1 }, + }); + }); + + it("is ignored while a request is pending", () => { + const state = started(); + + expect(reduce(state, { type: "fetchNextPage" })).toBe(state); + }); + + it("is ignored once every page is loaded", () => { + const state = loaded(started(), ["one"], false); + + expect(reduce(state, { type: "fetchNextPage" })).toBe(state); + }); + + it("asks for the failed page again instead of leaving a hole", () => { + const first = loaded(started(), ["one"], true); + const asking = reduce(first, { type: "fetchNextPage" }); + const failed = reduce(asking, { + type: "pageFailed", + request: { id: 2, query, page: 1 }, + error: "boom", + }); + + expect(reduce(failed, { type: "fetchNextPage" })).toMatchObject({ + pendingRequest: { id: 3, query, page: 1 }, + }); + }); + }); + + describe("refetch", () => { + it("starts the same query over", () => { + const complete = loaded(started(), ["one"], false); + + expect(reduce(complete, { type: "refetch" })).toMatchObject({ + query, + status: DataStatus.Fetching, + items: [], + hasMore: true, + pendingRequest: { id: 2, query, page: 0 }, + }); + }); + }); +}); diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts new file mode 100644 index 00000000..40c489ac --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts @@ -0,0 +1,225 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { DataStatus } from "../types"; + +/** One page the effect must have in flight. Identified so a late answer can be dropped. */ +export interface PageRequest { + id: number; + query: TQuery; + page: number; +} + +export interface LoadedPage { + items: TItem[]; + hasMore: boolean; + /** Undefined when the response carried no count, which is not zero. */ + totalCount?: number; +} + +/** A settled answer the query needs no request for. */ +export type LocalResolution = + | { status: DataStatus.Complete; items: TItem[] } + | { status: DataStatus.TokenRequired | DataStatus.ContextRequired }; + +export interface InfiniteQueryPolicy { + /** A settled answer that needs no request, or undefined to fetch. */ + resolveLocally: (query: TQuery) => LocalResolution | undefined; + /** Whether the loaded items still answer the new query. */ + decideOnQueryChange: ( + previous: TQuery, + next: TQuery, + loaded: { hasMore: boolean } + ) => "keep" | "restart"; +} + +export interface InfiniteQueryState { + query: TQuery; + /** Undefined only before the first transition. */ + status: DataStatus | undefined; + items: TItem[]; + hasMore: boolean; + error: unknown; + totalCount: number | undefined; + lastRequestedPage: number; + pendingRequest: PageRequest | undefined; + requestCount: number; +} + +export type InfiniteQueryAction = + | { type: "start" } + | { type: "queryChanged"; query: TQuery } + | { + type: "pageLoaded"; + request: PageRequest; + page: LoadedPage; + } + | { type: "pageFailed"; request: PageRequest; error: unknown } + | { type: "fetchNextPage" } + | { type: "refetch" }; + +export const initialUndecidedState = ( + query: TQuery +): InfiniteQueryState => ({ + query, + status: undefined, + items: [], + hasMore: true, + error: undefined, + totalCount: undefined, + lastRequestedPage: 0, + pendingRequest: undefined, + requestCount: 0, +}); + +const answersThePendingRequest = ( + state: InfiniteQueryState, + request: PageRequest +) => state.pendingRequest?.id === request.id; + +const isFirstPage = (request: PageRequest) => + request.page === 0; + +const localItems = (resolution: LocalResolution) => + resolution.status === DataStatus.Complete ? resolution.items : []; + +/** Requests one page, leaving the status alone: a later page loads behind a Complete status. */ +const requestingPage = ( + state: InfiniteQueryState, + page: number +): InfiniteQueryState => { + const id = state.requestCount + 1; + return { + ...state, + pendingRequest: { id, query: state.query, page }, + requestCount: id, + }; +}; + +/** Drops everything loaded and answers the query again, locally when the policy can. */ +const startingOver = ( + state: InfiniteQueryState, + query: TQuery, + policy: InfiniteQueryPolicy +): InfiniteQueryState => { + const settled = { + ...state, + query, + error: undefined, + totalCount: undefined, + lastRequestedPage: 0, + }; + const resolution = policy.resolveLocally(query); + if (resolution !== undefined) { + return { + ...settled, + status: resolution.status, + items: localItems(resolution), + hasMore: false, + pendingRequest: undefined, + }; + } + return requestingPage( + { ...settled, status: DataStatus.Fetching, items: [], hasMore: true }, + 0 + ); +}; + +const nextPageToRequest = ( + state: InfiniteQueryState +) => + state.status === DataStatus.FetchFailed + ? // Ask for the same page again. Advancing would leave a hole where it should have been. + state.lastRequestedPage + : state.lastRequestedPage + 1; + +const reduceQueryChanged = ( + state: InfiniteQueryState, + query: TQuery, + policy: InfiniteQueryPolicy +): InfiniteQueryState => { + if (Object.is(state.query, query)) { + return state; + } + const nothingDecidedYet = state.status === undefined; + if (nothingDecidedYet) { + return { ...state, query }; + } + const decision = policy.decideOnQueryChange(state.query, query, { + hasMore: state.hasMore, + }); + return decision === "keep" + ? { ...state, query } + : startingOver(state, query, policy); +}; + +const reducePageLoaded = ( + state: InfiniteQueryState, + request: PageRequest, + page: LoadedPage +): InfiniteQueryState => { + if (!answersThePendingRequest(state, request)) { + return state; + } + return { + ...state, + status: DataStatus.Complete, + items: isFirstPage(request) ? page.items : [...state.items, ...page.items], + hasMore: page.hasMore, + totalCount: page.totalCount ?? state.totalCount, + error: undefined, + lastRequestedPage: request.page, + pendingRequest: undefined, + }; +}; + +const reducePageFailed = ( + state: InfiniteQueryState, + request: PageRequest, + error: unknown +): InfiniteQueryState => { + if (!answersThePendingRequest(state, request)) { + return state; + } + // hasMore is left alone, so a query change on a failed query still restarts it. + return { + ...state, + status: DataStatus.FetchFailed, + items: isFirstPage(request) ? [] : state.items, + error, + lastRequestedPage: request.page, + pendingRequest: undefined, + }; +}; + +const reduceFetchNextPage = ( + state: InfiniteQueryState +): InfiniteQueryState => { + const busyOrDone = state.pendingRequest !== undefined || !state.hasMore; + return busyOrDone ? state : requestingPage(state, nextPageToRequest(state)); +}; + +/** Every no-op returns the state it was given, so React bails out without a render. */ +export const reduceInfiniteQuery = ( + state: InfiniteQueryState, + action: InfiniteQueryAction, + policy: InfiniteQueryPolicy +): InfiniteQueryState => { + switch (action.type) { + case "start": + return state.status === undefined + ? startingOver(state, state.query, policy) + : state; + case "queryChanged": + return reduceQueryChanged(state, action.query, policy); + case "pageLoaded": + return reducePageLoaded(state, action.request, action.page); + case "pageFailed": + return reducePageFailed(state, action.request, action.error); + case "fetchNextPage": + return reduceFetchNextPage(state); + case "refetch": + return startingOver(state, state.query, policy); + } +}; From 44877fd8cb973c3e320d2ec2d69fe8c1dfa5d5cf Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 16:13:04 -0400 Subject: [PATCH 04/19] chore: satisfy the repo's default-case rule on the exhaustive switch --- .../modules/imodel-browser/src/hooks/infiniteQueryReducer.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts index 40c489ac..09987480 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts @@ -221,5 +221,6 @@ export const reduceInfiniteQuery = ( return reduceFetchNextPage(state); case "refetch": return startingOver(state, state.query, policy); + // no default } }; From 1de776a3e895286f6b41423b3ca85fcf6d6d2690 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 23:49:32 -0400 Subject: [PATCH 05/19] feat: add the useInfiniteQuery core hook --- .../src/hooks/useInfiniteQuery.test.ts | 252 ++++++++++++++++++ .../src/hooks/useInfiniteQuery.ts | 117 ++++++++ 2 files changed, 369 insertions(+) create mode 100644 packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts create mode 100644 packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts new file mode 100644 index 00000000..2c18c65f --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts @@ -0,0 +1,252 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { act, renderHook } from "@testing-library/react-hooks"; + +import { deferred } from "../tests/helpers"; +import { DataStatus } from "../types"; +import { LoadedPage, PageRequest } from "./infiniteQueryReducer"; +import { useInfiniteQuery, UseInfiniteQueryOptions } from "./useInfiniteQuery"; + +describe("useInfiniteQuery", () => { + interface Query { + text: string; + } + type Options = UseInfiniteQueryOptions; + + const restarting = { + resolveLocally: () => undefined, + decideOnQueryChange: () => "restart" as const, + }; + + const renderWith = (initialProps: Options) => + renderHook>>( + (props) => useInfiniteQuery(props), + { initialProps } + ); + + const page = (items: string[], hasMore = false): LoadedPage => ({ + items, + hasMore, + }); + + it("has no status on the first render, then fetches the first page", async () => { + const fetchPage = jest.fn(async () => page(["one"])); + + const { result, waitForNextUpdate } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + expect(result.all[0]).toHaveProperty("status", undefined); + await waitForNextUpdate(); + + expect(result.current.status).toEqual(DataStatus.Complete); + expect(result.current.items).toEqual(["one"]); + expect(fetchPage).toHaveBeenCalledTimes(1); + expect(fetchPage.mock.calls[0][0]).toMatchObject({ page: 0 }); + }); + + it("resolves locally without a request", async () => { + const fetchPage = jest.fn(async () => page(["never"])); + + const { result } = renderWith({ + query: { text: "" }, + fetchPage, + decideOnQueryChange: () => "restart", + resolveLocally: () => ({ status: DataStatus.TokenRequired }), + }); + await act(async () => undefined); + + expect(result.current.status).toEqual(DataStatus.TokenRequired); + expect(result.current.hasMore).toBe(false); + expect(fetchPage).not.toHaveBeenCalled(); + }); + + it("aborts the in-flight request when the query changes", async () => { + const signals: AbortSignal[] = []; + const neverSettles = deferred>(); + const fetchPage = jest.fn( + async (_request: PageRequest, signal: AbortSignal) => { + signals.push(signal); + return neverSettles.promise; + } + ); + const first: Query = { text: "" }; + const second: Query = { text: "next" }; + + const { rerender } = renderWith({ query: first, fetchPage, ...restarting }); + await act(async () => undefined); + // Still in flight, so nothing has touched its signal yet. + expect(signals).toHaveLength(1); + expect(signals[0].aborted).toBe(false); + + rerender({ query: second, fetchPage, ...restarting }); + await act(async () => undefined); + + expect(signals).toHaveLength(2); + expect(signals[0].aborted).toBe(true); + expect(signals[1].aborted).toBe(false); + }); + + it("ignores a page that arrives after its query was replaced", async () => { + const stale = deferred>(); + const live = deferred>(); + const pages = [stale.promise, live.promise]; + const fetchPage = jest.fn( + () => pages.shift() ?? Promise.reject(new Error("unexpected request")) + ); + + const { result, rerender, waitFor } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + rerender({ query: { text: "next" }, fetchPage, ...restarting }); + stale.resolve(page(["stale"])); + live.resolve(page(["live"])); + await waitFor(() => expect(result.current.items).toEqual(["live"])); + await act(async () => undefined); + + expect(result.current.items).toEqual(["live"]); + }); + + it("ignores a failure that arrives after its query was replaced", async () => { + const stale = deferred>(); + const live = deferred>(); + const pages = [stale.promise, live.promise]; + const fetchPage = jest.fn( + () => pages.shift() ?? Promise.reject(new Error("unexpected request")) + ); + + const { result, rerender, waitFor } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + rerender({ query: { text: "next" }, fetchPage, ...restarting }); + stale.reject(new Error("stale failure")); + live.resolve(page(["live"])); + await waitFor(() => expect(result.current.items).toEqual(["live"])); + await act(async () => undefined); + + expect(result.current.status).toEqual(DataStatus.Complete); + expect(result.current.error).toBeUndefined(); + }); + + it("keeps the items when the policy keeps the query", async () => { + const fetchPage = jest.fn(async () => page(["one"])); + + const { result, rerender, waitForNextUpdate } = renderWith({ + query: { text: "" }, + fetchPage, + resolveLocally: () => undefined, + decideOnQueryChange: () => "keep", + }); + await waitForNextUpdate(); + rerender({ + query: { text: "next" }, + fetchPage, + resolveLocally: () => undefined, + decideOnQueryChange: () => "keep", + }); + await act(async () => undefined); + + expect(fetchPage).toHaveBeenCalledTimes(1); + expect(result.current.items).toEqual(["one"]); + }); + + it("keeps one identity for fetchNextPage and refetch", async () => { + const fetchPage = jest.fn(async () => page(["one"], true)); + + const { result, rerender, waitForNextUpdate } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + await waitForNextUpdate(); + const { fetchNextPage, refetch } = result.current; + rerender({ + query: { text: "" }, + fetchPage: async () => page([]), + ...restarting, + }); + + expect(result.current.fetchNextPage).toBe(fetchNextPage); + expect(result.current.refetch).toBe(refetch); + }); + + it("does not restart a request when only fetchPage's identity changed", async () => { + const calls: number[] = []; + const countingFetchPage = () => async (request: PageRequest) => { + calls.push(request.page); + return page(["one"], true); + }; + // One query object, reused. The hook's contract is that the query is memoized, so holding it + // fixed is what isolates a change of fetchPage's identity on its own. + const query: Query = { text: "" }; + + const { rerender, waitForNextUpdate } = renderWith({ + query, + fetchPage: countingFetchPage(), + ...restarting, + }); + await waitForNextUpdate(); + rerender({ query, fetchPage: countingFetchPage(), ...restarting }); + rerender({ query, fetchPage: countingFetchPage(), ...restarting }); + await act(async () => undefined); + + expect(calls).toEqual([0]); + }); + + it("fetches the next page and appends it", async () => { + const fetchPage = jest.fn(async (request: PageRequest) => + page([`page${request.page}`], request.page === 0) + ); + + const { result, waitForNextUpdate, waitFor } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + await waitForNextUpdate(); + act(() => result.current.fetchNextPage()); + await waitFor(() => expect(result.current.hasMore).toBe(false)); + + expect(result.current.items).toEqual(["page0", "page1"]); + }); + + it("refetches the same query from the first page", async () => { + let round = 0; + const fetchPage = jest.fn(async () => page([`round${(round += 1)}`])); + + const { result, waitForNextUpdate, waitFor } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + await waitForNextUpdate(); + act(() => result.current.refetch()); + await waitFor(() => expect(result.current.items).toEqual(["round2"])); + + expect(fetchPage).toHaveBeenCalledTimes(2); + }); + + it("reports a failure and stops fetching", async () => { + const fetchPage = jest.fn(async () => { + throw new Error("boom"); + }); + + const { result, waitForValueToChange } = renderWith({ + query: { text: "" }, + fetchPage, + ...restarting, + }); + await waitForValueToChange(() => result.current.status); + + expect(result.current.status).toEqual(DataStatus.FetchFailed); + expect(result.current.error).toEqual(new Error("boom")); + expect(result.current.isFetching).toBe(false); + }); +}); diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts new file mode 100644 index 00000000..b9126b8d --- /dev/null +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts @@ -0,0 +1,117 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import React from "react"; + +import { DataStatus } from "../types"; +import { + InfiniteQueryAction, + InfiniteQueryPolicy, + InfiniteQueryState, + initialUndecidedState, + LoadedPage, + PageRequest, + reduceInfiniteQuery, +} from "./infiniteQueryReducer"; +import { useEventCallback } from "./useEventCallback"; + +export interface UseInfiniteQueryOptions + extends InfiniteQueryPolicy { + /** MUST be memoized. A new object every render restarts the query. */ + query: TQuery; + /** Resolves with one page, or rejects with what the source answered. */ + fetchPage: ( + request: PageRequest, + signal: AbortSignal + ) => Promise>; +} + +export interface InfiniteQueryResult { + items: TItem[]; + /** Undefined on the first render, before anything has been decided. */ + status: DataStatus | undefined; + /** Whether a page is in flight, which a later page does not show in the status. */ + isFetching: boolean; + hasMore: boolean; + error: unknown; + totalCount: number | undefined; + fetchNextPage: () => void; + refetch: () => void; +} + +const isAbortError = (error: unknown) => + error instanceof Error && error.name === "AbortError"; + +/** + * Pages a query, one request at a time, and drops the answers of superseded requests. The state + * machine in `infiniteQueryReducer` is the single source of truth; the two effects here only start + * the query and keep the pending request in flight. + */ +export const useInfiniteQuery = ({ + query, + fetchPage, + resolveLocally, + decideOnQueryChange, +}: UseInfiniteQueryOptions): InfiniteQueryResult => { + const reduce = ( + current: InfiniteQueryState, + action: InfiniteQueryAction + ) => + reduceInfiniteQuery(current, action, { + resolveLocally, + decideOnQueryChange, + }); + const [state, dispatch] = React.useReducer(reduce, query, (initial: TQuery) => + initialUndecidedState(initial) + ); + + // The query prop moved ahead of the state. Telling the reducer during render makes React + // re-render with the reduced state before anything is committed: its documented way to adjust + // state when a prop changes, with no effect and no intermediate commit. + if (state.query !== query) { + dispatch({ type: "queryChanged", query }); + } + + React.useEffect(() => dispatch({ type: "start" }), []); + + const runFetchPage = useEventCallback(fetchPage); + React.useEffect(() => { + const request = state.pendingRequest; + if (request === undefined) { + return; + } + const controller = new AbortController(); + void runFetchPage(request, controller.signal).then( + (page) => { + if (!controller.signal.aborted) { + dispatch({ type: "pageLoaded", request, page }); + } + }, + (error: unknown) => { + if (controller.signal.aborted || isAbortError(error)) { + return; + } + dispatch({ type: "pageFailed", request, error }); + } + ); + return () => controller.abort(); + }, [state.pendingRequest, runFetchPage]); + + const fetchNextPage = React.useCallback( + () => dispatch({ type: "fetchNextPage" }), + [] + ); + const refetch = React.useCallback(() => dispatch({ type: "refetch" }), []); + + return { + items: state.items, + status: state.status, + isFetching: state.pendingRequest !== undefined, + hasMore: state.hasMore, + error: state.error, + totalCount: state.totalCount, + fetchNextPage, + refetch, + }; +}; From 3c013c7bab109eaaf911000685a9284ba6d20d34 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 23:55:07 -0400 Subject: [PATCH 06/19] test: pin the pending request's identity across a keep --- .../src/hooks/infiniteQueryReducer.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts index 99eb81c1..7277fb81 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts @@ -160,6 +160,21 @@ describe("infiniteQueryReducer", () => { reduce(complete, { type: "queryChanged", query: other }, keeping) ).toEqual({ ...complete, query: other }); }); + + it("carries the in-flight request object through unchanged when keeping", () => { + const inFlight = started(); + + const kept = reduce( + inFlight, + { type: "queryChanged", query: other }, + keeping + ); + + // Identity, not equality: useInfiniteQuery uses this object as an effect dependency, so a + // rebuilt-but-equal request would abort and restart a request that is still in flight. + expect(kept.pendingRequest).toBe(inFlight.pendingRequest); + expect(kept.query).toBe(other); + }); }); describe("pageLoaded", () => { From d7dc1416fc85f7192142bf485802a2c0c79f08c9 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 4 Sep 2026 23:57:50 -0400 Subject: [PATCH 07/19] feat: extract the iTwins page request into iTwinsApi --- .../containers/ITwinGrid/iTwinsApi.test.ts | 212 ++++++++++++++++++ .../src/containers/ITwinGrid/iTwinsApi.ts | 98 ++++++++ 2 files changed, 310 insertions(+) create mode 100644 packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts create mode 100644 packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts new file mode 100644 index 00000000..47be2564 --- /dev/null +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts @@ -0,0 +1,212 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { rest } from "msw"; + +import { server } from "../../tests/mocks/server"; +import { ITwinFull } from "../../types"; +import { + buildITwinsPageUrl, + fetchITwinsPage, + ITwinQueryKey, + ITWINS_PAGE_SIZE, +} from "./iTwinsApi"; + +describe("iTwinsApi", () => { + const baseQuery: ITwinQueryKey = { + requestType: "", + filterText: "", + iTwinSubClass: "Project", + orderby: undefined, + credentialKey: "accessToken", + }; + + describe("buildITwinsPageUrl", () => { + it("pages the default request", () => { + expect(buildITwinsPageUrl({ query: baseQuery, page: 2 })).toEqual( + "https://api.bentley.com/itwins/?subClass=Project&$skip=200&$top=100" + ); + }); + + it("uses the endpoint of a client side filtered request type", () => { + expect( + buildITwinsPageUrl({ + query: { ...baseQuery, requestType: "favorites" }, + page: 0, + }) + ).toContain("/itwins/favorites?subClass=Project"); + }); + + it("sends an empty subClass for All", () => { + expect( + buildITwinsPageUrl({ + query: { ...baseQuery, iTwinSubClass: "All" }, + page: 0, + }) + ).toContain("?subClass=&"); + }); + + it("uses the server environment prefix", () => { + expect( + buildITwinsPageUrl({ + query: { ...baseQuery, serverEnvironmentPrefix: "dev" }, + page: 0, + }) + ).toContain("https://dev-api.bentley.com/itwins/"); + }); + + it("searches with the filter text, encoded and trimmed", () => { + expect( + buildITwinsPageUrl({ + query: { ...baseQuery, filterText: " a b+c " }, + page: 0, + }) + ).toContain("&$search=a%20b%2Bc"); + }); + + it("orders with the orderby, encoded", () => { + expect( + buildITwinsPageUrl({ + query: { ...baseQuery, orderby: "displayName ASC" }, + page: 0, + }) + ).toContain("&$orderby=displayName%20ASC"); + }); + + it("drops search and orderby for a client side filtered request type", () => { + const url = buildITwinsPageUrl({ + query: { + ...baseQuery, + requestType: "recents", + filterText: "ignored", + orderby: "displayName ASC", + }, + page: 0, + }); + + expect(url).not.toContain("$search"); + expect(url).not.toContain("$orderby"); + }); + }); + + describe("fetchITwinsPage", () => { + const iTwins: ITwinFull[] = [{ id: "alpha", displayName: "alpha" }]; + const requestWatcher = jest.fn(); + + beforeAll(() => server.listen()); + afterEach(() => { + server.resetHandlers(); + jest.clearAllMocks(); + }); + afterAll(() => server.close()); + + const pinResponse = ({ + status = 200, + body = { iTwins }, + totalCount, + text, + }: { + status?: number; + body?: unknown; + totalCount?: string; + text?: string; + }) => + server.use( + rest.get("https://api.bentley.com/itwins/", (req, res, ctx) => { + requestWatcher({ + authorization: req.headers.get("Authorization"), + cacheControl: req.headers.get("Cache-Control"), + accept: req.headers.get("Accept"), + prefer: req.headers.get("Prefer"), + totalCount: req.headers.get("x-total-count"), + }); + return res( + ctx.status(status), + ...(text === undefined ? [ctx.json(body)] : [ctx.text(text)]), + ...(totalCount === undefined + ? [] + : [ctx.set("x-total-count", totalCount)]) + ); + }) + ); + + const fetchWith = ( + overrides: Partial[0]> = {} + ) => + fetchITwinsPage({ + query: baseQuery, + page: 0, + accessToken: "accessToken", + bypassCache: false, + signal: new AbortController().signal, + ...overrides, + }); + + it("returns the page and no total count when the response carries none", async () => { + pinResponse({}); + + await expect(fetchWith()).resolves.toEqual({ + items: iTwins, + hasMore: false, + totalCount: undefined, + }); + }); + + it("reads the total count header", async () => { + pinResponse({ totalCount: "42" }); + + await expect(fetchWith()).resolves.toMatchObject({ totalCount: 42 }); + }); + + it("has more pages when the page is full", async () => { + const fullPage = Array.from( + { length: ITWINS_PAGE_SIZE }, + (_unused, index) => ({ id: `id${index}` }) + ); + pinResponse({ body: { iTwins: fullPage } }); + + await expect(fetchWith()).resolves.toMatchObject({ hasMore: true }); + }); + + it("sends the platform headers and no Cache-Control by default", async () => { + pinResponse({}); + + await fetchWith(); + + expect(requestWatcher).toHaveBeenCalledWith({ + authorization: "accessToken", + cacheControl: null, + accept: "application/vnd.bentley.itwin-platform.v1+json", + prefer: "return=representation", + totalCount: "true", + }); + }); + + it("sends no-cache when told to bypass the cache", async () => { + pinResponse({}); + + await fetchWith({ bypassCache: true }); + + expect(requestWatcher).toHaveBeenCalledWith( + expect.objectContaining({ cacheControl: "no-cache" }) + ); + }); + + it("resolves a token provider at request time", async () => { + pinResponse({}); + + await fetchWith({ accessToken: async () => "fresh" }); + + expect(requestWatcher).toHaveBeenCalledWith( + expect.objectContaining({ authorization: "fresh" }) + ); + }); + + it("throws what the api answered", async () => { + pinResponse({ status: 401, text: "no soup for you" }); + + await expect(fetchWith()).rejects.toEqual(new Error("no soup for you")); + }); + }); +}); diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts new file mode 100644 index 00000000..99a84367 --- /dev/null +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts @@ -0,0 +1,98 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import { LoadedPage } from "../../hooks/infiniteQueryReducer"; +import { AccessTokenProvider, ITwinDataQuery, ITwinFull } from "../../types"; +import { _getAPIServer } from "../../utils/_apiOverrides"; + +export const ITWINS_PAGE_SIZE = 100; + +/** Favorites and recents come whole and are filtered in the browser. */ +export const isClientSideFiltered = ( + requestType: ITwinDataQuery["requestType"] +) => requestType === "favorites" || requestType === "recents"; + +/** Everything that identifies a request, so two equal keys mean one query. */ +export interface ITwinQueryKey extends ITwinDataQuery { + /** Undefined without a credential, the token itself for a string, "provider" for a function. */ + credentialKey: string | undefined; + serverEnvironmentPrefix?: "" | "dev" | "qa"; + providedData?: ITwinFull[]; +} + +export const buildITwinsPageUrl = ({ + query, + page, +}: { + query: ITwinQueryKey; + page: number; +}) => { + const { requestType, filterText, iTwinSubClass, orderby } = query; + const clientSideFiltered = isClientSideFiltered(requestType); + const endpoint = clientSideFiltered ? requestType : ""; + const subClass = `?subClass=${iTwinSubClass === "All" ? "" : iTwinSubClass}`; + const paging = `&$skip=${page * ITWINS_PAGE_SIZE}&$top=${ITWINS_PAGE_SIZE}`; + // Hand-built rather than URLSearchParams, which would encode the $ and turn spaces into +. + const search = + clientSideFiltered || !filterText + ? "" + : `&$search=${encodeURIComponent(filterText.trim())}`; + const ordering = + clientSideFiltered || !orderby + ? "" + : `&$orderby=${encodeURIComponent(orderby.trim())}`; + const server = _getAPIServer(query.serverEnvironmentPrefix); + return `${server}/itwins/${endpoint}${subClass}${paging}${search}${ordering}`; +}; + +export interface FetchITwinsPageOptions { + query: ITwinQueryKey; + page: number; + accessToken: AccessTokenProvider; + bypassCache: boolean; + signal: AbortSignal; +} + +const platformHeaders = async ({ + accessToken, + bypassCache, +}: { + accessToken: AccessTokenProvider; + bypassCache: boolean; +}): Promise> => ({ + Authorization: + typeof accessToken === "function" ? await accessToken() : accessToken, + Accept: "application/vnd.bentley.itwin-platform.v1+json", + Prefer: "return=representation", + "x-total-count": "true", + ...(bypassCache ? { "Cache-Control": "no-cache" } : {}), +}); + +/** + * One page of iTwins, or a rejection carrying what the API answered. A totalCount of undefined + * means the response had no count, which is not zero. + */ +export const fetchITwinsPage = async ({ + query, + page, + accessToken, + bypassCache, + signal, +}: FetchITwinsPageOptions): Promise> => { + const response = await fetch(buildITwinsPageUrl({ query, page }), { + signal, + headers: await platformHeaders({ accessToken, bypassCache }), + }); + if (!response.ok) { + throw new Error(await response.text()); + } + const { iTwins }: { iTwins: ITwinFull[] } = await response.json(); + const totalCountHeader = response.headers.get("x-total-count"); + return { + items: iTwins, + hasMore: iTwins.length === ITWINS_PAGE_SIZE, + totalCount: + totalCountHeader !== null ? Number(totalCountHeader) : undefined, + }; +}; From 005ec99eacd8e996facfc6d4827aef6dbb0f220e Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Sat, 5 Sep 2026 00:01:18 -0400 Subject: [PATCH 08/19] refactor: rebuild useITwinData on the shared infinite query core --- .../src/containers/ITwinGrid/useITwinData.ts | 363 +++++++----------- .../containers/ITwinGrid/useITwinDataState.ts | 165 -------- 2 files changed, 130 insertions(+), 398 deletions(-) delete mode 100644 packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinDataState.ts diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index 8439c01c..d1fa9724 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -5,17 +5,25 @@ import React from "react"; import { useLogger } from "../../contexts/LoggerContext"; +import { LocalResolution, PageRequest } from "../../hooks/infiniteQueryReducer"; +import { useInfiniteQuery } from "../../hooks/useInfiniteQuery"; +import { useReportChanges } from "../../hooks/useReportChanges"; import { AccessTokenProvider, ApiOverrides, + DataStatus, ITwinDataQuery, ITwinDataState, ITwinFilterOptions, ITwinFull, ITwinSubClass, } from "../../types"; -import { _getAPIServer } from "../../utils/_apiOverrides"; -import { useITwinDataState } from "./useITwinDataState"; +import { + fetchITwinsPage, + isClientSideFiltered, + ITwinQueryKey, +} from "./iTwinsApi"; +import { useITwinFilter } from "./useITwinFilter"; export interface ProjectDataHookOptions { requestType?: "favorites" | "recents" | ""; @@ -29,10 +37,65 @@ export interface ProjectDataHookOptions { onDataStateChange?: (state: ITwinDataState) => void; } -const PAGE_SIZE = 100; +/** + * Identifies the credential without holding it. An inline `async () => token` provider is the + * documented way to keep a token fresh and changes identity on every render, so keying on the + * function itself would restart a settled query forever. The provider is read at request time. + */ +const toCredentialKey = (accessToken?: AccessTokenProvider) => { + if (typeof accessToken === "function") { + return "provider"; + } + // An empty token reads as no credential, as it does today, so `??` would not do. + if (accessToken === undefined || accessToken === "") { + return undefined; + } + return accessToken; +}; -const isClientSideFiltered = (requestType: string) => - ["favorites", "recents"].includes(requestType); +/** Provided data wins over a missing token, which is the precedence the grid ships with. */ +const resolveITwinQueryLocally = ( + query: ITwinQueryKey +): LocalResolution | undefined => { + if (query.providedData !== undefined) { + return { status: DataStatus.Complete, items: query.providedData }; + } + if (query.credentialKey === undefined) { + return { status: DataStatus.TokenRequired }; + } + return undefined; +}; + +const differsOnlyByFilterText = (a: ITwinQueryKey, b: ITwinQueryKey) => + a.requestType === b.requestType && + a.iTwinSubClass === b.iTwinSubClass && + a.orderby === b.orderby && + a.credentialKey === b.credentialKey && + a.serverEnvironmentPrefix === b.serverEnvironmentPrefix && + a.providedData === b.providedData; + +/** + * Favorites and recents are filtered in the browser, so once every page is loaded a new filter + * text is answered by the iTwins in hand. Anything else restarts the query. + */ +const decideOnITwinQueryChange = ( + previous: ITwinQueryKey, + next: ITwinQueryKey, + loaded: { hasMore: boolean } +) => + isClientSideFiltered(next.requestType) && + differsOnlyByFilterText(previous, next) && + !loaded.hasMore + ? "keep" + : "restart"; + +/** The core requests a page only after `resolveITwinQueryLocally` accepted the credential. */ +const requireAccessToken = (accessToken?: AccessTokenProvider) => { + if (accessToken === undefined || accessToken === "") { + throw new Error("A page was requested without an access token"); + } + return accessToken; +}; export const useITwinData = ({ requestType = "", @@ -46,12 +109,11 @@ export const useITwinData = ({ onDataStateChange, }: ProjectDataHookOptions) => { const logger = useLogger(); - const data = apiOverrides?.data; + const credentialKey = toCredentialKey(accessToken); + const providedData = apiOverrides?.data; const serverEnvironmentPrefix = apiOverrides?.serverEnvironmentPrefix; - const [totalCount, setTotalCount] = React.useState(); - const [page, setPage] = React.useState(0); - const query = React.useMemo( + const dataQuery = React.useMemo( () => ({ requestType, filterText: filterOptions ?? "", @@ -60,241 +122,76 @@ export const useITwinData = ({ }), [requestType, filterOptions, iTwinSubClass, orderbyOptions] ); - const { - status, - iTwins, - hasMore, - reset, - applyQuery, - markFetching, - pageLoaded, - pageFailed, - tokenRequired, - dataProvided, - } = useITwinDataState(query, onDataStateChange); - - const resetData = React.useCallback(() => { - reset(); - setTotalCount(undefined); - setPage(0); - fetchingMoreRef.current = true; - lastPageFailedRef.current = false; - }, [reset]); - - // We start in a fetching state - const fetchingMoreRef = React.useRef(true); - const lastPageFailedRef = React.useRef(false); - const [retryCount, setRetryCount] = React.useState(0); - const fetchMore = React.useCallback(() => { - if (fetchingMoreRef.current) { - return; - } - fetchingMoreRef.current = true; - if (lastPageFailedRef.current) { - // Ask for the same page again. Advancing would leave a hole where it should have been. - lastPageFailedRef.current = false; - setRetryCount((count) => count + 1); - return; - } - setPage((page) => page + 1); - }, []); - - // counter to force a new request when resetting the existing state would not change an effect dependency - const [refetchCount, setRefetchCount] = React.useState(0); - const refetchITwins = React.useCallback(() => { - resetData(); - setRefetchCount((count) => count + 1); - }, [resetData]); - - const activeRequestRef = React.useRef(undefined); - - const morePagesRef = React.useRef(hasMore); - React.useEffect(() => { - morePagesRef.current = hasMore; - }, [hasMore]); + const queryKey = React.useMemo( + () => ({ + ...dataQuery, + credentialKey, + serverEnvironmentPrefix, + providedData, + }), + [dataQuery, credentialKey, serverEnvironmentPrefix, providedData] + ); - React.useEffect(() => { - // If filter changes but we already have all the data for favorites or recents, - // let client side filtering do its job, otherwise, refetch from scratch. - // Use ref so "morePages" changes itself does not trigger the effect. - if (morePagesRef.current || !isClientSideFiltered(requestType)) { - resetData(); - } else { - applyQuery(query); + const fetchPage = async ( + request: PageRequest, + signal: AbortSignal + ) => { + const forFavorites = request.query.requestType === "favorites"; + const page = await fetchITwinsPage({ + query: request.query, + page: request.page, + accessToken: requireAccessToken(accessToken), + bypassCache: forFavorites && Boolean(shouldRefetchFavorites), + signal, + }); + if (forFavorites && !signal.aborted) { + resetShouldRefetchFavorites?.(); } - }, [query, requestType, resetData, applyQuery]); - - React.useEffect(() => { - // If any of the dependencies change, always restart the fetch from scratch. - resetData(); - }, [ - accessToken, - requestType, - iTwinSubClass, - orderbyOptions, - data, - serverEnvironmentPrefix, - resetData, - ]); + return page; + }; - React.useEffect(() => { - if (!hasMore) { - return; - } - if (data) { - dataProvided(data); - return; - } - if (!accessToken) { - tokenRequired(); - return; - } - if (page === 0) { - markFetching(); - } - const requestId = Symbol(); - activeRequestRef.current = requestId; - const { abortController, fetchITwins } = createFetchITwinsFn({ - query, - accessToken, - page, - serverEnvironmentPrefix, - shouldRefetchFavorites, + const { items, status, hasMore, error, totalCount, fetchNextPage, refetch } = + useInfiniteQuery({ + query: queryKey, + fetchPage, + resolveLocally: resolveITwinQueryLocally, + decideOnQueryChange: decideOnITwinQueryChange, }); - const applyResult = async () => { - const result = await fetchITwins(); - if (activeRequestRef.current !== requestId) { - return; - } - if (result.totalCount !== undefined) { - setTotalCount(result.totalCount); + const iTwins = useITwinFilter(items, dataQuery.filterText); + const dataState = React.useMemo( + () => + status === undefined + ? undefined + : { query: dataQuery, status, iTwins, hasMore, error }, + [status, dataQuery, iTwins, hasMore, error] + ); + + const reportDataState = React.useCallback( + (state: ITwinDataState | undefined) => { + if (state !== undefined) { + onDataStateChange?.(state); } - fetchingMoreRef.current = false; - requestType === "favorites" && resetShouldRefetchFavorites?.(); - pageLoaded({ - iTwins: result.iTwins, - isFirstPage: page === 0, - hasMore: result.hasMore, - }); - }; + }, + [onDataStateChange] + ); + useReportChanges(dataState, reportDataState); - applyResult().catch((e) => { - if (activeRequestRef.current !== requestId || e.name === "AbortError") { - // Superseded or aborted, not a failure worth reporting. - return; + const reportFailure = React.useCallback( + (failure: unknown) => { + if (failure !== undefined) { + logger.logError("Failed to fetch iTwins", failure); } - fetchingMoreRef.current = false; - lastPageFailedRef.current = true; - pageFailed({ error: e, isFirstPage: page === 0 }); - logger.logError("Failed to fetch iTwins", e); - }); - return () => { - activeRequestRef.current = undefined; - abortController.abort(); - }; - }, [ - accessToken, - requestType, - data, - serverEnvironmentPrefix, - query, - page, - hasMore, - refetchCount, - retryCount, - shouldRefetchFavorites, - resetShouldRefetchFavorites, - logger, - dataProvided, - tokenRequired, - markFetching, - pageLoaded, - pageFailed, - ]); + }, + [logger] + ); + useReportChanges(error, reportFailure); + return { iTwins, status, totalCount, - fetchMore: hasMore ? fetchMore : undefined, - refetchITwins, + fetchMore: hasMore ? fetchNextPage : undefined, + refetchITwins: refetch, }; }; - -/** - * Builds the request for one page of iTwins. Resolves with the page, or throws what the API - * answered. A totalCount of undefined means the response carried no count, which is not zero. - */ -const createFetchITwinsFn = ({ - query, - accessToken, - page, - serverEnvironmentPrefix, - shouldRefetchFavorites, -}: { - query: ITwinDataQuery; - accessToken: AccessTokenProvider; - page: number; - serverEnvironmentPrefix?: "" | "dev" | "qa"; - shouldRefetchFavorites?: boolean; -}): { - abortController: AbortController; - fetchITwins: () => Promise<{ - iTwins: ITwinFull[]; - totalCount: number | undefined; - hasMore: boolean; - }>; -} => { - const { requestType, filterText, iTwinSubClass, orderby } = query; - const clientSideFiltered = isClientSideFiltered(requestType); - const endpoint = clientSideFiltered ? requestType : ""; - const subClass = `?subClass=${iTwinSubClass === "All" ? "" : iTwinSubClass}`; - const paging = `&$skip=${page * PAGE_SIZE}&$top=${PAGE_SIZE}`; - const search = - clientSideFiltered || !filterText - ? "" - : `&$search=${encodeURIComponent(filterText.trim())}`; - const ordering = - clientSideFiltered || !orderby - ? "" - : `&$orderby=${encodeURIComponent(orderby.trim())}`; - - const abortController = new AbortController(); - const url = `${_getAPIServer( - serverEnvironmentPrefix - )}/itwins/${endpoint}${subClass}${paging}${search}${ordering}`; - - const doFetchRequest = async () => { - const options: RequestInit = { - signal: abortController.signal, - headers: { - "Cache-Control": - requestType === "favorites" && shouldRefetchFavorites - ? "no-cache" - : "", - Authorization: - typeof accessToken === "function" ? await accessToken() : accessToken, - Accept: "application/vnd.bentley.itwin-platform.v1+json", - Prefer: "return=representation", - "x-total-count": "true", - }, - }; - - const response = await fetch(url, options); - const result: { iTwins: ITwinFull[] } = response.ok - ? await response.json() - : await response.text().then((errorText) => { - throw new Error(errorText); - }); - - const totalCountHeader = response.headers.get("x-total-count"); - return { - iTwins: result.iTwins, - totalCount: - totalCountHeader !== null ? Number(totalCountHeader) : undefined, - hasMore: result.iTwins.length === PAGE_SIZE, - }; - }; - - return { abortController, fetchITwins: doFetchRequest }; -}; diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinDataState.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinDataState.ts deleted file mode 100644 index 241e96ee..00000000 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinDataState.ts +++ /dev/null @@ -1,165 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Bentley Systems, Incorporated. All rights reserved. - * See LICENSE.md in the project root for license terms and full copyright notice. - *--------------------------------------------------------------------------------------------*/ -import React from "react"; - -import { - DataStatus, - ITwinDataQuery, - ITwinDataState, - ITwinFull, -} from "../../types"; -import { useITwinFilter } from "./useITwinFilter"; - -/** One object, so a render can never pair one query's status with another query's iTwins. */ -interface FetchState extends Omit { - /** Every page fetched, where a report carries only what client side filtering kept. */ - iTwins: ITwinFull[]; - /** Undefined until the first transition, which is what postProcessCallback sees on first render. */ - status: DataStatus | undefined; -} - -const startingOver = (query: ITwinDataQuery): FetchState => ({ - query, - status: DataStatus.Fetching, - iTwins: [], - hasMore: true, - error: undefined, -}); - -const nothingDecidedYet = (query: ITwinDataQuery): FetchState => ({ - ...startingOver(query), - status: undefined, -}); - -const sameQuery = (a: ITwinDataQuery, b: ITwinDataQuery) => - a.requestType === b.requestType && - a.filterText === b.filterText && - a.iTwinSubClass === b.iTwinSubClass && - a.orderby === b.orderby; - -const hasStartedOver = (state: FetchState, query: ITwinDataQuery) => - sameQuery(state.query, query) && - state.status === DataStatus.Fetching && - state.iTwins.length === 0 && - state.hasMore; - -/** - * @param query MUST be memoized. A new object every render reports the same state again on every - * render. - */ -export const useITwinDataState = ( - query: ITwinDataQuery, - onDataStateChange?: (state: ITwinDataState) => void -) => { - const [fetchState, setFetchState] = React.useState(() => - nothingDecidedYet(query) - ); - const iTwins = useITwinFilter(fetchState.iTwins, query.filterText); - - const queryRef = React.useRef(query); - const onDataStateChangeRef = React.useRef(onDataStateChange); - React.useEffect(() => { - queryRef.current = query; - onDataStateChangeRef.current = onDataStateChange; - }); - - const dataState = React.useMemo(() => { - const { status } = fetchState; - return status === undefined ? undefined : { ...fetchState, status, iTwins }; - }, [fetchState, iTwins]); - React.useEffect(() => { - // The new query has not reached the state yet, so reporting now would pair it with the - // previous query's result. - if (dataState && sameQuery(dataState.query, query)) { - onDataStateChangeRef.current?.(dataState); - } - }, [dataState, query]); - - /** Start over for the query in hand. Reads it from the ref so the caller can reset from an - * effect that must not depend on the query. */ - const reset = React.useCallback(() => { - setFetchState((state) => - hasStartedOver(state, queryRef.current) - ? state - : startingOver(queryRef.current) - ); - }, []); - - /** The iTwins in hand already answer the new query, so keep them and just retarget. */ - const applyQuery = React.useCallback((query: ITwinDataQuery) => { - setFetchState((state) => - sameQuery(state.query, query) ? state : { ...state, query } - ); - }, []); - - const markFetching = React.useCallback(() => { - setFetchState((state) => - state.status === DataStatus.Fetching - ? state - : { ...state, status: DataStatus.Fetching, error: undefined } - ); - }, []); - - const pageLoaded = React.useCallback( - (page: { iTwins: ITwinFull[]; isFirstPage: boolean; hasMore: boolean }) => { - setFetchState((state) => ({ - ...state, - status: DataStatus.Complete, - iTwins: page.isFirstPage - ? page.iTwins - : [...state.iTwins, ...page.iTwins], - hasMore: page.hasMore, - error: undefined, - })); - }, - [] - ); - - const pageFailed = React.useCallback( - (failure: { error: unknown; isFirstPage: boolean }) => { - setFetchState((state) => ({ - ...state, - status: DataStatus.FetchFailed, - iTwins: failure.isFirstPage ? [] : state.iTwins, - error: failure.error, - })); - }, - [] - ); - - /** Leaves hasMore alone: flipping it here would re-run the caller's fetch effect into this - * same branch. */ - const tokenRequired = React.useCallback(() => { - setFetchState((state) => ({ - ...state, - status: DataStatus.TokenRequired, - iTwins: [], - error: undefined, - })); - }, []); - - const dataProvided = React.useCallback((iTwins: ITwinFull[]) => { - setFetchState((state) => ({ - ...state, - status: DataStatus.Complete, - iTwins, - hasMore: false, - error: undefined, - })); - }, []); - - return { - status: fetchState.status, - iTwins, - hasMore: fetchState.hasMore, - reset, - applyQuery, - markFetching, - pageLoaded, - pageFailed, - tokenRequired, - dataProvided, - }; -}; From ea958d03293513e22b0da2087be122eca138ba8b Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Sat, 5 Sep 2026 00:14:15 -0400 Subject: [PATCH 09/19] test: pin the cleared favorites filter and inline token provider behavior --- .../containers/ITwinGrid/useITwinData.test.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts index c9d1e529..b659bf85 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts @@ -1047,4 +1047,70 @@ describe("useITwinData hook", () => { expect(states).toHaveLength(2); }); }); + + describe("stability", () => { + it("does not refetch when the favorites filter is cleared back to empty", async () => { + const urlWatcher = jest.fn(); + server.use( + rest.get( + "https://api.bentley.com/itwins/favorites", + (req, res, ctx) => { + urlWatcher(req.url.toString()); + return res( + ctx.status(200), + ctx.json({ iTwins: [{ id: "fav1", displayName: "favName1" }] }) + ); + } + ) + ); + + const { result, rerender, waitForNextUpdate } = renderHook< + Parameters, + ReturnType + >((initialValue) => useITwinData(...initialValue), { + initialProps: [{ accessToken, requestType: "favorites" }], + }); + await waitForNextUpdate(); + expect(urlWatcher).toHaveBeenCalledTimes(1); + + rerender([ + { accessToken, requestType: "favorites", filterOptions: "fav" }, + ]); + rerender([{ accessToken, requestType: "favorites", filterOptions: "" }]); + await act(async () => undefined); + + expect(urlWatcher).toHaveBeenCalledTimes(1); + expect(result.current.status).toEqual(DataStatus.Complete); + expect(result.current.iTwins.map(ids)).toEqual(["fav1"]); + }); + + it("does not refetch when an inline token provider changes identity", async () => { + const urlWatcher = jest.fn(); + server.use( + rest.get("https://api.bentley.com/itwins/", (req, res, ctx) => { + urlWatcher(req.url.toString()); + return res( + ctx.status(200), + ctx.json({ iTwins: [{ id: "my1", displayName: "myName1" }] }) + ); + }) + ); + + const { result, rerender, waitForNextUpdate } = renderHook< + Parameters, + ReturnType + >((initialValue) => useITwinData(...initialValue), { + initialProps: [{ accessToken: async () => accessToken }], + }); + await waitForNextUpdate(); + expect(urlWatcher).toHaveBeenCalledTimes(1); + + rerender([{ accessToken: async () => accessToken }]); + rerender([{ accessToken: async () => accessToken }]); + await act(async () => undefined); + + expect(urlWatcher).toHaveBeenCalledTimes(1); + expect(result.current.status).toEqual(DataStatus.Complete); + }); + }); }); From f186d0ad6120b6d0a21eb08c90191579d9d46fb0 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Sat, 5 Sep 2026 00:19:27 -0400 Subject: [PATCH 10/19] chore: add the rush change file --- ...ndata-infinite-query_2026-09-04-00-00.json | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json diff --git a/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json new file mode 100644 index 00000000..6188e63c --- /dev/null +++ b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json @@ -0,0 +1,25 @@ +{ + "changes": [ + { + "packageName": "@itwin/imodel-browser-react", + "comment": "Fix `ITwinGrid` refetching repeatedly when an inline access token provider is used", + "type": "patch" + }, + { + "packageName": "@itwin/imodel-browser-react", + "comment": "Stop a `shouldRefetchFavorites` flip from restarting a query with more pages to load", + "type": "patch" + }, + { + "packageName": "@itwin/imodel-browser-react", + "comment": "Do not offer `fetchMore` while an access token is required", + "type": "patch" + }, + { + "packageName": "@itwin/imodel-browser-react", + "comment": "Omit the empty `Cache-Control` header unless the favorites cache is bypassed", + "type": "patch" + } + ], + "packageName": "@itwin/imodel-browser-react" +} From 5e122539c235b6d87f7df9718ae86521e15b59f5 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Sat, 5 Sep 2026 04:43:26 -0400 Subject: [PATCH 11/19] test: pin that a shouldRefetchFavorites flip does not restart an unfinished query --- .../containers/ITwinGrid/useITwinData.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts index b659bf85..0decce03 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts @@ -1112,5 +1112,51 @@ describe("useITwinData hook", () => { expect(urlWatcher).toHaveBeenCalledTimes(1); expect(result.current.status).toEqual(DataStatus.Complete); }); + + it("does not restart an unfinished query when shouldRefetchFavorites flips", async () => { + const urlWatcher = jest.fn(); + const fullPage = Array.from({ length: 100 }, (_unused, index) => ({ + id: `fav${index}`, + displayName: `favName${index}`, + })); + server.use( + rest.get( + "https://api.bentley.com/itwins/favorites", + (req, res, ctx) => { + urlWatcher(req.url.toString()); + return res(ctx.status(200), ctx.json({ iTwins: fullPage })); + } + ) + ); + + const resetShouldRefetchFavorites = jest.fn(); + const { rerender, waitForNextUpdate } = renderHook< + Parameters, + ReturnType + >((initialValue) => useITwinData(...initialValue), { + initialProps: [ + { + accessToken, + requestType: "favorites", + shouldRefetchFavorites: false, + resetShouldRefetchFavorites, + }, + ], + }); + await waitForNextUpdate(); + expect(urlWatcher).toHaveBeenCalledTimes(1); + + rerender([ + { + accessToken, + requestType: "favorites", + shouldRefetchFavorites: true, + resetShouldRefetchFavorites, + }, + ]); + await act(async () => undefined); + + expect(urlWatcher).toHaveBeenCalledTimes(1); + }); }); }); From f6388efe5e583492ec4c1cc95eaa5a3c9216c70b Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Mon, 7 Sep 2026 21:52:43 -0400 Subject: [PATCH 12/19] docs: tighten the comments on the new hooks and the adapter --- .../src/containers/ITwinGrid/iTwinsApi.ts | 5 +---- .../src/containers/ITwinGrid/useITwinData.ts | 14 +++++++------- .../src/hooks/infiniteQueryReducer.ts | 8 +++----- .../imodel-browser/src/hooks/useEventCallback.ts | 6 +++--- .../imodel-browser/src/hooks/useInfiniteQuery.ts | 12 +++++------- .../imodel-browser/src/hooks/useReportChanges.ts | 4 ++-- 6 files changed, 21 insertions(+), 28 deletions(-) diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts index 99a84367..2019a857 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts @@ -69,10 +69,7 @@ const platformHeaders = async ({ ...(bypassCache ? { "Cache-Control": "no-cache" } : {}), }); -/** - * One page of iTwins, or a rejection carrying what the API answered. A totalCount of undefined - * means the response had no count, which is not zero. - */ +/** Rejects with the text a non-OK response carried, so callers see the API's own message. */ export const fetchITwinsPage = async ({ query, page, diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index d1fa9724..077dcd20 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -38,22 +38,22 @@ export interface ProjectDataHookOptions { } /** - * Identifies the credential without holding it. An inline `async () => token` provider is the - * documented way to keep a token fresh and changes identity on every render, so keying on the - * function itself would restart a settled query forever. The provider is read at request time. + * Identifies the credential without holding it. An inline `async () => token` provider changes + * identity every render, so keying on the function itself would refetch forever; it is instead + * read at request time. */ const toCredentialKey = (accessToken?: AccessTokenProvider) => { if (typeof accessToken === "function") { return "provider"; } - // An empty token reads as no credential, as it does today, so `??` would not do. + // An empty token means no credential, so `??` would be wrong here. if (accessToken === undefined || accessToken === "") { return undefined; } return accessToken; }; -/** Provided data wins over a missing token, which is the precedence the grid ships with. */ +/** Provided data wins over a missing token. */ const resolveITwinQueryLocally = ( query: ITwinQueryKey ): LocalResolution | undefined => { @@ -75,8 +75,8 @@ const differsOnlyByFilterText = (a: ITwinQueryKey, b: ITwinQueryKey) => a.providedData === b.providedData; /** - * Favorites and recents are filtered in the browser, so once every page is loaded a new filter - * text is answered by the iTwins in hand. Anything else restarts the query. + * Favorites and recents are filtered in the browser, so once every page is loaded the iTwins in + * hand already answer a new filter text. Anything else restarts the query. */ const decideOnITwinQueryChange = ( previous: ITwinQueryKey, diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts index 09987480..199ef73a 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DataStatus } from "../types"; -/** One page the effect must have in flight. Identified so a late answer can be dropped. */ +/** The id is what lets a late answer be recognised and dropped. */ export interface PageRequest { id: number; query: TQuery; @@ -18,13 +18,12 @@ export interface LoadedPage { totalCount?: number; } -/** A settled answer the query needs no request for. */ export type LocalResolution = | { status: DataStatus.Complete; items: TItem[] } | { status: DataStatus.TokenRequired | DataStatus.ContextRequired }; export interface InfiniteQueryPolicy { - /** A settled answer that needs no request, or undefined to fetch. */ + /** A settled answer needing no request, or undefined to fetch. */ resolveLocally: (query: TQuery) => LocalResolution | undefined; /** Whether the loaded items still answer the new query. */ decideOnQueryChange: ( @@ -84,7 +83,7 @@ const isFirstPage = (request: PageRequest) => const localItems = (resolution: LocalResolution) => resolution.status === DataStatus.Complete ? resolution.items : []; -/** Requests one page, leaving the status alone: a later page loads behind a Complete status. */ +/** Leaves the status alone: a later page loads behind a Complete status. */ const requestingPage = ( state: InfiniteQueryState, page: number @@ -97,7 +96,6 @@ const requestingPage = ( }; }; -/** Drops everything loaded and answers the query again, locally when the policy can. */ const startingOver = ( state: InfiniteQueryState, query: TQuery, diff --git a/packages/modules/imodel-browser/src/hooks/useEventCallback.ts b/packages/modules/imodel-browser/src/hooks/useEventCallback.ts index e27b7655..45dbc69f 100644 --- a/packages/modules/imodel-browser/src/hooks/useEventCallback.ts +++ b/packages/modules/imodel-browser/src/hooks/useEventCallback.ts @@ -5,9 +5,9 @@ import React from "react"; /** - * A function with one identity for the life of the hook that always calls the latest `fn`. Lets a - * caller pass an inline closure where a stable dependency is needed. The standard `useEffectEvent` - * polyfill: an insertion effect writes the ref before any layout or passive effect can read it. + * A function with one identity for the life of the hook that always calls the latest `fn`, so an + * inline closure can be passed where a stable dependency is needed. The `useEffectEvent` polyfill: + * the insertion effect writes the ref before any layout or passive effect can read it. */ export const useEventCallback = ( fn: (...args: TArgs) => TResult diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts index b9126b8d..3b55a215 100644 --- a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts @@ -20,7 +20,6 @@ export interface UseInfiniteQueryOptions extends InfiniteQueryPolicy { /** MUST be memoized. A new object every render restarts the query. */ query: TQuery; - /** Resolves with one page, or rejects with what the source answered. */ fetchPage: ( request: PageRequest, signal: AbortSignal @@ -44,9 +43,9 @@ const isAbortError = (error: unknown) => error instanceof Error && error.name === "AbortError"; /** - * Pages a query, one request at a time, and drops the answers of superseded requests. The state - * machine in `infiniteQueryReducer` is the single source of truth; the two effects here only start - * the query and keep the pending request in flight. + * Pages a query, one request at a time, dropping the answers of superseded requests. + * `infiniteQueryReducer` is the single source of truth; the effects here only keep the pending + * request in flight. */ export const useInfiniteQuery = ({ query, @@ -66,9 +65,8 @@ export const useInfiniteQuery = ({ initialUndecidedState(initial) ); - // The query prop moved ahead of the state. Telling the reducer during render makes React - // re-render with the reduced state before anything is committed: its documented way to adjust - // state when a prop changes, with no effect and no intermediate commit. + // The query prop moved ahead of the state. Dispatching during render is React's documented way + // to adjust state on a prop change: it re-renders with the reduced state before committing. if (state.query !== query) { dispatch({ type: "queryChanged", query }); } diff --git a/packages/modules/imodel-browser/src/hooks/useReportChanges.ts b/packages/modules/imodel-browser/src/hooks/useReportChanges.ts index d7fddb7b..d0ee0c30 100644 --- a/packages/modules/imodel-browser/src/hooks/useReportChanges.ts +++ b/packages/modules/imodel-browser/src/hooks/useReportChanges.ts @@ -5,8 +5,8 @@ import React from "react"; /** - * Calls `report(value)` once per distinct value, the first one included. Only this effect writes - * the ref, so `report` can stay a dependency without re-reporting and without a ref-sync effect. + * Calls `report(value)` once per distinct value, the first one included. Deduping on the last + * reported value is what lets `report` stay a dependency without re-reporting. */ export const useReportChanges = ( value: TValue, From e58a573253fd422d6758516ce52cd265de795bf6 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Thu, 10 Sep 2026 15:39:09 -0400 Subject: [PATCH 13/19] docs: record why the query keys on credential presence --- .../lk-useitwindata-infinite-query_2026-09-04-00-00.json | 2 +- .../imodel-browser/src/containers/ITwinGrid/useITwinData.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json index 6188e63c..44bbe86f 100644 --- a/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json +++ b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@itwin/imodel-browser-react", - "comment": "Fix `ITwinGrid` refetching repeatedly when an inline access token provider is used", + "comment": "ITwinGrid no longer refetches iTwins on every render with an unmemoized token provider", "type": "patch" }, { diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index 077dcd20..35569f90 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -40,7 +40,11 @@ export interface ProjectDataHookOptions { /** * Identifies the credential without holding it. An inline `async () => token` provider changes * identity every render, so keying on the function itself would refetch forever; it is instead - * read at request time. + * read at request time, which always yields the latest provider. + * + * The trade-off: every function is the same key, so swapping one provider for another does not + * restart a settled query. Note `` still documents that a provider must be memoized, + * because `useITwinFavorites` keys on its identity. */ const toCredentialKey = (accessToken?: AccessTokenProvider) => { if (typeof accessToken === "function") { From 2701a6ec4a9222f2d31f29681c6ab819d2ac5a5d Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Thu, 10 Sep 2026 16:08:06 -0400 Subject: [PATCH 14/19] refactor: report state changes without a dedicated hook --- .../src/containers/ITwinGrid/useITwinData.ts | 28 ++++---- .../src/hooks/useReportChanges.test.ts | 71 ------------------- .../src/hooks/useReportChanges.ts | 26 ------- 3 files changed, 14 insertions(+), 111 deletions(-) delete mode 100644 packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts delete mode 100644 packages/modules/imodel-browser/src/hooks/useReportChanges.ts diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index 35569f90..f0910eb6 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -6,8 +6,8 @@ import React from "react"; import { useLogger } from "../../contexts/LoggerContext"; import { LocalResolution, PageRequest } from "../../hooks/infiniteQueryReducer"; +import { useEventCallback } from "../../hooks/useEventCallback"; import { useInfiniteQuery } from "../../hooks/useInfiniteQuery"; -import { useReportChanges } from "../../hooks/useReportChanges"; import { AccessTokenProvider, ApiOverrides, @@ -171,25 +171,25 @@ export const useITwinData = ({ [status, dataQuery, iTwins, hasMore, error] ); - const reportDataState = React.useCallback( + const reportDataState = useEventCallback( (state: ITwinDataState | undefined) => { if (state !== undefined) { onDataStateChange?.(state); } - }, - [onDataStateChange] + } ); - useReportChanges(dataState, reportDataState); + React.useEffect(() => { + reportDataState(dataState); + }, [dataState, reportDataState]); - const reportFailure = React.useCallback( - (failure: unknown) => { - if (failure !== undefined) { - logger.logError("Failed to fetch iTwins", failure); - } - }, - [logger] - ); - useReportChanges(error, reportFailure); + const reportFailure = useEventCallback((failure: unknown) => { + if (failure !== undefined) { + logger.logError("Failed to fetch iTwins", failure); + } + }); + React.useEffect(() => { + reportFailure(error); + }, [error, reportFailure]); return { iTwins, diff --git a/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts b/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts deleted file mode 100644 index 75b9591e..00000000 --- a/packages/modules/imodel-browser/src/hooks/useReportChanges.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Bentley Systems, Incorporated. All rights reserved. - * See LICENSE.md in the project root for license terms and full copyright notice. - *--------------------------------------------------------------------------------------------*/ -import { renderHook } from "@testing-library/react-hooks"; - -import { useReportChanges } from "./useReportChanges"; - -describe("useReportChanges", () => { - interface Props { - value: string | undefined; - report: (value: string | undefined) => void; - } - - const renderWith = (initialProps: Props) => - renderHook( - ({ value, report }) => useReportChanges(value, report), - { initialProps } - ); - - it("reports the first value", () => { - const report = jest.fn(); - - renderWith({ value: "first", report }); - - expect(report).toHaveBeenCalledTimes(1); - expect(report).toHaveBeenCalledWith("first"); - }); - - it("reports an initial undefined value", () => { - const report = jest.fn(); - - renderWith({ value: undefined, report }); - - expect(report).toHaveBeenCalledTimes(1); - expect(report).toHaveBeenCalledWith(undefined); - }); - - it("does not report the same value again when only the reporter changed", () => { - const report = jest.fn(); - const { rerender } = renderWith({ value: "same", report }); - - rerender({ value: "same", report: (value) => report(value) }); - rerender({ value: "same", report }); - - expect(report).toHaveBeenCalledTimes(1); - }); - - it("reports a new value once, through the latest reporter", () => { - const first = jest.fn(); - const second = jest.fn(); - const { rerender } = renderWith({ value: "one", report: first }); - - rerender({ value: "two", report: second }); - - expect(first).toHaveBeenCalledTimes(1); - expect(first).toHaveBeenCalledWith("one"); - expect(second).toHaveBeenCalledTimes(1); - expect(second).toHaveBeenCalledWith("two"); - }); - - it("reports a value that comes back, because only the last one is remembered", () => { - const report = jest.fn(); - const { rerender } = renderWith({ value: "one", report }); - - rerender({ value: "two", report }); - rerender({ value: "one", report }); - - expect(report).toHaveBeenCalledTimes(3); - }); -}); diff --git a/packages/modules/imodel-browser/src/hooks/useReportChanges.ts b/packages/modules/imodel-browser/src/hooks/useReportChanges.ts deleted file mode 100644 index d0ee0c30..00000000 --- a/packages/modules/imodel-browser/src/hooks/useReportChanges.ts +++ /dev/null @@ -1,26 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Bentley Systems, Incorporated. All rights reserved. - * See LICENSE.md in the project root for license terms and full copyright notice. - *--------------------------------------------------------------------------------------------*/ -import React from "react"; - -/** - * Calls `report(value)` once per distinct value, the first one included. Deduping on the last - * reported value is what lets `report` stay a dependency without re-reporting. - */ -export const useReportChanges = ( - value: TValue, - report: (value: TValue) => void -) => { - const lastReported = React.useRef<{ value: TValue } | undefined>(undefined); - React.useEffect(() => { - const alreadyReported = - lastReported.current !== undefined && - Object.is(lastReported.current.value, value); - if (alreadyReported) { - return; - } - lastReported.current = { value }; - report(value); - }, [value, report]); -}; From ce5355915768264ec20ac71e3e1669d525c6bc70 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Thu, 10 Sep 2026 17:42:01 -0400 Subject: [PATCH 15/19] Change comment --- packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts index 3b55a215..85b7ed7a 100644 --- a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts @@ -30,7 +30,9 @@ export interface InfiniteQueryResult { items: TItem[]; /** Undefined on the first render, before anything has been decided. */ status: DataStatus | undefined; - /** Whether a page is in flight, which a later page does not show in the status. */ + /** Whether a request is in flight. This is different from the status, which reports + * `Fetching` only for the first page. Here we know a request is in flight even after + * the first page has loaded. */ isFetching: boolean; hasMore: boolean; error: unknown; From 65a48da046eeef811dd98a0f1f1a4adfdc57e168 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 11 Sep 2026 11:43:07 -0400 Subject: [PATCH 16/19] Address code review comments --- .../containers/ITwinGrid/iTwinsApi.test.ts | 4 ++-- .../src/containers/ITwinGrid/iTwinsApi.ts | 8 +++---- .../src/containers/ITwinGrid/useITwinData.ts | 23 +++++++++---------- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts index 47be2564..65088f12 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts @@ -9,12 +9,12 @@ import { ITwinFull } from "../../types"; import { buildITwinsPageUrl, fetchITwinsPage, - ITwinQueryKey, + ITwinQueryParams, ITWINS_PAGE_SIZE, } from "./iTwinsApi"; describe("iTwinsApi", () => { - const baseQuery: ITwinQueryKey = { + const baseQuery: ITwinQueryParams = { requestType: "", filterText: "", iTwinSubClass: "Project", diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts index 2019a857..ecff6b4b 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts @@ -13,8 +13,8 @@ export const isClientSideFiltered = ( requestType: ITwinDataQuery["requestType"] ) => requestType === "favorites" || requestType === "recents"; -/** Everything that identifies a request, so two equal keys mean one query. */ -export interface ITwinQueryKey extends ITwinDataQuery { +/** Everything that identifies a request, so two equal values generate the same query. */ +export interface ITwinQueryParams extends ITwinDataQuery { /** Undefined without a credential, the token itself for a string, "provider" for a function. */ credentialKey: string | undefined; serverEnvironmentPrefix?: "" | "dev" | "qa"; @@ -25,7 +25,7 @@ export const buildITwinsPageUrl = ({ query, page, }: { - query: ITwinQueryKey; + query: ITwinQueryParams; page: number; }) => { const { requestType, filterText, iTwinSubClass, orderby } = query; @@ -47,7 +47,7 @@ export const buildITwinsPageUrl = ({ }; export interface FetchITwinsPageOptions { - query: ITwinQueryKey; + query: ITwinQueryParams; page: number; accessToken: AccessTokenProvider; bypassCache: boolean; diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index f0910eb6..08b0a225 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -21,7 +21,7 @@ import { import { fetchITwinsPage, isClientSideFiltered, - ITwinQueryKey, + ITwinQueryParams, } from "./iTwinsApi"; import { useITwinFilter } from "./useITwinFilter"; @@ -38,9 +38,8 @@ export interface ProjectDataHookOptions { } /** - * Identifies the credential without holding it. An inline `async () => token` provider changes - * identity every render, so keying on the function itself would refetch forever; it is instead - * read at request time, which always yields the latest provider. + * Identifies the credential: the token itself for a string, `"provider"` for a function. Keying on + * a function's identity would refetch every render, so the provider is read at request time. * * The trade-off: every function is the same key, so swapping one provider for another does not * restart a settled query. Note `` still documents that a provider must be memoized, @@ -59,7 +58,7 @@ const toCredentialKey = (accessToken?: AccessTokenProvider) => { /** Provided data wins over a missing token. */ const resolveITwinQueryLocally = ( - query: ITwinQueryKey + query: ITwinQueryParams ): LocalResolution | undefined => { if (query.providedData !== undefined) { return { status: DataStatus.Complete, items: query.providedData }; @@ -70,7 +69,7 @@ const resolveITwinQueryLocally = ( return undefined; }; -const differsOnlyByFilterText = (a: ITwinQueryKey, b: ITwinQueryKey) => +const differsOnlyByFilterText = (a: ITwinQueryParams, b: ITwinQueryParams) => a.requestType === b.requestType && a.iTwinSubClass === b.iTwinSubClass && a.orderby === b.orderby && @@ -83,8 +82,8 @@ const differsOnlyByFilterText = (a: ITwinQueryKey, b: ITwinQueryKey) => * hand already answer a new filter text. Anything else restarts the query. */ const decideOnITwinQueryChange = ( - previous: ITwinQueryKey, - next: ITwinQueryKey, + previous: ITwinQueryParams, + next: ITwinQueryParams, loaded: { hasMore: boolean } ) => isClientSideFiltered(next.requestType) && @@ -93,7 +92,7 @@ const decideOnITwinQueryChange = ( ? "keep" : "restart"; -/** The core requests a page only after `resolveITwinQueryLocally` accepted the credential. */ +/** A query with no credential resolves to TokenRequired, so no page is requested. */ const requireAccessToken = (accessToken?: AccessTokenProvider) => { if (accessToken === undefined || accessToken === "") { throw new Error("A page was requested without an access token"); @@ -126,7 +125,7 @@ export const useITwinData = ({ }), [requestType, filterOptions, iTwinSubClass, orderbyOptions] ); - const queryKey = React.useMemo( + const queryParams = React.useMemo( () => ({ ...dataQuery, credentialKey, @@ -137,7 +136,7 @@ export const useITwinData = ({ ); const fetchPage = async ( - request: PageRequest, + request: PageRequest, signal: AbortSignal ) => { const forFavorites = request.query.requestType === "favorites"; @@ -156,7 +155,7 @@ export const useITwinData = ({ const { items, status, hasMore, error, totalCount, fetchNextPage, refetch } = useInfiniteQuery({ - query: queryKey, + query: queryParams, fetchPage, resolveLocally: resolveITwinQueryLocally, decideOnQueryChange: decideOnITwinQueryChange, From 1ba02cb2ee41aa5983537e0a7e6dbf5ba8414e7f Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 11 Sep 2026 15:01:29 -0400 Subject: [PATCH 17/19] refactor: key the query on the access token itself --- ...ndata-infinite-query_2026-09-04-00-00.json | 5 ---- .../containers/ITwinGrid/iTwinsApi.test.ts | 2 +- .../src/containers/ITwinGrid/iTwinsApi.ts | 3 +- .../containers/ITwinGrid/useITwinData.test.ts | 29 ------------------- .../src/containers/ITwinGrid/useITwinData.ts | 28 +++--------------- 5 files changed, 6 insertions(+), 61 deletions(-) diff --git a/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json index 44bbe86f..f704965a 100644 --- a/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json +++ b/common/changes/@itwin/imodel-browser-react/lk-useitwindata-infinite-query_2026-09-04-00-00.json @@ -1,10 +1,5 @@ { "changes": [ - { - "packageName": "@itwin/imodel-browser-react", - "comment": "ITwinGrid no longer refetches iTwins on every render with an unmemoized token provider", - "type": "patch" - }, { "packageName": "@itwin/imodel-browser-react", "comment": "Stop a `shouldRefetchFavorites` flip from restarting a query with more pages to load", diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts index 65088f12..960b1cc7 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.test.ts @@ -19,7 +19,7 @@ describe("iTwinsApi", () => { filterText: "", iTwinSubClass: "Project", orderby: undefined, - credentialKey: "accessToken", + accessToken: "accessToken", }; describe("buildITwinsPageUrl", () => { diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts index ecff6b4b..b074cf63 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/iTwinsApi.ts @@ -15,8 +15,7 @@ export const isClientSideFiltered = ( /** Everything that identifies a request, so two equal values generate the same query. */ export interface ITwinQueryParams extends ITwinDataQuery { - /** Undefined without a credential, the token itself for a string, "provider" for a function. */ - credentialKey: string | undefined; + accessToken?: AccessTokenProvider; serverEnvironmentPrefix?: "" | "dev" | "qa"; providedData?: ITwinFull[]; } diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts index 0decce03..844035bf 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.test.ts @@ -1084,35 +1084,6 @@ describe("useITwinData hook", () => { expect(result.current.iTwins.map(ids)).toEqual(["fav1"]); }); - it("does not refetch when an inline token provider changes identity", async () => { - const urlWatcher = jest.fn(); - server.use( - rest.get("https://api.bentley.com/itwins/", (req, res, ctx) => { - urlWatcher(req.url.toString()); - return res( - ctx.status(200), - ctx.json({ iTwins: [{ id: "my1", displayName: "myName1" }] }) - ); - }) - ); - - const { result, rerender, waitForNextUpdate } = renderHook< - Parameters, - ReturnType - >((initialValue) => useITwinData(...initialValue), { - initialProps: [{ accessToken: async () => accessToken }], - }); - await waitForNextUpdate(); - expect(urlWatcher).toHaveBeenCalledTimes(1); - - rerender([{ accessToken: async () => accessToken }]); - rerender([{ accessToken: async () => accessToken }]); - await act(async () => undefined); - - expect(urlWatcher).toHaveBeenCalledTimes(1); - expect(result.current.status).toEqual(DataStatus.Complete); - }); - it("does not restart an unfinished query when shouldRefetchFavorites flips", async () => { const urlWatcher = jest.fn(); const fullPage = Array.from({ length: 100 }, (_unused, index) => ({ diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index 08b0a225..6bf872eb 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -37,25 +37,6 @@ export interface ProjectDataHookOptions { onDataStateChange?: (state: ITwinDataState) => void; } -/** - * Identifies the credential: the token itself for a string, `"provider"` for a function. Keying on - * a function's identity would refetch every render, so the provider is read at request time. - * - * The trade-off: every function is the same key, so swapping one provider for another does not - * restart a settled query. Note `` still documents that a provider must be memoized, - * because `useITwinFavorites` keys on its identity. - */ -const toCredentialKey = (accessToken?: AccessTokenProvider) => { - if (typeof accessToken === "function") { - return "provider"; - } - // An empty token means no credential, so `??` would be wrong here. - if (accessToken === undefined || accessToken === "") { - return undefined; - } - return accessToken; -}; - /** Provided data wins over a missing token. */ const resolveITwinQueryLocally = ( query: ITwinQueryParams @@ -63,7 +44,7 @@ const resolveITwinQueryLocally = ( if (query.providedData !== undefined) { return { status: DataStatus.Complete, items: query.providedData }; } - if (query.credentialKey === undefined) { + if (!query.accessToken) { return { status: DataStatus.TokenRequired }; } return undefined; @@ -73,7 +54,7 @@ const differsOnlyByFilterText = (a: ITwinQueryParams, b: ITwinQueryParams) => a.requestType === b.requestType && a.iTwinSubClass === b.iTwinSubClass && a.orderby === b.orderby && - a.credentialKey === b.credentialKey && + a.accessToken === b.accessToken && a.serverEnvironmentPrefix === b.serverEnvironmentPrefix && a.providedData === b.providedData; @@ -112,7 +93,6 @@ export const useITwinData = ({ onDataStateChange, }: ProjectDataHookOptions) => { const logger = useLogger(); - const credentialKey = toCredentialKey(accessToken); const providedData = apiOverrides?.data; const serverEnvironmentPrefix = apiOverrides?.serverEnvironmentPrefix; @@ -128,11 +108,11 @@ export const useITwinData = ({ const queryParams = React.useMemo( () => ({ ...dataQuery, - credentialKey, + accessToken, serverEnvironmentPrefix, providedData, }), - [dataQuery, credentialKey, serverEnvironmentPrefix, providedData] + [dataQuery, accessToken, serverEnvironmentPrefix, providedData] ); const fetchPage = async ( From 249846d9ee6626b943a5ae9872dc9ed393ecb556 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 11 Sep 2026 16:08:49 -0400 Subject: [PATCH 18/19] refactor: rename the query-change policy to shouldRestartQuery --- .../src/containers/ITwinGrid/useITwinData.ts | 17 +++++++++-------- .../src/hooks/infiniteQueryReducer.test.ts | 4 ++-- .../src/hooks/infiniteQueryReducer.ts | 12 +++++------- .../src/hooks/useInfiniteQuery.test.ts | 8 ++++---- .../src/hooks/useInfiniteQuery.ts | 4 ++-- 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts index 6bf872eb..5a365301 100644 --- a/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts +++ b/packages/modules/imodel-browser/src/containers/ITwinGrid/useITwinData.ts @@ -62,16 +62,17 @@ const differsOnlyByFilterText = (a: ITwinQueryParams, b: ITwinQueryParams) => * Favorites and recents are filtered in the browser, so once every page is loaded the iTwins in * hand already answer a new filter text. Anything else restarts the query. */ -const decideOnITwinQueryChange = ( +const shouldRestartITwinQuery = ( previous: ITwinQueryParams, next: ITwinQueryParams, loaded: { hasMore: boolean } -) => - isClientSideFiltered(next.requestType) && - differsOnlyByFilterText(previous, next) && - !loaded.hasMore - ? "keep" - : "restart"; +) => { + const answeredByClientSideFilter = + isClientSideFiltered(next.requestType) && + differsOnlyByFilterText(previous, next) && + !loaded.hasMore; + return !answeredByClientSideFilter; +}; /** A query with no credential resolves to TokenRequired, so no page is requested. */ const requireAccessToken = (accessToken?: AccessTokenProvider) => { @@ -138,7 +139,7 @@ export const useITwinData = ({ query: queryParams, fetchPage, resolveLocally: resolveITwinQueryLocally, - decideOnQueryChange: decideOnITwinQueryChange, + shouldRestartQuery: shouldRestartITwinQuery, }); const iTwins = useITwinFilter(items, dataQuery.filterText); diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts index 7277fb81..cfb18ba6 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.test.ts @@ -19,11 +19,11 @@ describe("infiniteQueryReducer", () => { const fetching: InfiniteQueryPolicy = { resolveLocally: () => undefined, - decideOnQueryChange: () => "restart", + shouldRestartQuery: () => true, }; const keeping: InfiniteQueryPolicy = { ...fetching, - decideOnQueryChange: () => "keep", + shouldRestartQuery: () => false, }; const query: Query = { text: "", scope: "all" }; diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts index 199ef73a..07042f51 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts @@ -25,12 +25,12 @@ export type LocalResolution = export interface InfiniteQueryPolicy { /** A settled answer needing no request, or undefined to fetch. */ resolveLocally: (query: TQuery) => LocalResolution | undefined; - /** Whether the loaded items still answer the new query. */ - decideOnQueryChange: ( + /** Whether the new query must be fetched again, or the loaded items already answer it. */ + shouldRestartQuery: ( previous: TQuery, next: TQuery, loaded: { hasMore: boolean } - ) => "keep" | "restart"; + ) => boolean; } export interface InfiniteQueryState { @@ -144,12 +144,10 @@ const reduceQueryChanged = ( if (nothingDecidedYet) { return { ...state, query }; } - const decision = policy.decideOnQueryChange(state.query, query, { + const restart = policy.shouldRestartQuery(state.query, query, { hasMore: state.hasMore, }); - return decision === "keep" - ? { ...state, query } - : startingOver(state, query, policy); + return restart ? startingOver(state, query, policy) : { ...state, query }; }; const reducePageLoaded = ( diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts index 2c18c65f..4afcd82d 100644 --- a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.test.ts @@ -17,7 +17,7 @@ describe("useInfiniteQuery", () => { const restarting = { resolveLocally: () => undefined, - decideOnQueryChange: () => "restart" as const, + shouldRestartQuery: () => true, }; const renderWith = (initialProps: Options) => @@ -54,7 +54,7 @@ describe("useInfiniteQuery", () => { const { result } = renderWith({ query: { text: "" }, fetchPage, - decideOnQueryChange: () => "restart", + shouldRestartQuery: () => true, resolveLocally: () => ({ status: DataStatus.TokenRequired }), }); await act(async () => undefined); @@ -142,14 +142,14 @@ describe("useInfiniteQuery", () => { query: { text: "" }, fetchPage, resolveLocally: () => undefined, - decideOnQueryChange: () => "keep", + shouldRestartQuery: () => false, }); await waitForNextUpdate(); rerender({ query: { text: "next" }, fetchPage, resolveLocally: () => undefined, - decideOnQueryChange: () => "keep", + shouldRestartQuery: () => false, }); await act(async () => undefined); diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts index 85b7ed7a..1d821204 100644 --- a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts @@ -53,7 +53,7 @@ export const useInfiniteQuery = ({ query, fetchPage, resolveLocally, - decideOnQueryChange, + shouldRestartQuery, }: UseInfiniteQueryOptions): InfiniteQueryResult => { const reduce = ( current: InfiniteQueryState, @@ -61,7 +61,7 @@ export const useInfiniteQuery = ({ ) => reduceInfiniteQuery(current, action, { resolveLocally, - decideOnQueryChange, + shouldRestartQuery, }); const [state, dispatch] = React.useReducer(reduce, query, (initial: TQuery) => initialUndecidedState(initial) From 04a635367fdc0841ac0281724faca00398dd56c1 Mon Sep 17 00:00:00 2001 From: Lukasz Kokot Date: Fri, 11 Sep 2026 16:29:35 -0400 Subject: [PATCH 19/19] docs: note the preserved behavior for hasMore and isFetching --- .../modules/imodel-browser/src/hooks/infiniteQueryReducer.ts | 2 ++ packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts index 07042f51..099975a1 100644 --- a/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts +++ b/packages/modules/imodel-browser/src/hooks/infiniteQueryReducer.ts @@ -64,6 +64,8 @@ export const initialUndecidedState = ( query, status: undefined, items: [], + /** It would be logical for this status to be "false" by default, but that is + * the existing behavior. */ hasMore: true, error: undefined, totalCount: undefined, diff --git a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts index 1d821204..f970bbb8 100644 --- a/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts +++ b/packages/modules/imodel-browser/src/hooks/useInfiniteQuery.ts @@ -31,8 +31,8 @@ export interface InfiniteQueryResult { /** Undefined on the first render, before anything has been decided. */ status: DataStatus | undefined; /** Whether a request is in flight. This is different from the status, which reports - * `Fetching` only for the first page. Here we know a request is in flight even after - * the first page has loaded. */ + * `Fetching` only for the first page (existing behavior). Here we know a request + * is in flight even after the first page has loaded. */ isFetching: boolean; hasMore: boolean; error: unknown;