From 185af3555d3df35ca522a1fd4b8516877594f44f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 03:14:35 +0000 Subject: [PATCH] fix(plugin-view): wrap table.defaultSort before handing it to the delegated list view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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` 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 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .changeset/6235-mergedsort-wrap.md | 39 +++++++ packages/plugin-view/src/ObjectView.tsx | 32 ++++- .../ObjectView.canonicalTableKeys.test.tsx | 16 ++- .../ObjectView.filterSources.test.tsx | 109 ++++++++++++++++-- 4 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 .changeset/6235-mergedsort-wrap.md diff --git a/.changeset/6235-mergedsort-wrap.md b/.changeset/6235-mergedsort-wrap.md new file mode 100644 index 0000000000..7c90482826 --- /dev/null +++ b/.changeset/6235-mergedsort-wrap.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-view': patch +--- + +`ObjectView` wraps `table.defaultSort` before handing it to a delegated list view, so a +view whose only ordering is the deprecated key actually sorts (objectui#6235). + +`ObjectGridSchema.defaultSort` is declared a SINGLE `{ field, order }` object — the zod +mirror agrees (`z.object({ field, order })`, not a union) — while the `list-view` node's +`sort` slot is declared `string | SortConfig[]`, imported by reference from the spec's own +`ListViewSchema`. `mergedSort`'s last branch forwarded the bare object into that slot +unwrapped. The three branches ahead of it all produce an array or a string, so this was the +one shape the slot never declared. + +Nothing crashed and nothing warned: every reader of that slot drops an unparseable sort +silently. `ListView.parseSortConfig` and `ObjectGrid.parseSchemaSort` both open +`typeof sort === 'string' ? [sort] : Array.isArray(sort) ? sort : []`, so a bare object +yields `[]`; the shared sink `convertSortToQueryParams` returns `undefined` for it. Both +in-tree hosts feed the slot straight into `ListView` (`app-shell`'s `fullSchema` and +Studio's `renderStudioGridList`), so the symptom was an unsorted list with no error — +while the SAME metadata sorted correctly as a grid, because `ObjectGrid` performs this +lowering for the same pair. + +The wrap is verbatim the one the non-grid fetch path in this same file already applies +(`|| (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined)`), so all three +consumers now agree and no fourth dialect is introduced. The shared sink is deliberately +NOT widened to accept a bare `{ field, order }`: that is the widening the maintainer ruling +of 2026-08-22 rejected on the merits, because the same slot legitimately carries +`$orderby`'s own `Record` map, in which `{ field: 'desc' }` is a legal +ordering by a column literally named `field`. + +Precedence is unchanged — a named view's sort still outranks `table.sort`, which still +outranks `table.defaultSort`. Only the final branch changes shape. + +One behaviour note for hosts writing off-schema metadata: an ARRAY in `table.defaultSort` +was previously forwarded verbatim by this path alone and is now lowered like every other +resolver in the repo, which leaves it unreadable rather than rescuing it. That input is +already refused by the zod mirror and already behaves this way on the fetch path and in +`ObjectGrid`; the canonical slot for an array is `table.sort`. diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 0ed68e6a51..fa765c8316 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -1594,6 +1594,36 @@ export const ObjectView: React.FC = ({ // deprecated one as its alias (objectui#5102). Both land on `list-view`'s // own `filter` / `sort` keys below, so a canonical value arrives in the slot // that already matches its shape. + // + // objectui#6235: that last sentence used to be FALSE of the sort chain's + // final branch. `list-view`'s `sort` slot is declared `string | SortConfig[]` + // (the spec's own `ListViewSchema.sort`, imported by reference into + // `packages/types/src/zod/objectql.zod.ts`), and every branch above the last + // produces one of those two — but `table.defaultSort` is declared a SINGLE + // `{ field, order }` object, and it was forwarded BARE. There is no + // compile-time witness: `ObjectViewSchema.table` collapses to a bare index + // signature (objectui#5102) and this node is assembled on the host- + // composition surface (objectui#5097), whose `renderListView` slot types + // `schema` as `any`. + // + // Every reader of that slot then drops the sort SILENTLY — no crash, no + // error, just an unsorted list: `ListView.parseSortConfig` and + // `ObjectGrid.parseSchemaSort` both open `typeof sort === 'string' ? [sort] + // : Array.isArray(sort) ? sort : []`, so a bare object yields `[]`, and the + // shared sink `convertSortToQueryParams` returns `undefined` for it. Both + // in-tree hosts feed this slot straight into `ListView` + // (`app-shell/src/views/ObjectView.tsx` `fullSchema`, and + // `studio-design/StudioDesignSurface.tsx` `renderStudioGridList`). + // + // So the legacy member of the pair is lowered HERE, in the caller, verbatim + // as the non-grid fetch path above already does it and as `ObjectGrid` + // performs it for this exact pair. ⛔ The alternative — teaching the shared + // sink to accept a bare `{ field, order }` — is the widening the maintainer + // ruling of 2026-08-22 REJECTED on the merits (quoted with the non-grid + // fetch above): that slot legitimately also carries `$orderby`'s own + // `Record` map, in which `{ field: 'desc' }` is a legal + // ordering by a column literally named `field`, so the sink would have to + // GUESS. Precedence is untouched — only the last branch changes shape. const mergedFilters = currentNamedViewConfig?.filter || activeView?.filter || schema.table?.filter @@ -1602,7 +1632,7 @@ export const ObjectView: React.FC = ({ const mergedSort = currentNamedViewConfig?.sort || activeView?.sort || schema.table?.sort - || schema.table?.defaultSort; + || (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined); // --- Content renderer --- const renderContent = () => { diff --git a/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx index 5fb2623233..c13d0110ff 100644 --- a/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx +++ b/packages/plugin-view/src/__tests__/ObjectView.canonicalTableKeys.test.tsx @@ -400,8 +400,20 @@ describe('delegated renderListView: canonical first, alias still working', () => expect(s.sort).toBe('name desc'); }); - it('still hands over table.defaultSort alone', () => { + it('still hands over table.defaultSort alone — WRAPPED', () => { + // objectui#6235, and the same transition the `$orderby` assertion above + // made for the fetch path: the legacy spelling is still HONOURED when it + // is the only source (that is objectui#5102's half, and it is what this + // exact-value assertion keeps pinned); it is now honoured in the shape the + // delegated slot declares. + // + // This used to read `{ field: 'created', order: 'asc' }` — the bare object, + // handed to `list-view`'s `sort`, declared `string | SortConfig[]`. Every + // reader of that slot dropped it silently: `ListView.parseSortConfig` and + // `ObjectGrid.parseSchemaSort` return `[]` for a non-array, and the shared + // sink returns `undefined`. So this cell was green while the forwarded + // value could not be read by anything downstream. expect(delegatedSchema({ table: { defaultSort: { field: 'created', order: 'asc' } } as any }).sort) - .toEqual({ field: 'created', order: 'asc' }); + .toEqual([{ field: 'created', order: 'asc' }]); }); }); diff --git a/packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx index 39ff8b63ca..aca147b2f0 100644 --- a/packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx +++ b/packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx @@ -32,6 +32,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, waitFor } from '@testing-library/react'; +import { convertSortToQueryParams } from '@object-ui/core'; import { ObjectView } from '../ObjectView'; import type { ObjectViewSchema } from '@object-ui/types'; @@ -157,23 +158,109 @@ describe('ObjectView hands the view filter to the delegated renderer', () => { expect(seen[0]?.filter).toEqual(rules); }); - // Spelled `order`, the key `ObjectGridSchema.defaultSort` actually declares - // (`packages/types/src/objectql.ts`, and the zod mirror agrees). It used to - // say `direction` — a spelling this surface never declared, retired - // repo-wide by objectui#5293 / objectui#6011. Re-spelling changes nothing - // this test grades: the assertion pins VERBATIM pass-through and never reads - // the direction key, so any word passed. The remaining `as any` is about - // ARITY, not spelling — `defaultSort` is declared a single `{ field, order }` - // and this passes an array on purpose (see ObjectView.namedViewSortArity for - // what the arity costs elsewhere). - it('forwards the sort alongside it', () => { - const seen = renderDelegated({ table: { defaultSort: [{ field: 'name', order: 'asc' }] } as any }); + // ⚠️ CONTROL, green in BOTH states — named so it is not read as evidence + // for objectui#6235. `table.sort` is the slot that legitimately carries an + // array (`ObjectGridSchema.sort: string | SortConfig[]`), and it must reach + // the delegated node UNWRAPPED. What this guards is the wrong shape the fix + // could have taken: wrapping the chain's RESULT instead of its final branch, + // which would re-wrap this into `[[{ field, order }]]` — verbatim + // objectui#5270's failure, where `parseSchemaSort` and + // `ListView.parseSortConfig` both skip a nested-array entry and return `[]`, + // so the user sees no sort at all. Only the LAST branch may change shape; + // this cell is how we know it was the only one. + it('forwards a canonical table.sort array UNWRAPPED', () => { + const seen = renderDelegated({ table: { sort: [{ field: 'name', order: 'asc' }] } as any }); expect(seen[0]?.sort).toEqual([{ field: 'name', order: 'asc' }]); + expect(convertSortToQueryParams(seen[0]?.sort)).toEqual({ name: 'asc' }); + }); + + // This cell used to be `forwards the sort alongside it`, and it asserted that + // an ARRAY in `table.defaultSort` reached the delegated slot verbatim. Two + // things were wrong with it, both recorded by objectui#6235: + // + // 1. It could not fail in either state — a verbatim `toEqual` on a verbatim + // pass-through (objectui#5270's recorded trap). + // 2. Its input is metadata the schema REFUSES. + // `packages/types/src/zod/objectql.zod.ts` declares + // `defaultSort: z.object({ field, order })` — not a union, not an array — + // so no conforming author can produce what it was pinning. + // + // The wrap lowers it verbatim as the non-grid fetch path and `ObjectGrid` do + // for this exact pair, which means invalid input stays invalid instead of + // being rescued. That is the point, not a gap: the shared sink REFUSES it + // rather than guessing, which is the 2026-08-22 ruling's whole basis. + // ⛔ Do not "fix" this with an `Array.isArray` flatten in the caller — that + // is a tolerant second dialect for input the protocol already rejects, and it + // would make this surface disagree with both of its siblings again. + it('does not rescue an ARRAY in defaultSort — the arity the schema refuses', () => { + const seen = renderDelegated({ table: { defaultSort: [{ field: 'name', order: 'asc' }] } as any }); + expect(seen[0]?.sort).toEqual([[{ field: 'name', order: 'asc' }]]); + // Refused, not guessed — the same answer `:862` and ObjectGrid give it. + expect(convertSortToQueryParams(seen[0]?.sort)).toBeUndefined(); + }); + + // ── objectui#6235 — the DECLARED arity, which is where the defect lived ──── + // + // Everything above hands `defaultSort` an array. `ObjectGridSchema` declares + // it a SINGLE `{ field, order }` object, and that shape — the only one an + // author following the type can write — was forwarded BARE into a slot + // declared `string | SortConfig[]` (`list-view`'s `sort`, imported by + // reference from the spec's own `ListViewSchema`). No compile-time witness: + // `ObjectViewSchema.table` is a bare index signature (objectui#5102) and the + // `renderListView` slot types `schema` as `any` (objectui#5097). + // + // These two cells are the discriminating ones. Against `origin/main` the + // first reads `{ field: 'created', order: 'asc' }` and the second reads + // `undefined`; both are green only with the wrap at `mergedSort`. + it('WRAPS a bare-object table.defaultSort into the SortConfig[] the slot declares', () => { + const seen = renderDelegated({ table: { defaultSort: { field: 'created', order: 'asc' } } as any }); + // Before objectui#6235 this was the bare object — the arity the slot does + // not declare, and the one three of the four chain branches never produce. + expect(seen[0]?.sort).toEqual([{ field: 'created', order: 'asc' }]); + }); + + it('hands the delegated slot a sort its READERS can actually parse', () => { + // The symptom, not the shape. A verbatim pass-through assertion cannot + // fail in either state (objectui#5270's recorded trap, and the cell above + // this block is the resident example), so this one runs the forwarded + // value through a real reader of that slot instead. + // + // `convertSortToQueryParams` is the repo's ONE sort sink and is what the + // delegated node's `sort` ultimately reaches on every non-grid view type + // (`ObjectCalendar` / `ObjectMap` / `ObjectTimeline` / `ObjectGantt` each + // call it on `schema.sort`). It refuses a bare `{ field, order }` BY + // DESIGN — the 2026-08-22 maintainer ruling rejected widening it, because + // the same slot legitimately carries `$orderby`'s own + // `Record` map where `{ field: 'desc' }` orders by a + // column named `field`. So the caller must wrap, and until it did, the + // sink returned `undefined`: a silently UNSORTED list, no error anywhere. + // `ListView.parseSortConfig` and `ObjectGrid.parseSchemaSort` — the two + // readers the in-tree hosts reach through — fail the same way, returning + // `[]` from the same `Array.isArray(sort) ? sort : []` opening. + const seen = renderDelegated({ table: { defaultSort: { field: 'created', order: 'asc' } } as any }); + expect(convertSortToQueryParams(seen[0]?.sort)).toEqual({ created: 'asc' }); + }); + + it('keeps the canonical table.sort ahead of the wrapped legacy default', () => { + // CONTROL — green in both states. Named so it is not read as evidence for + // the fix: it guards the wrong shape where the wrap is written so that the + // `defaultSort` branch starts winning (e.g. by wrapping the chain's result + // rather than its final branch, making the always-truthy array outrank + // everything). Precedence is the half of `mergedSort` that must NOT move. + const seen = renderDelegated({ + table: { sort: [{ field: 'name', order: 'desc' }], defaultSort: { field: 'created', order: 'asc' } } as any, + }); + expect(seen[0]?.sort).toEqual([{ field: 'name', order: 'desc' }]); }); it('forwards nothing when the view declares neither', () => { + // CONTROL — green in both states. Guards the other wrong shape: an + // unconditional `[schema.table.defaultSort]`, which forwards `[undefined]` + // when nothing was authored. That is truthy and one entry long, so the + // sink and both parsers would report a sort that does not exist. const seen = renderDelegated({}); expect(seen[0]?.filter).toBeUndefined(); expect(seen[0]?.sort).toBeUndefined(); + expect(convertSortToQueryParams(seen[0]?.sort)).toBeUndefined(); }); });