Add onDataStateChange to ITwinGrid - #231
Open
LukaszKokot wants to merge 13 commits into
Open
Conversation
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
commented
Aug 18, 2026
| * 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 = ({ |
Contributor
Author
There was a problem hiding this comment.
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
commented
Aug 18, 2026
| tokenRequired, | ||
| dataProvided, | ||
| } = useITwinDataState(query, onDataStateChange); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
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
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
LukaszKokot
force-pushed
the
lk/itwin-grid-data-state
branch
from
August 19, 2026 19:21
e75fa51 to
db65f8b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.onDataStateChangereports from an effect and carries the query it belongs to, plushasMore, sincecompletemeans 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:
statusisfetchingrather thanundefinedon the first render, whichpostProcessCallbackreceives as its second argument. It has its own change file entry.Why it is split this way
The hook state was three
useStatecalls set from one async callback, so a render could showcompletebeside 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
setFetchStatecalls, 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
createFetchIModelsFninuseIModelData.Testing
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.