fix(plugin-list): declare onSortChange/onFilterChange at the types they fire with (objectui#8106) - #8965
Merged
Conversation
…ey fire with (objectui#8106) The objectui#4528 sweep named every other prop on `ListViewProps` "at the type each one actually lands on" and left these two at `any`. `dataSource?: any` carries its own written justification in the same file; these two carried none. Each parameter was measured at its emit site, not read off the declaration, and the two are asymmetric. `sort` crosses one boundary, `emitSortChange`, whose two legs are the array handed in and `filterPlatformSortableSort`'s return - generic in the element, so `SortItem[]` either way; normalized-vs-raw does not move the type here. `filters` is the toolbar `FilterBuilder`'s own `onChange` value passed straight through, so it is the builder's `FilterGroup`, deliberately not the later query-path AST that `normalizeFilters` / `buildEffectiveFilter` speak. Both types were already exported by `@object-ui/components` and already imported by this file. Pinned in the #4528 props-resolution test, including the `IsAny` halves that make a future re-widening fail loudly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…ed line (objectui#8106) `UNGATED_EXAMPLES` keys each accepted `@example` failure by a string that embeds the block's LINE NUMBER. The narrowing in the previous commit added lines above `ListViewHandle`'s doc comment, moving its `@example` tag from line 808 to line 830, so the ledger row matched no block (stale) and the block itself read as an undeclared failure - the same three diagnostics, [2304, 2686], that were already accepted at 808. Only the key's line number changes. The row's `card`, `codes` and `reason` are untouched, no row is added or removed, and the example itself is deliberately left failing. The block was re-located BY ANCHOR - the `@example` tag on `ListViewHandle` - and confirmed against the base, where the same tag sits at 808. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-steve
marked this pull request as ready for review
September 10, 2026 11:53
os-steve
deleted the
claude/issue-8106-listview-callback-param-types
branch
September 10, 2026 12:10
baozhoutao
pushed a commit
that referenced
this pull request
Sep 10, 2026
One conflict, one line, in `scripts/check-doc-example-types.mjs`. `fb828773d` (objectui#8106, PR #8965) had to RE-DERIVE a line-keyed `UNGATED_EXAMPLES` row — `ListView.tsx:808` to `:830` — because its own diff moved lines above an `@example` block it never touched. That is the third instance of this defect in a day, and it landed while clause 3 was abolishing the key shape that causes it. Resolved onto the new key scheme, ⛔ not by picking a side: the row is now `packages/plugin-list/src/ListView.tsx ListViewHandle #1`, and the line number that had to be re-derived is simply gone. Re-derived against the MERGED tree rather than carried: that symbol has exactly one `@example` block in that file, at line 830, so its ordinal is 1. Verified on the merged tree: 124 blocks, 124 unique generated keys, 89 ledger rows, 0 rows naming no block; and against `origin/main` `fb828773d` itself, 89 rows before, 89 after, 89 of 89 carrying an unchanged card / codes / reason under the derived key map, 0 unmapped. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
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
2 participants
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.
Fixes #8106
Notation: this body spells generic shapes out in words rather than in their
angle-bracket form, because this repository has measured that GitHub's body
sanitizer deletes tag-shaped fragments even inside backticks.
What changed
ListViewProps.onSortChangeandonFilterChangeare declared at the types theyactually fire with, instead of
any:onSortChangetakes an array ofSortItem(already exported by@object-ui/components, already imported by this file).onFilterChangetakesFilterGroup(likewise).Each carries a doc comment saying why it lands on that type, matching this
interface's own convention — the same convention that gives
dataSource?: anyits written justification and gave these two none.
Question 2 was a read, and it was read at the call sites
The card's question 2 (normalized value or raw one?) decides its question 1, and
the two callbacks turned out asymmetric:
sort— normalized, but the type does not depend on that. Every emit inthe component crosses one boundary,
emitSortChange(ListView.tsx), and bothof its legs carry the same element type: the array handed in, and
filterPlatformSortableSort's return. That helper is generic in the element —it takes a readonly array of T and returns an array of T — so it preserves
whatever it is given. Normalization decides only whether platform-unsortable
entries are still present (ListView's sort picker persists a platform-refused sort — the in-use exception re-emits it, and
filterPlatformSortableSorthas only one consumer #6455), not the type. The only other occurrence,onSortChange: handleHeaderSort, is a prop handed down to the child grid,not this prop being invoked.
filters— raw. Its one call site passes the toolbarFilterBuilder's ownonChangevalue straight through, beside asetCurrentFiltersthat is itselfReact state of type
FilterGroup. Deliberately not the filter ASTnormalizeFilters/buildEffectiveFilterspeak: those run later on thequery-building path and nothing they produce reaches this callback. The host
gets the builder's group verbatim, which is what lets it round-trip back in
through
initialFilters.No new type was minted and no local near-copy written.
Acceptance — the card's own instrument, proved to fail both ways
Assigning each parameter to a
numberinside the README's "With Callbacks"block and running
pnpm check:doc-snippets. Both runs are single runs carryingall four probes, so the controls and the targets are measured together:
view(ViewType)search(string)sortfiltersThe before-state was measured on this branch, not quoted from the card. Both
probe runs mutated the README on disk under a trap, proved the mutation by anchor
count (4 markers) and blob hash, and restored by state (
git diff HEADempty,blob back to
902c058). The README itself is unmodified by this PR.Consumer sweep — done before the declaration was finalised
Every in-repo file importing
ListViewfrom@object-ui/plugin-listwasenumerated, then every package containing one was type-checked:
packages/app-shell/src/views/ObjectView.tsxonSortChangetwice,onFilterChangeoncepackages/app-shell/src/views/InterfaceListPage.tsxonSortChangepackages/app-shell/src/views/studio-design/StudioDesignSurface.tsxListView, passes neitherpackages/app-shell/src/views/ObjectDataPage.tsxpackages/plugin-map(one test)apps/console(one test)Two corrections to the card's premise, neither of which contradicts the ruling —
both make the sweep smaller:
StudioDesignSurfaceis named by the card as aconsumer that passes these callbacks and does not pass them, and
ObjectDataPageopts out in writing.
The contravariance cost is zero in-repo. Every passing handler annotates its
parameter explicitly (
(sort: any) => ...), and an explicitly-anyhandler staysassignable to the narrowed signature — contravariance permits it, and the body
keeps its
anysemantics. No in-repo handler was contextually typed, so nonechanged meaning and none needed touching. Out-of-repo hosts that destructure or
index the parameter in ways only
anypermitted will now get a real error, whichis the point. This is written into the changeset body as instructed.
Pin, proved two ways
Added to
ListView.propsResolution.test.ts— the objectui#4528 compile-time pinfile, the natural home — four assertions: an exact-signature pin per callback,
plus the
IsAnyhalf per callback that makes a silent re-widening fail loudly(the same discriminating shape as that file's existing pin 2).
pnpm --filter @object-ui/plugin-list type-checkexit 0.anyfails all fournew assertions —
TS2344: Type 'false' does not satisfy the constraint 'true'at lines 106, 109, 112, 115 — exit 2.
anyanchor 0 to 1, blob8f48a25to0435754); restore proved by state(blob back to
8f48a25,git diff HEADempty), never by an exit code. Bothlegs ran from the committed implementation, under a trap with absolute paths.
Source vs dist
The card read
packages/plugin-list/dist/ListView.d.ts, a built artifact. Checkedagainst source: they agreed exactly before the change (source lines 233-237,
dist lines 82-86, identical declarations), so the card's dist read was faithful.
Line numbers had moved, as expected for a card written 2026-09-06; everything was
re-located by anchor. After the change, dist carries the narrowing.
Gates
scripts/pm/dispatch-gates.mjsdoes not exist in objectui, so gate familieswere derived by hand and are declared as such.
That hand-derivation initially omitted the
@example-block gate(
check:doc-examples/scripts/check-doc-example-types.mjs), which is why thefailure below reached CI rather than being caught locally. It is in the table now
and runs at exit 0.
check:doc-snippets(35 packages, turbo)pnpm check:doc-snippets(un-mutated; 638 of 638 blocks)pnpm check:doc-examples(89 of 89 ledger rows matched)pnpm check:doc-typespnpm check:control-bytestype-check:@object-ui/plugin-listtype-check:@object-ui/app-shelltype-check:@object-ui/plugin-maptype-check:@object-ui/consolevitest run packages/plugin-list/from repo ROOT (73 files, 895 tests)lint:@object-ui/plugin-list(0 errors; 499 pre-existing warnings)check-changeset-fixed/-no-major/-overwrite/-presencecheck-governed-queue-guard --teston the changed pathsEach exit code was redirected to a file and captured before being read, never
through a pipe.
@object-ui/console's type-check first came back non-zero with three@object-ui/plugin-treemodule-resolution errors — that package is outside thedoc-gate build closure and so was simply unbuilt. PREREQUISITE NOT MET, not a
verdict; after building
@object-ui/plugin-treeit passes at exit 0. Nodiagnostic in that run mentioned
ListView,SortItemorFilterGroup.Patch round - the ledger row that moved (position axis, not typing)
CI came back red on
Doc Snippet Type Check. The cause is positional, not adefect in the narrowing:
UNGATED_EXAMPLESinscripts/check-doc-example-types.mjskeys each accepted@examplefailure by astring that embeds the line number of the block. The
@exampleonListViewHandlewas already a known, accepted failure. The declaration changeadded lines above it, so the tag moved from line 808 to line 830: the ledger row
matched nothing (stale) and the block re-appeared as undeclared, carrying the same
three diagnostics [2304, 2686] that were already accepted at 808.
Reproduced locally before touching anything - exit 1,
UNDECLARED FAILUREat:830,
STALE LEDGER ROWat :808, 89 rows with 88 matched. The block wasre-located by anchor (the
@exampletag onListViewHandle), never byarithmetic and never by trusting the :830 the gate printed; confirmed against the
base, where the same tag sits at 808. Worth noting the file moved 24 net lines
while the block moved 22, so arithmetic would have been wrong.
Only the line number in the key changed. The
card,codesandreasonof thatrow are untouched, no row was added or removed, and the example itself is left
deliberately failing.
Positive control after the fix: exit 0, zero stale rows, zero
undeclared failures, row count unchanged at 89, and the census moved from
"88 of those declared" to "89 of those declared". A base-vs-branch diff of the
extracted key list shows exactly one line differing, and only in its number:
Because the diff now edits a gate script, that script owns suites that were run
too:
scripts/__tests__/check-doc-example-types.test.tsandcheck-doc-example-shared-reader.test.ts, 80 tests, exit 0. The gate first exited2 on an unbuilt
dist- PREREQUISITE NOT MET, not a green - and wasre-run to a real 0 after building its closure.
This class is a repeat, not a novelty: the same line-number-embedding key bit
objectui#8885 / PR #8895 today. The cross-file citation census cannot see these
keys; only the gate itself catches them.
Changeset
@object-ui/plugin-list:minor— a real changeset, not empty frontmatter;this is a published package and this moves a published type. The repo forbids
major. The narrowing cost is spelled out in the body as instructed.Serial constraint, re-derived twice
First derivation (round 1): the claim measured only PR #8959. Re-measured
across all 8 PRs then open, each with a lit control (total changed files), the
reading had aged - PRs #8941 and #7058 each touched
packages/plugin-list.Second derivation (this patch round, before pushing): 12 PRs now open, 4 of
them new. Re-measured against both file surfaces this branch now holds:
packages/plugin-list/- still only chore(deps): lucide-react 1.31.0 -> 1.43.0, with the one retired spelling repaired #8941 and chore(deps): bump lucide-react from 1.31.0 to 1.41.0 #7058, and in both the single hitis the same one line of
packages/plugin-list/package.json(a lucide-reactdependency bump), disjoint from
ListView.tsx.scripts/check-doc-example-types.mjs(the ledger this round edits) - 0 hitsacross all 11 other open PRs, each with its lit control. This one matters:
a sibling PR editing the same ledger would collide directly.
origin/mainalso advanced 2 commits since this branch was cut; neither touchedListView.tsxnor the ledger, so the base-vs-branch key-list diff above stands.Out of scope, noted and not filed
The four in-repo handler sites still annotate their parameter
(sort: any)/(filter: any), which now shadows the contextual type this PR provides. Droppingthose annotations would let the real types flow into the bodies. That is
packages/app-shellsource, outside this card's declared file surface, andnothing is wrong today. Successor: whoever next edits
ObjectView's persistencehandlers.
Generated by Claude Code