Add onDataStateChange to ITwinGrid by LukaszKokot · Pull Request #231 · iTwin/admin-components-react

LukaszKokot · 2026-08-18T21:29:25Z

Partially solves AB#2114177

Why

Consumers need to know when the grid has finished answering a query. The only way that state left the grid was with postProcessCallback. And it runs during render and never says which request a status belongs to, so a consumer (like Studio) reconstructs that from its own props.
onDataStateChange reports from an effect and carries the query it belongs to, plus hasMore, since complete means the page landed rather than all the data being present, and the error, which until now only reached the logger.

One behaviour changes for everyone: status is fetching rather than undefined on the first render, which postProcessCallback receives as its second argument. It has its own change file entry.

Why it is split this way

The hook state was three useState calls set from one async callback, so a render could show complete beside the previous query's iTwins. Merging them into one object is what makes a report worth trusting.
It was then still written from seven scattered setFetchState calls, four of them inside the fetch effect. From each write, we created and extracted a properly named function.

The rest is placement. The state and the page request have their own homes now, the latter following createFetchIModelsFn in useIModelData.

Testing

Additional unit tests created
Manually tested in Storybook, and the callback function has been instrumented with logging so we can see how it behaves in Storybook's UI.
Tested in Studio: I actually went on and changed the e2e code in my local worktree-itwin-picker-steps branch (see https://github.com/iTwin/studio/pull/4089) to use the new callback (I pointed Studio's pnpm to a tarball of this branch here).
No problem to report, no retrying steps, and a much cleaner way to infer state.

For reviewers

I suggest looking at individual commits to see how/what went into the refactor ; it's easier to follow.

The status, the iTwins and the has-more flag were three useState calls set from the same async fetch callback. Nothing guarantees they land in one render, so a render could show Complete beside the previous query's iTwins. Initial status is now fetching rather than undefined, which is what postProcessCallback receives as its second argument on the first render.

The grid's fetching state only escaped through postProcessCallback, a hook for transforming the iTwin array that consumers were reading a status out of. It runs during render, so a consumer had to work out which request a status belonged to from its own props. The new prop reports from an effect, and carries the query it belongs to. It also carries hasMore, since complete means the page landed rather than all the data being present, and the error behind error_fetchFailed, which until now only reached the logger. The query names the subclass as well as the request type, filter text and ordering, so that switching subclass is not mistaken for a refresh of the previous one. Partially solves AB#2114177

onDataStateChange matches the argTypesRegex in preview.tsx, so Storybook infers an action spy for it in every story that does not declare one. The grid calls the prop while it renders, which an inferred spy throws on, taking down every story in both files.

Logs each report with the time its query took to settle, and sends the report itself to the Actions panel. Searching the favorites or recents tab shows the case that is otherwise hard to see: a result reported with no fetch before it, answered from the iTwins already loaded.

Follows createFetchIModelsFn in useIModelData, which already owns URL assembly and the fetch for the iModel grid. Takes an options object rather than that one's ten positional parameters, and reads the query descriptor directly, since the request type, filter text, subclass and ordering are exactly what the URL needs. The effect keeps everything stateful: the superseded-request guard, the totalCount and pagination writes, the favorites reset, and the abort on cleanup. A totalCount of undefined now means the response carried no count. Returning Number(null) instead would have reported zero.

The state became one object so that a render could never show one query's status beside another query's iTwins, but it was still written from seven scattered setFetchState calls, four of them inside the fetch effect. The invariant held by convention. Each write is now a named transition and setFetchState is referenced nowhere else. Every transition takes what it needs as an argument so it can carry empty dependencies and sit in the effect dependency arrays without re-running them. Three things stay with the caller on purpose: the logging, since a transition that logs would need the logger in its dependencies; the totalCount, pagination and favorites writes, so their order relative to the state write is unchanged; and the page-zero check ahead of markFetching, which is pagination rather than state.

useITwinData now derives the query, decides what to request and when, and drives the transitions. What the grid's data is, and what a report says about it, lives next door. Client side filtering moves with it, because the filtered list is what a report carries rather than something the fetch needs. The hook takes only the query: filterText is filterOptions ?? "", and useITwinFilter lowercases both to the same empty string. The ref that reset reads is now declared inside the hook, which keeps it ahead of the two reset effects that call reset, since a hook's effects are queued where the hook is called.

searchParams.get returns null rather than an empty string, so ?? says what is meant and clears the lint warning.

The two declared the same five fields, so a field added to the report had to be added twice. FetchState now takes them from ITwinDataState. They stay separate types because the iTwins differ: the state holds every page fetched, a report carries only what client side filtering kept. Collapsing them would leave the variable name as the only thing saying which is which, and reporting the state directly would then typecheck.

The payload belongs in the prop's own documentation, not the changelog.

LukaszKokot · 2026-08-18T21:37:46Z

+ * 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 = ({


The fetch function has been extracted from the hook itself. It arguably improves the readability of the hook itself ; and while I did not notice at first, it is also how the iModel hook/fetch has been split.

It argued for where the code sits rather than saying anything the signature does not.

LukaszKokot · 2026-08-18T21:42:08Z

+    tokenRequired,
+    dataProvided,
+  } = useITwinDataState(query, onDataStateChange);



As you can see here, the whole state management has been moved to a dedicated (sub-)hook. By extracting the state, we make the main hook more readable too.

LukaszKokot · 2026-08-24T14:12:20Z

@alexdunae @ben-polinsky could you please take a look ? I've tested this locally on my Studio branch targeting this local branch as override. I can confirm it works fine and we can remove a bunch of code from there, including defensive assertions.

alexdunae · 2026-08-24T17:42:49Z

We should probably add the equivalent ITwinGrid (non-MUI) while we're here

alexdunae · 2026-08-24T17:49:00Z

+
+const startingOver = (query: ITwinDataQuery): FetchState => ({
+  query,
+  status: DataStatus.Fetching,


Not sure what to do about this one. postProcessCallback currently starts with status=undefined and this PR changes that.

So technically an API change.

It's hard to imagine that this will be a breaking change for people

If we wanted pure API compatibility we could

Suggested change

status: DataStatus.Fetching,

status: undefined,

and then in markFetching set status=DataStatus.Fetching

Would that complicate your code even more?

I'll check, I was hesitant in including this as it IS indeed an API change. It can certainly break fragile e2e test assertions for example.
I think it's best to move it out. At worse, we could actually have another PR just for this, that way people could know "oh, it's because of THAT change".

alexdunae · 2026-08-24T17:52:57Z

+  state.hasMore;
+
+export const useITwinDataState = (
+  query: ITwinDataQuery,


nit: maybe a jsdoc to ensure query is always passed in memoized by callers

Backs out the one part of this branch that changed behaviour for consumers who never pass the new prop, since postProcessCallback receives the status as its second argument. Only the initial state is undefined. startingOver keeps fetching, because it also runs on every reset and those went to fetching before. The report skips a state with no status, so the new callback still never reports one.

The report effect depends on the query, so a new object every render reports the same state again on every render.

LukaszKokot added 11 commits August 18, 2026 14:15

Use nullish coalescing for the subclass test's fallback

cfeea73

searchParams.get returns null rather than an empty string, so ?? says what is meant and clears the lint warning.

Merge remote-tracking branch 'origin/main' into lk/itwin-grid-data-state

de56439

Shorten the onDataStateChange change file entry

fa25770

The payload belongs in the prop's own documentation, not the changelog.

LukaszKokot commented Aug 18, 2026

View reviewed changes

Drop the useITwinDataState doc comment

6e8d6fb

It argued for where the code sits rather than saying anything the signature does not.

LukaszKokot commented Aug 18, 2026

View reviewed changes

LukaszKokot self-assigned this Aug 18, 2026

LukaszKokot marked this pull request as ready for review August 18, 2026 22:08

LukaszKokot requested review from alexdunae, aruniverse, ben-polinsky, kckst8 and toddsouthenbentley as code owners August 18, 2026 22:08

Merge remote-tracking branch 'origin/main' into lk/itwin-grid-data-state

db65f8b

LukaszKokot force-pushed the lk/itwin-grid-data-state branch from e75fa51 to db65f8b Compare August 19, 2026 19:21

alexdunae reviewed Aug 24, 2026

View reviewed changes

LukaszKokot added 2 commits August 24, 2026 18:20

Note that useITwinDataState needs a memoized query

f4d4d3b

The report effect depends on the query, so a new object every render reports the same state again on every render.

Navigation Menu

Uh oh!

Add onDataStateChange to ITwinGrid - #231

Add onDataStateChange to ITwinGrid#231
LukaszKokot wants to merge 15 commits into
mainfrom
lk/itwin-grid-data-state

LukaszKokot commented Aug 18, 2026 •

edited

Loading

Uh oh!

LukaszKokot Aug 18, 2026 •

edited

Loading

Uh oh!

LukaszKokot Aug 18, 2026

Uh oh!

LukaszKokot commented Aug 24, 2026

Uh oh!

alexdunae Aug 24, 2026

Uh oh!

alexdunae Aug 24, 2026

Uh oh!

LukaszKokot Aug 24, 2026

Uh oh!

alexdunae Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

LukaszKokot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Why it is split this way

Testing

For reviewers

Uh oh!

LukaszKokot Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LukaszKokot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszKokot commented Aug 24, 2026

Uh oh!

alexdunae Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

alexdunae Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszKokot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

alexdunae Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LukaszKokot commented Aug 18, 2026 •

edited

Loading

LukaszKokot Aug 18, 2026 •

edited

Loading