feat(OneDataCollection): keep every loaded children page live and replaceable - #5245
Conversation
🔍 Review policy: FeatureThe PR title starts with Required approvals
How this was decided
Policy source: |
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
🔍 Visual review for your branch is published 🔍Here are the links to: |
📦 Alpha Package Version PublishedUse Use |
|
♿ Accessibility (axe) — components changed in this PR7 issues across 4 stories — all non-blocking (
Scope: only stories in the files/component folders this PR changed. It can't yet flag downstream ripple from shared-code/token changes, or diff against |
|
The comments regarding Accesiblity were not added in this PR |
…laceable A nested row kept one subscription for all its children pages and accumulated emissions into a single flat list. Both halves broke once a second page existed: loading it unsubscribed the first, and the reducer appended to whatever the list held when that page subscribed. So the rows loaded earlier froze at that snapshot, a reordered slice duplicated one of them, and rows removed upstream stayed on screen until a filter change reset the row. Keep children per requested page and one subscription per page. A page that re-emits replaces its own slice, the flat list is recomposed in page order, and only the highest page loaded owns `hasMore`/`currentPage` so an earlier page cannot rewind the cursor. The "See more" label was a hardcoded English string; it now reads from `collections.table.seeMoreChildren`. The story mock echoed the whole children array back with `hasMore` pinned true, so the only place this was exercised appended duplicates by design and hid all of the above. It now returns a real page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
deac915 to
0fa0d9c
Compare
factmarc10010
changed the title
2 participants
What
A nested row now keeps one subscription per children page, and children are stored per page rather than in a single flat list. A page that re-emits replaces its own slice; the flat list is recomposed in page order.
Also here: the "See more" label reads from
collections.table.seeMoreChildreninstead of being a hardcoded English string, and the story mock returns a real page instead of echoing the whole array back.Screen.Recording.2026-08-25.at.17.24.44.mov
Why
useLoadChildrenheld a singlesubscriptionRefand accumulated every emission into one list. Both halves break as soon as a second page exists:loadChildrenunsubscribes the previous page before requesting the next one, so only the last page can still receive updates from the consumer.processChildrenDatadid[...children, ...loadedChildren]withchildrencaptured in its closure. The subscription'snexthandler holds one instance of that callback, so every later emission rebuilds from the list as it was when that page subscribed.Together that means, for any consumer whose children can change after they load:
None of it was reachable from the consumer side: the frozen baseline lives in this hook.
The reason it went unnoticed is the last commit in the diff. The only place children pagination was exercised — the OneDataCollection story mock — returned
item.childrenin full on every call withhasMorepinned totrue, so "See more" appended the same records forever. That shape can't surface any of the three rows above.How
pagesRef: Map<page, R[]>— the records of each requested page.processChildrenDatatakes the page it is processing, sets that entry, and recomposes the flat list by page order. It no longer depends onchildren, so its identity is stable and there is no captured baseline.subscriptionsRef: Map<page, Subscription>—loadChildrenreplaces only the subscription for the page it is about to request. Earlier pages keep listening. Unmount and the filters/sortings reset unsubscribe all of them.frontierRef—hasMoreandcurrentPagedescribe the frontier, so only the highest page loaded writes them. Without this, an earlier page re-emitting would rewind the cursor and re-offer "See more".0holds the children restored from the cache on a remount: their subscriptions died with the previous mount, so nothing can re-emit them, and they sort before page 1.Compatibility. A consumer that returns no
paginationInfohas a single page, which is replaced on every emission — the same effective behaviour as before. A consumer that returns a constantcurrentPage(the old mock's shape) still accumulates, because the requested page comes from the pagination state handed back to it, not from the response.Testing
useLoadChildren.test.tsxis new — the hook had no unit test. 8 cases, each driving a long-lived observable per page so a specific page can be re-emitted, which is exactly what the flat accumulator could not express.Against
main, 4 of the 8 fail: every loaded page staying subscribed, an update to an earlier page landing, a boundary shift not duplicating, and the filters reset unsubscribing every page. The other 4 pass on both and are there as guards on behaviour this must not change (page-by-page append, the frontier owning the cursor, a duplicate same-page request superseding itself, removals dropping out).Full unit suite green (
pnpm vitest:ci), except 6 pre-existing failures inF0DataChart/__tests__/BarChart.test.tsxthat also fail on a cleanorigin/main.tsc --noEmitandoxlintclean.Note for consumers — the flagged API change
The breaking-API bot flags the new
collections.table.seeMoreChildrenkey and asks for a major. It ships as a minor — hencefeat, even though the substance of the change is a fix:withDefaultsini18n-provideralready fills a consumer's gaps from the English defaults, recursively — it exists precisely so a dictionary written against an older version degrades to English instead of renderingundefined.TranslationShapemakes every key required, so a consumer that maintains a full dictionary gets atscerror naming the key. That error is the mechanism by which consumers learn there is something new to translate; making the shape partial would trade it for silently shipping English.feat(...)—periods.*(feat(DateNavigator): select consumer-defined periods #5161),lockColumn/unlockColumn(feat(OneDataCollection): freeze table columns from settings #5188, the adjacent block incollections.table),dropWidgetToDiscuss(feat(ai): drop a dashboard widget into the chat to quote it #5126). A major per label would contradict all three.A patch would have been the wrong signal in the other direction: it would break a consumer's typecheck without the version anticipating it. So
featit is — one line to add per consumer on bump, which is how every other key has landed.Note on the a11y report
The 6 axe findings are pre-existing and unrelated: they are on
empty-states.stories.tsx, which this PR does not touch, and the samenested-interactive/target-sizepairs appear on #5231 and #5225. They are in scope only because this PR edits__stories__/mockData.tsx, in the same folder. The flagged stories render an empty or failed collection, so no children row — and therefore no code path from this diff — is involved.