Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .changeset/6235-mergedsort-wrap.md
Original file line numberDiff line numberDiff line change
@@ -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<field, direction>` 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`.
32 changes: 31 additions & 1 deletion packages/plugin-view/src/ObjectView.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1594,6 +1594,36 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
// 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<field, direction>` 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
Expand All@@ -1602,7 +1632,7 @@ export const ObjectView: React.FC<ObjectViewProps> = ({
const mergedSort = currentNamedViewConfig?.sort
|| activeView?.sort
|| schema.table?.sort
|| schema.table?.defaultSort;
|| (schema.table?.defaultSort ? [schema.table.defaultSort] : undefined);

// --- Content renderer ---
const renderContent = () => {
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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' }]);
});
});
109 changes: 98 additions & 11 deletions packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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';

Expand DownExpand Up@@ -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<field, direction>` 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();
});
});
Loading