Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-view): wrap table.defaultSort before handing it to the delegated list view - #6479
Merged
Merged
Conversation
…egated list view
`mergedSort`'s last branch forwarded `schema.table.defaultSort` — declared a
SINGLE `{ field, order }` object, and a `z.object(...)` in the zod mirror — into
`list-view`'s `sort` slot, declared `string | SortConfig[]`. The three branches
ahead of it all produce an array or a string, so this was the one shape the slot
never declared.
There is no compile-time witness: `ObjectViewSchema.table` collapses to a bare
index signature and the delegated node is assembled on the host-composition
surface, whose `renderListView` slot types `schema` as `any`. Every reader of the
slot then drops the sort SILENTLY — `ListView.parseSortConfig` and
`ObjectGrid.parseSchemaSort` both return `[]` for a non-array, and the shared sink
`convertSortToQueryParams` returns `undefined` — so the symptom is an unsorted
list with no error, while the same metadata sorts correctly as a grid.
The wrap is verbatim the one the non-grid fetch path in this file already applies,
and the one `ObjectGrid` performs for this exact pair, so all three consumers
agree and no fourth dialect appears. The shared sink is deliberately NOT widened:
that is the widening the maintainer ruling of 2026-08-22 rejected on the merits,
since the same slot legitimately carries `$orderby`'s own
`Record<field, direction>` map where `{ field: 'desc' }` orders by a column named
`field`. Precedence is untouched; only the final branch changes shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qContributor
✅ 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-support-ai
marked this pull request as ready for review
August 26, 2026 04:25
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#6235
Route 1 of the card, which the triage closed by applying the recorded 2026-08-22 maintainer ruling rather than adjudicating afresh. One line of source:
That is byte-identical (modulo indentation) to the wrap the non-grid fetch path in this same file already applies — both sites now read verbatim the same expression, at
:862and:1635.The ruling, quoted from the file's own comment
The triage pointed at
~:779; onorigin/mainit now sits at:850-854(see the line-number note below). Verbatim:So the wrap belongs in the caller. The shared sink is untouched.
The defect
ObjectGridSchema.defaultSortis declared a single{ field, order }object, and the zod mirror agrees —packages/types/src/zod/objectql.zod.ts:130isz.object({ field, order }), not a union and not an array. Thelist-viewnode'ssortslot is declaredstring | SortConfig[], imported by reference from the spec's ownListViewSchema. The three branches ahead of the last all produce an array or a string; only thedefaultSortfallback produced a bare object, so this was the one shape the slot never declared.No compile-time witness:
ObjectViewSchema.tablecollapses to a bare index signature (#5102) and the delegated node is assembled on the host-composition surface (#5097), whoserenderListViewslot typesschemaasany.The symptom is silence. Every reader of that slot drops an unparseable sort without erroring:
ListView.parseSortConfigtypeof sort === 'string' ? [sort] : Array.isArray(sort) ? sort : [][]ObjectGrid.parseSchemaSort[]convertSortToQueryParams(shared sink)undefinedBoth in-tree hosts feed this slot straight into
ListView—app-shell/src/views/ObjectView.tsx(fullSchema,sort: viewDef.sort ?? listSchema.sort) andstudio-design/StudioDesignSurface.tsx(renderStudioGridList,...listSchema). So a view whose only ordering wastable.defaultSortrendered unsorted with no error, while the same metadata sorted correctly as a grid, becauseObjectGridperforms this exact lowering for this exact pair.Fork fence — checked, no fork
The triage said to stop and report if an out-of-tree
renderListViewwere shown to depend on receiving the bare object. Censused by shape ("hands a sort value to the delegated view"), not by one identifier:ListView; both call paths are listed above.grepfor a bare-object read of a delegated sort (sort.field/sort.order) returns nothing on this surface — the only hits areRecordPickerDialog's unrelatedquery.sortandcore/src/query/query-ast.tsiterating array entries.No evidence of a fork. Proceeded with route 1.
Line numbers — the card and the triage have both drifted
The card cites
mergedSortat:1522-1525and its comment at:1513-1516; the triage cites~:1513and the ruling at~:779. My measurement onorigin/main@f7c52e21c:origin/main:779-784:850-854:791:862defaultSort:1375:1455:1513-1516:1593-1596mergedSort:1522-1525:1602-1605This confirms the PM's reading (
mergedSortat:1602) and supersedes the card's. The obligation attaches to the sentence at:1593-1596— "Both land onlist-view's ownfilter/sortkeys below, so a canonical value arrives in the slot that already matches its shape" — which was true of the filter chain and of the sort chain's first three branches, and false of its fourth. That paragraph now states the arity, cites the ruling, and names the readers.Tests
The two discriminating cells (new)
The existing
forwards the sort alongside itwas a ghost: it passed an already-arraydefaultSortbehindas anyand asserted verbatimtoEqual, so it could not fail in either state — #5270's own recorded trap. The new cells pass the declared bare-object arity:WRAPS a bare-object table.defaultSort into the SortConfig[] the slot declares— the arity at the slot.hands the delegated slot a sort its READERS can actually parse— the symptom, not the shape. It runs the forwarded value throughconvertSortToQueryParams, a real reader of that slot. Againstorigin/mainit readsundefined: the silently unsorted list.Controls — all green in both states, each named with the wrong shape it guards
forwards a canonical table.sort array UNWRAPPED— guards wrapping the chain's result instead of its final branch, which would produce[[{field,order}]]: verbatim A named view'ssortarray is forwarded intogridSchema.defaultSort, declared a SINGLE{ field, order }— ObjectGrid re-wraps it andparseSchemaSortdrops it, so the grid shows no sort at all #5270's failure, where both parsers skip a nested-array entry and return[].keeps the canonical table.sort ahead of the wrapped legacy default— guards precedence moving, the half ofmergedSortthat must not change.forwards nothing when the view declares neither— guards an unconditional[schema.table.defaultSort], which would forward[undefined]: truthy, one entry long, a sort that does not exist.Two pre-existing assertions changed — called out, not smoothed
ObjectView.canonicalTableKeys.test.tsx→still hands over table.defaultSort aloneasserted the bare object. It pinsObjectViewSchema的table/form子配置类型零声明成员(Omit 被索引签名塌掉),且 ObjectView 的转发白名单钉在 ObjectGridSchema 的 legacy 键上 —— 规范键 pagination/selection/filter/sort 全部不读 #5102's "alias still honoured" half, so the assertion moved to the wrapped shape — the same transition the$orderbyassertion directly above it already made for the fetch path, whose comment supplied the wording.ObjectView.filterSources.test.tsx→ the old ghost fed an array intodefaultSort, which the zod mirror refuses. Verbatim lowering necessarily changes that input's outcome, so the cell was rewritten to record it: the wrap does not rescue invalid metadata, and the sink refuses it rather than guessing — the same answer:862andObjectGridgive it. It carries a⛔against "fixing" it with anArray.isArrayflatten, which would be a tolerant second dialect for input the protocol already rejects.Reverse verification
Predicted before the run: 4 cells red, every control green.
ObjectView.tsxrestored fromorigin/main(test files kept), mutation proven on disk by grepping the injected and the deleted text plus a blob-hash change, under anEXIT/INT/TERMtrap holding absolute paths.No rebuild needed, and this is verified rather than assumed: the subject is a relative source import (
../ObjectView) andvitest.config.mts:255-268aliases@object-ui/coreand@object-ui/plugin-gridto each package'ssrc, so nodist/sits in the resolution path.The 4 red were exactly the 4 predicted. The money line:
ObjectView.sortSink.test.tsxandObjectView.namedViewSortArity.test.tsxstayed fully green through the ablation — the guard is isolated, not blanket-broken. Restore proven by blob hash equality (fa765c83…both sides) plus an emptygit diff HEAD.Gate verdicts — quoted, exit codes captured before any pipe
All at
185af3555:pnpm exec vitest run packages/plugin-view/→Test Files 24 passed (24)/Tests 235 passed (235), exit 0pnpm --filter @object-ui/plugin-view run type-check→ exit 0 (tsc --noEmit && tsc -p tsconfig.test.json). Both edited pin files confirmed in the program viatsc -p tsconfig.test.json --listFiles.pnpm lint(full repo, not narrowed) →Tasks: 47 successful, 47 total, exit 0check-control-bytes→✅ check-control-bytes: OK (scanned 5350 tracked text file(s); skipped 85 binary).check-vi-mock-specifiers→✅ check-vi-mock-specifiers: OK (3810 tracked source file(s), 2112 test-named; 449 carry a mock; …)changeset:check→✅ All workspace packages are in the changeset fixed group./✅ No changeset declares a major bump.check-changeset-presence→✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/6235-mergedsort-wrap.md.A repo guard (
OBJECTUI_VITEST_GUARD) refusedpnpm --filter … exec vitestas a false-green path; every run above uses the sanctioned repo-root invocation instead. The guard was obeyed, never bypassed.Clause ② — public surface: no change, stated rather than inferred
**/src/index.tstouched;packages/types/untouched; no.d.tstouched.a local
constinside a function body. Everything else in the diff is comments, two test files and the changeset.Note for the reviewer
table.defaultSortis@deprecated Use sort instead, so the card's route 3 (retire it rather than teach a fourth reader) remains live and is explicitly not attempted here — this change makes the three existing consumers agree, which is what the ruling directs. The ADR-0049 retirement is its own card, asObjectView.sortSink.test.tsx's header already records for the sibling site.Changeset:
.changeset/6235-mergedsort-wrap.md(@object-ui/plugin-view: patch), including the off-schema-array behaviour note.Generated by Claude Code