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
56 changes: 56 additions & 0 deletions .changeset/8221-retire-legacy-string-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
'@object-ui/core': minor
'@object-ui/types': minor
'@object-ui/app-shell': minor
'@object-ui/plugin-form': minor
'@object-ui/plugin-timeline': minor
'@object-ui/plugin-view': patch
'@object-ui/plugin-map': patch
---

Retire the legacy string `sort` clause: one spelling, the array
(objectui#8221) — `convertSortToQueryParams` now REFUSES `"name desc"` with a
diagnostic naming `[{ field: 'name', order: 'desc' }]`, instead of lowering it.

**BREAKING for `@object-ui/core` consumers — scored `minor`, not `major`, per
AGENTS.md 版本号策略** (every package is in one fixed group, so a `major` here
would carry all 39 off the `@objectstack` major this repo is pinned to). The
breaking semantics are stated below rather than encoded in the version.

Director ruling, decision batch #77 (2026-09-07), option B. Three faces
disagreed about one key: `@object-ui/core` implemented the string clause
on purpose (`sort-query.ts`, docblock and all), `content/docs/plugins/plugin-map.mdx`
taught it as `sort?: string | SortConfig[]`, and the html tier answered
`type-mismatch` for it because all seven `sort` registrations publish
`type: 'array'` alone — while `@objectstack/spec` refuses the string outright on
`element-record-picker`. Option A (per-block string arms) was rejected by name:
it would make one key mean different things on different blocks.

**What moves.** `convertSortToQueryParams(sort)` narrows from
`string | QuerySortEntry[]` to `QuerySortEntry[]`, and the three declarations
that published a string arm narrow with it — `ObjectGridSchema.sort`,
`ObjectMapSchema.sort` and `ObjectGanttSchema.sort`, in the TypeScript face AND
in the zod mirror, together, because a narrowing that left `z.string()` in the
mirror is the declared-vs-enforced split this change exists to close. The local
`sort` declarations on `LineItemsPanel`, `ObjectTimeline` and
`deriveRelatedLists`'s ListView input narrow the same way.

**What a string does now.** Types are erased, so the signature stops a string
only at compile time; authored JSON and stored `sys_metadata` rows still reach
the sink carrying `"name desc"`. Such a value is REFUSED — the query carries no
`$orderby` — and `console.error` names the array form, quotes what arrived and
states the consequence, once per spelling. A silent `undefined` was the one
outcome the ruling ruled out.

**Measured consequences you may see.** A related list that inherited its child
object's default list-view sort in the legacy spelling stops inheriting it (the
console says so). `@objectstack/spec@17.3.0` still ACCEPTS the string on
`ListViewSchema.sort` and on `RecordRelatedListProps.sort`, so such metadata is
still spec-legal today; the spec-side pull-back is its own card. Two surfaces
are deliberately untouched, because they are a DIFFERENT string dialect that
never reaches this sink: `record:related_list`'s `'field'` / `'-field'` form,
normalized by `RelatedList.normalizeSortSpec`, and `ListView.parseSortConfig`,
which reads the platform view record the spec still blesses.

Docs teach the array only: `content/docs/plugins/plugin-map.mdx`,
`content/docs/plugins/plugin-view.mdx` and `packages/plugin-view/README.md`.
2 changes: 1 addition & 1 deletion content/docs/plugins/plugin-map.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const schema: ObjectMapSchema = {
data?: ViewData, // Advanced data configuration (read first)
// At least one of data / staticData / objectName is required
filter?: Array<any>, // Query filter, sent as $filter
sort?: string | SortConfig[], // Sort, sent as $orderby
sort?: SortConfig[], // Sort, sent as $orderby
map?: ObjectMapConfig, // Map-specific configuration
enableClustering?: boolean, // Cluster nearby markers (auto past 100)
navigation?: NavigationConfig, // Record navigation (drawer/dialog/page)
Expand Down
4 changes: 2 additions & 2 deletions content/docs/plugins/plugin-view.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ the canonical one:
| `pagination: { pageSize, pageSizeOptions? }` | `pageSize: number` |
| `selection: { type: 'single' \| 'multiple' \| 'none' }` | `selectable: boolean \| 'single' \| 'multiple'` |
| `filter: [{ field, operator, value }, …]` (same shape as a named view's `filter`) | `defaultFilters: Record<field, value>` (equality-only) |
| `sort: 'field direction'` or `SortConfig[]` | `defaultSort: { field, order }` (**no string form** — that arity only exists on `sort`) |
| `sort: SortConfig[]` (`[{ field, order }]`) | `defaultSort: { field, order }` (a single entry, not an array) |

Before objectui#5102, `pagination` / `selection` / `filter` / `sort` had **no
read point at all** in this file: an author who wrote the canonical shape
Expand Down Expand Up @@ -269,7 +269,7 @@ const userDirectory: ObjectViewSchema = {
defaultViewType: 'grid',
table: {
columns: ['name', 'email', 'role', 'created_at'],
sort: 'created_at desc', // or [{ field: 'created_at', order: 'desc' }]
sort: [{ field: 'created_at', order: 'desc' }],
},
};
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"searchableFields": ["name", "email", "department"],
"table": {
"columns": ["name", "email", "role", "department", "status"],
"sort": "name asc",
"sort": [{ "field": "name", "order": "asc" }],
"pagination": { "pageSize": 5 }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
* ## The dialect trap this file exists to pin
*
* `ListView.sort` and `record:related_list.sort` declare the SAME union
* (`string | Array<{field, order}>`) and mean DIFFERENT things by the string
* arm:
* (`string | Array<{field, order}>`) in `@objectstack/spec` and mean DIFFERENT
* things by the string arm:
*
* - ListView's string is the legacy space-separated clause, `'seq_no desc'`
* (`@objectstack/spec` `ui/view.zod.ts`, annotated `Legacy "field desc"`);
Expand All @@ -30,18 +30,38 @@
*
* Inheriting the string verbatim therefore does not produce "a sort in another
* notation" — it produces `$orderby` on a field whose NAME is the seven
* characters `seq_no desc`, which no object has. The route taken (and pinned
* below) is to normalize at this boundary, always to the ARRAY arm, through
* `@object-ui/core`'s `convertSortToQueryParams` — the repo's one definition of
* both authored dialects — so no second parser of the legacy string exists to
* drift from it.
* characters `seq_no desc`, which no object has. The route taken is to resolve
* that at this boundary, always to the ARRAY arm, through `@object-ui/core`'s
* `convertSortToQueryParams`, so no second parser of the legacy string exists
* to drift from it. `deriveRelatedLists` is the ONE place that knows it is
* reading a ListView and writing a related list, which is why the resolution
* belongs here and not as a tolerant reader on the consuming end
* (AGENTS.md #0.1).
*
* `deriveRelatedLists` is the ONE place that knows it is reading a ListView and
* writing a related list, which is why the translation belongs here and not as
* a tolerant reader on the consuming end (AGENTS.md #0.1).
* ## What objectui#8221 changed, and what it did NOT
*
* Decision batch #77 (option B) RETIRED the legacy space-separated clause:
* `convertSortToQueryParams` no longer lowers it, it REFUSES it with a
* diagnostic naming the array form. So this boundary no longer TRANSLATES a
* ListView string — it drops it and says so, and the pins below moved with it.
*
* ⚠️ The trap the old translation prevented is still prevented, and that is the
* assertion worth keeping: a legacy string must never reach the wire as a FIELD
* NAME. "Refused, loudly" and "translated" both satisfy that; "forwarded
* verbatim" does not, and is what a later well-meaning simplification here
* would reintroduce.
*
* ⚠️ Measured, and the reason this is a behaviour change rather than a
* tidy-up: `@objectstack/spec@17.3.0`'s `ListViewSchema.sort` STILL accepts the
* string (`'name desc'` parses; `42` is refused `invalid_union`; a `bogusProp`
* control is refused by name on the same call). A platform view carrying the
* legacy clause is therefore still spec-legal and stops being inherited here.
* The spec-side pull-back is its own card; until it lands, this diagnostic is
* the only thing standing between an operator and a silently unordered list.
*/

import { describe, it, expect } from 'vitest';
import { describe, it, expect, vi } from 'vitest';
import { resetRetiredSortSpellingReports } from '@object-ui/core';
import { deriveRelatedLists } from '../deriveRelatedLists';

const PARENT = { name: 'task_version', label: 'Task Version', fields: {} };
Expand Down Expand Up @@ -89,23 +109,49 @@ describe('deriveRelatedLists — inherited default list-view sort (objectui#5795
]);
});

it('THE DIALECT PIN — normalizes the legacy space-separated string arm', () => {
const entry = derive(childWithList({ sort: 'seq_no desc' }));
expect(entry.sort).toEqual([{ field: 'seq_no', order: 'desc' }]);
// Stated as its own assertion because it is the whole failure mode: an
// un-normalized inherit yields a FIELD literally named `seq_no desc`.
expect(entry.sort?.[0].field).toBe('seq_no');
expect(entry.sort?.[0].field).not.toBe('seq_no desc');
});
it('THE RETIREMENT PIN — a legacy string arm is REFUSED, and refused OUT LOUD (objectui#8221)', () => {
resetRetiredSortSpellingReports();
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
const entry = derive(childWithList({ sort: 'seq_no desc' }));

it('reads a bare legacy string as ascending', () => {
expect(derive(childWithList({ sort: 'seq_no' })).sort).toEqual([
{ field: 'seq_no', order: 'asc' },
]);
// Nothing is inherited: the key is ABSENT, exactly as for a child that
// declared no order at all.
expect('sort' in entry).toBe(false);
// The list itself is still derived — so the missing key means "this
// order was refused", not "this derivation collapsed".
expect(entry.childObject).toBe('check_item');

// The original failure mode stays impossible: the seven characters
// `seq_no desc` must never travel as a FIELD NAME.
expect(JSON.stringify(entry)).not.toContain('seq_no desc');

// And it is LOUD. A silent drop here is an operator's row order
// disappearing with nothing in the console to explain it.
expect(errorSpy).toHaveBeenCalledTimes(1);
const message = String(errorSpy.mock.calls[0][0]);
expect(message).toContain("[{ field: 'name', order: 'desc' }]");
expect(message).toContain('"seq_no desc"');
} finally {
errorSpy.mockRestore();
}
});

it('is case-insensitive about the legacy direction word', () => {
expect(derive(childWithList({ sort: 'seq_no DESC' })).sort).toEqual([
it('the other legacy spellings are refused the same way', () => {
for (const spelling of ['seq_no', 'seq_no DESC']) {
resetRetiredSortSpellingReports();
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
expect('sort' in derive(childWithList({ sort: spelling }))).toBe(false);
expect(errorSpy).toHaveBeenCalledTimes(1);
} finally {
errorSpy.mockRestore();
}
}

// CONTROL — on the same derivation, the array arm still inherits, so the
// refusals above are about the SPELLING and not a broken boundary.
expect(derive(childWithList({ sort: [{ field: 'seq_no', order: 'desc' }] })).sort).toEqual([
{ field: 'seq_no', order: 'desc' },
]);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/app-shell/src/utils/deriveRelatedLists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ interface ObjectLike {
* fresh array once the views merge, so the memo over this derivation
* recomputes and the sort appears.
*/
list?: { sort?: string | Array<{ field?: string; order?: 'asc' | 'desc' }> };
list?: { sort?: Array<{ field?: string; order?: 'asc' | 'desc' }> };
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, waitFor, cleanup } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';
import { MetadataCtx } from '@object-ui/react';
import { resetRetiredSortSpellingReports } from '@object-ui/core';

vi.mock('@object-ui/auth', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
Expand Down Expand Up @@ -251,13 +252,31 @@ describe('derived related list — inherited $orderby on the wire (objectui#5795
expect(params.$orderby).toEqual([{ field: 'seq_no', order: 'desc' }]);
});

it('DIALECT — the legacy space-separated string arm reaches the wire normalized', async () => {
const params = await childQueryParams({ sort: 'seq_no desc' });
expect(params.$orderby).toEqual([{ field: 'seq_no', order: 'desc' }]);
// The failure this leg exists for, stated so a regression reads plainly:
// an un-normalized inherit orders by a FIELD NAMED `seq_no desc`.
expect(params.$orderby[0].field).toBe('seq_no');
expect(JSON.stringify(params.$orderby)).not.toContain('seq_no desc');
it('RETIRED DIALECT — a legacy string arm sends NO $orderby, and never a field named for the clause (objectui#8221)', async () => {
resetRetiredSortSpellingReports();
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
const params = await childQueryParams({ sort: 'seq_no desc' });

// Refused end to end: the retirement reaches the wire, not just the
// helper's unit test.
expect('$orderby' in params).toBe(false);
// The failure this leg has always existed for, stated so a regression
// reads plainly: an un-normalized inherit orders by a FIELD NAMED
// `seq_no desc`. Refusal prevents it just as translation did; forwarding
// the string verbatim would not.
expect(JSON.stringify(params)).not.toContain('seq_no desc');
// LIVE CONTROL — the query really ran and really is the related list's
// own, so the absent `$orderby` means "refused", not "nothing fetched".
expect(params.$filter).toEqual({ [PARENT]: RECORD_ID });
expect(params.$top).toBeGreaterThan(0);

// And the operator is told why their order vanished.
expect(errorSpy).toHaveBeenCalled();
expect(String(errorSpy.mock.calls[0][0])).toContain("[{ field: 'name', order: 'desc' }]");
} finally {
errorSpy.mockRestore();
}
});

it('COUNTER-PROBE — no declared sort sends NO $orderby, and the list still works', async () => {
Expand Down
Loading
Loading