fix(plugin-grid): refuse the retired string sort clause at object-grid's own read site (objectui#8767)

.changeset/8767-object-grid-refuses-string-sort.md

@@ -0,0 +1,62 @@
+---
+'@object-ui/plugin-grid': minor
+---
+
+`object-grid` REFUSES the retired string `sort` clause at its own read site
+(objectui#8767, maintainer ruling 2026-09-10 — route C).
+
+**Breaking, deliberately.** A `sort: "name desc"` on an `object-grid` node no
+longer reaches `$orderby`. It is reported once per spelling with the diagnostic
+objectui#8221 / PR #8758 already ship — the message names the offending value
+and prescribes the array form — and the query goes out carrying no ordering at
+all, exactly as the same key already behaves on `object-view`.
+
+**Why it was still lowering.** The #8221 ruling retired the legacy string
+clause: one spelling, the array, everywhere. PR #8758 narrowed the shared sink
+`convertSortToQueryParams` and every declaration that published a string arm,
+but `ObjectGrid` never used that sink — it reads `schema.sort` and lowers it
+with private code, so a bare grid went on honouring at runtime a spelling
+`object-view` refuses. One key, two meanings, chosen by which block you are on
+— which is the per-block divergence the ruling declined by name when it
+rejected option A.
+
+**Migration.** Write the array: `sort: [{ field: 'name', order: 'desc' }]`.
+**Both keys are required.** `SortConfig.order` carries no `?` in
+`@object-ui/types` (`packages/types/src/objectql.ts`) and no `.optional()` in
+its zod mirror, and the protocol's own reusable `SortItemSchema` requires
+`order` as well — measured: that schema refuses `[{ field: 'name' }]` with
+`invalid_value` at `0.order`. Do not omit it: this block's array arm
+interpolates whatever is present, so an omitted `order` lowers to
+`$orderby: 'name undefined'` today. That is pre-existing behaviour on the arm
+this change does not touch, and it is filed as a successor card rather than
+widened into here.
+
+**What the spec face does and does not say.** Measured against the installed
+`@objectstack/spec@17.4.0`: `ui.ObjectGridPropsSchema` is **value-agnostic** on
+this key — `sort` is `z.unknown().optional()`, so `safeParse` accepts
+`'name desc'`, `'name'`, `42`, `['name desc']` and `{ name: 'desc' }` alike,
+while an undeclared `bogusProp` is refused with `unrecognized_keys` (the
+control that shows those parse readings are real and not a schema that accepts
+everything). So the protocol's **validator** does not refuse the string, and
+this change is not a narrowing the validator already performed.
+
+What the protocol **declares and documents** is the array, in three places:
+that same key's own `describe` reads `Initial sort (array of { field, order })`;
+the sibling `ElementRecordPickerPropsSchema.sort` spells the identical intent as
+a typed `z.array(SortItemSchema)`, which refuses a string outright; and the
+`defaultSort` retirement text instructs authors to rename the key to `sort` and
+`wrap the value in an array`. The array is likewise the only spelling
+`ObjectGridSchema.sort` has declared since #8221, the only one the registered
+`sort` input publishes (`type: 'array'`), and the only one
+`convertSortToQueryParams` lowers. So type-checked metadata is already on it,
+and only untyped JSON or a stored `sys_metadata` row can still carry the string
+— which is exactly why the refusal is a loud runtime diagnostic rather than a
+type change.
+
+**What is deliberately unchanged.** The wire shape. The array arm still lowers
+to this block's own `"field order[, field order]"` join string, and the export
+path and the header-arrow reader `parseSchemaSort` still read the key exactly as
+before. Routing the whole key through the shared sink would send its
+`{ field: direction }` map where every grid today sends a string; that is a
+separate change with its own blast radius, and this ruling explicitly did not
+take it.

packages/plugin-grid/src/ObjectGrid.tsx

@@ -36,7 +36,7 @@ import {
   RefreshIndicator,
 } from '@object-ui/components';
 import { usePullToRefresh } from '@object-ui/mobile';
-import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName } from '@object-ui/core';
+import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, isObjectInlineEditable, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, convertSortToQueryParams, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName } from '@object-ui/core';
 import { usePermissions } from '@object-ui/permissions';
 import { ChevronRight, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react';
 import { useRowColor } from './useRowColor';
@@ -59,13 +59,22 @@ import type { BulkActionDef } from '@object-ui/types';
 /**
  * A view's declared `sort` → the shape the table's header indicators read.
  *
- * `@objectstack/spec` allows `"name desc"`, `["name desc", …]` and
- * `[{ field, order }, …]`, and this grid's own fetch path already reads all
- * three. The headers have to agree with it: a view that arrives sorted by
+ * `[{ field, order }, …]` is the ONE spelling `@objectstack/spec` still
+ * declares (objectui#8221 retired the string clauses). The headers have to
+ * agree with the fetch path on it: a view that arrives sorted by
  * `created_at desc` should show that arrow before anyone clicks anything —
  * otherwise the first click on that column produces `asc` while the list was
  * already `desc`, and the arrow tells the truth only from the second click on.
  *
+ * ⚠️ This reader is WIDER than the fetch path as of objectui#8767, and the
+ * sentence this docblock used to carry — that the fetch path reads all three
+ * spellings — is no longer true. The fetch path now REFUSES a string and sends
+ * no `$orderby` at all, while this reader still parses `"name desc"` and
+ * `["name desc", …]`, so a grid authored with a retired spelling shows an
+ * arrow for an ordering its query does not carry. Narrowing this reader moves
+ * the wire shape and takes the export path with it — the route the #8767
+ * ruling deliberately did not take. It is NOT fixed here.
+ *
  * Exported for the test that pins it against the fetch path's own reading.
  */
 export function parseSchemaSort(sort: unknown): TableSortItem[] {
@@ -1851,7 +1860,30 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
             params.$orderby = headerSort.map((s) => ({ field: s.field, order: s.order }));
           } else if (schemaSort) {
             if (typeof schemaSort === 'string') {
-              params.$orderby = schemaSort;
+              // objectui#8767 — the legacy string `sort` clause is RETIRED
+              // (objectui#8221, decision batch #77) and is REFUSED here rather
+              // than lowered. This block owns a PRIVATE lowering, so #8758's
+              // narrowing of the shared sink never reached it: a bare
+              // `object-grid` went on honouring a spelling `object-view`
+              // already refuses — one key meaning two things depending on which
+              // block you are on, which is the per-block divergence the #8221
+              // ruling declined by name.
+              //
+              // The refusal is #8758's OWN, not a second one: calling the
+              // shared sink on this arm reports the retired spelling once per
+              // spelling and answers `undefined`, so the query carries no
+              // `$orderby`. Its return value is deliberately unused — the wire
+              // shape stays this block's, and the array arm below is untouched
+              // (with it the export path and `parseSchemaSort`). Routing the
+              // whole key through the sink is a different card: it would send
+              // the sink's `{field: direction}` map where every grid today
+              // sends a `"field order"` string.
+              //
+              // Read through `unknown`, exactly as the sink does: types are
+              // erased, so the array-only `ObjectGridSchema.sort` declaration
+              // cannot stop a string arriving from authored JSON, a stored
+              // `sys_metadata` row or an `as any` bag.
+              convertSortToQueryParams(schemaSort as unknown as QuerySortEntry[]);
             } else if (Array.isArray(schemaSort)) {
               params.$orderby = schemaSort
                 .map((s: any) => `${s.field} ${s.order}`)
@@ -4023,6 +4055,11 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
   // arrow, and the first click on that column would ask for `asc` on a list
   // that was already `desc`.
   //
+  // ⚠️ One spelling now escapes that agreement: since objectui#8767 the fetch
+  // path REFUSES a string `sort` and sends no `$orderby`, while
+  // {@link parseSchemaSort} still parses one. Closing that gap narrows this
+  // reader and moves the wire shape with it — the route #8767 did not take.
+  //
   // A plain expression, not a `useMemo`: this sits below the component's early
   // returns, where a hook would be skipped on some renders and change the hook
   // order. Parsing at most a handful of sort keys costs nothing worth a hook.

packages/plugin-grid/src/__tests__/gridRetiredStringSort-8767.test.tsx

@@ -0,0 +1,173 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * `object-grid` REFUSES the retired string `sort` clause (objectui#8767).
+ *
+ * ## What was wrong
+ *
+ * The #8221 ruling (decision batch #77) retired the legacy OData-ish string
+ * clause: one spelling, the array, everywhere. PR #8758 narrowed the shared
+ * sink `convertSortToQueryParams` so a string that still arrives at runtime is
+ * refused out loud and the query carries no `$orderby`.
+ *
+ * `ObjectGrid` never went through that sink. It reads `schema.sort` at its own
+ * site and lowers it with private code, so after #8758 a bare `object-grid`
+ * still forwarded a string verbatim to `$orderby` while the SAME key routed
+ * through `object-view` was refused with a diagnostic — one key meaning two
+ * things depending on which block you are on. That is precisely the shape the
+ * ruling rejected when it declined option A by name:
+ *
+ *   > per-block arms would make one key mean different things on different
+ *   > blocks and keep a spelling the spec already refuses on one of them.
+ *
+ * ## What these tests pin, and what they deliberately do NOT
+ *
+ * Route C, as ruled: the string arm is refused with #8758's own reporter, and
+ * NOTHING else about this block moves. So the file has two halves, and the
+ * second is what stops it passing by refusing everything:
+ *
+ *   1. a string `sort` reaches no `$orderby`, and says so once per spelling;
+ *   2. the array arm still lowers to the very same `"field order"` join string
+ *      this block has always sent — byte for byte, single- and multi-key.
+ *
+ * The join string is the wire shape route B would change and route C keeps.
+ * The header-arrow reader `parseSchemaSort` and the export path read the same
+ * key and are untouched here; they belong to that other card.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, waitFor } from '@testing-library/react';
+import React from 'react';
+import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react';
+import { resetRetiredSortSpellingReports } from '@object-ui/core';
+// Registers `object-grid` and its `view:grid` alias.
+import '../index';
+
+function makeAdapter() {
+  return {
+    find: vi.fn().mockResolvedValue({
+      data: [{ id: '1', name: 'Acme', status: 'active' }],
+      total: 1,
+    }),
+    findOne: vi.fn(),
+    create: vi.fn(),
+    update: vi.fn(),
+    delete: vi.fn(),
+    getObjectSchema: vi.fn().mockResolvedValue({
+      name: 'account',
+      fields: { id: { type: 'text' }, name: { type: 'text' }, status: { type: 'text' } },
+    }),
+  };
+}
+
+/**
+ * Render the block and return the params of its first `find` call — the same
+ * harness shape as the sibling `gridDefaultFiltersLowering.test.tsx`, so both
+ * legs of this read site are observed through one lens.
+ */
+async function findParamsFor(schema: Record<string, unknown>) {
+  const adapter = makeAdapter();
+  render(
+    <SchemaRendererProvider dataSource={adapter as any}>
+      <SchemaRenderer schema={schema as any} />
+    </SchemaRendererProvider>,
+  );
+  await waitFor(() => expect(adapter.find).toHaveBeenCalled());
+  return (adapter.find.mock.calls[0] as [string, any])[1];
+}
+
+const BASE = { type: 'object-grid', objectName: 'account', columns: [{ field: 'name' }] };
+
+/**
+ * The retired spelling, reached the only way it still can be. The declaration
+ * is `SortConfig[]` since #8221, so a string arrives from authored JSON, a
+ * stored `sys_metadata` row or an `as any` bag — never from a typed caller.
+ */
+const authoredAtRuntime = (sort: unknown) => ({ ...BASE, sort }) as Record<string, unknown>;
+
+let errorSpy: ReturnType<typeof vi.spyOn>;
+
+beforeEach(() => {
+  // The reporter dedupes per spelling in MODULE state. Without this reset the
+  // second test to assert the diagnostic would observe silence and pass for
+  // the wrong reason.
+  resetRetiredSortSpellingReports();
+  errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+});
+
+afterEach(() => {
+  errorSpy.mockRestore();
+});
+
+describe('object-grid — the retired string `sort` clause is REFUSED at this block’s own read site (objectui#8767)', () => {
+  it('carries NO `$orderby` for a string `sort`, and names the array form', async () => {
+    const params = await findParamsFor(authoredAtRuntime('name desc'));
+
+    // The defect in one line: this used to be the string `'name desc'`, i.e.
+    // the grid honoured at runtime what `object-view` refuses.
+    expect(params.$orderby).toBeUndefined();
+    expect(Object.prototype.hasOwnProperty.call(params, '$orderby')).toBe(false);
+
+    expect(errorSpy).toHaveBeenCalled();
+    const message = String(errorSpy.mock.calls[0][0]);
+    // #8758's OWN diagnostic, not a second one written here: it quotes the
+    // offending spelling and prescribes the array form, because a refusal with
+    // no prescription only moves the author's problem.
+    expect(message).toContain('"name desc"');
+    expect(message).toContain("[{ field: 'name', order: 'desc' }]");
+    expect(message).toContain('objectui#8221');
+  });
+
+  it('refuses a bare field string too — every retired spelling, not just the two-word one', async () => {
+    const params = await findParamsFor(authoredAtRuntime('name'));
+    expect(params.$orderby).toBeUndefined();
+    expect(errorSpy).toHaveBeenCalled();
+  });
+
+  it('reports once per spelling, not once per grid', async () => {
+    await findParamsFor(authoredAtRuntime('name desc'));
+    await findParamsFor(authoredAtRuntime('name desc'));
+    // Two blocks inheriting the same bad view sort print one line between them
+    // — the dedupe is the reporter's, and reusing it is what keeps this read
+    // site from becoming a second, differently-behaved diagnostic.
+    expect(errorSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not fall through to the legacy `defaultSort` leg when it refuses', async () => {
+    // A refusal that quietly handed the query to the next arm would be a
+    // silent substitution — a different ordering than either the author asked
+    // for or the refusal announced.
+    const params = await findParamsFor({
+      ...authoredAtRuntime('name desc'),
+      defaultSort: { field: 'status', order: 'asc' },
+    });
+    expect(params.$orderby).toBeUndefined();
+  });
+});
+
+describe('CONTROL — the array arm lowers UNCHANGED (route C keeps this block’s wire shape)', () => {
+  it('still sends the single-key `"field order"` join string', async () => {
+    const params = await findParamsFor({ ...BASE, sort: [{ field: 'name', order: 'desc' }] });
+    // Byte-identical to what this block sent before #8767. Route B would send
+    // the shared sink's `{ name: 'desc' }` map here; that is a different card,
+    // and this line is what would catch it arriving by accident.
+    expect(params.$orderby).toBe('name desc');
+    expect(errorSpy).not.toHaveBeenCalled();
+  });
+
+  it('still joins a multi-key array with `, `', async () => {
+    const params = await findParamsFor({
+      ...BASE,
+      sort: [
+        { field: 'status', order: 'asc' },
+        { field: 'name', order: 'desc' },
+      ],
+    });
+    expect(params.$orderby).toBe('status asc, name desc');
+    expect(errorSpy).not.toHaveBeenCalled();
+  });
+});