Uh oh!
There was an error while loading. Please reload this page.
refactor(plugin-view)!: retire the direction sort spelling from the published toSortItems export - #6234
Merged
Merged
Conversation
…spelling
`toSortItems` is a published export (re-exported from the package root and
listed in the README). It folded `s.order || s.direction || 'asc'` — two
spellings for one key, silently preferring the canonical one. That is the
tolerance layer objectui#4869 ruled against, and objectui#5293 retired the
same word on `ObjectViewProps.views[].sort`.
The fallback is gone: the export now reads `order` only. An unmigrated caller
passing `{ field, direction }` gets the documented `'asc'` default instead of
the direction it asked for — a silent behaviour break, shipped `minor` per
AGENTS.md 版本号策略 with a changeset naming the retired key so a host greping
`direction` lands on it.
The function had no in-repo production caller, so no existing suite could go
red for this. A dedicated pin now grades the retirement.
Also corrected: the `ObjectViewProps.views` doc paragraph that recorded this
export as deliberately not retired, and a fixture that wrote the retired
spelling behind an `as any` on a surface declaring `order`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7LThe README export line described the return shape but not the accepted one,
so a host greping the retired `direction` key found nothing here either way.
Naming `{ field, order }` puts the surviving spelling in the published doc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7LContributor
✅ 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
|
yinlianghui
marked this pull request as ready for review
August 25, 2026 03:33
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#6011
Retires the
directionsort spelling fromtoSortItems, a published export of@object-ui/plugin-view. The ruling is inherited, not made here: #4869 ruled that aspelling the sink does not recognise gets ruled into the contract or rejected at the
producer, never absorbed by a tolerance layer; #5293 executed the identical retirement for
ObjectViewProps.views[].sortand shipped itminor. This is the same treatment on thelast dormant tolerance layer of the sort family's public surface.
Clause ② — contract review (published-export acceptance change)
What the export accepted before.
toSortItems(draftSort)foldeds.order || s.direction || 'asc'per entry. It therefore accepted two spellings for onekey —
{ field, order }and{ field, direction }— and, when both were present,silently preferred
order. No warning, no rejection, no record that a second spelling wasin play.
What it accepts after.
s.order || 'asc'.orderonly.directionis now anunrecognised key like any other: read by nothing, reported by nothing.
Nothing else about the helper moved.
idis preserved when present and minted withcrypto.randomUUID()otherwise;fieldstill defaults to''; a non-array draft stillyields
[]. The TypeScript signature is byte-identical ((draftSort: any): SortItem[]),so no downstream consumer's types change — this is an acceptance change only, invisible to
the compiler on both sides.
Exact failure mode of an unmigrated caller. A caller passing
{ field: 'created_at', direction: 'desc' }used to get{ id, field: 'created_at', order: 'desc' }. It now gets{ id, field: 'created_at', order: 'asc' }— the documented default for an entry naming nodirection. The break is silent and it is a wrong answer, not an absent one: nothing
throws, nothing warns, the
SortBuilderrow renders, the field is right, and the list sortsascending while the caller asked for descending. That is the whole cost, and it is the
cost #5293's changeset already priced for the sibling surface.
Why the unknown out-of-tree caller is not a blocker. The card is right that this repo
cannot answer whether any host calls it (zero in-repo production callers). objectui's stated
policy — it ships its own breaks as
minorwith a changeset naming the retired key — is theanswer to that unknown, not a reason to keep the tolerant read.
The trap this card carries, and how the change was actually graded
toSortItemshas no in-repo production caller. The one in-repo importer of the publishedsymbol,
apps/console/src/__tests__/insecure-origin-crypto.test.ts, passes the canonicalorderand only gradescrypto.randomUUID. So no pre-existing suite could go red for thischange — a green run of the existing suites proves nothing about the retirement.
The pin was therefore written first and measured red against the unfixed function
(
packages/plugin-view/src/config/__tests__/view-config-utils.toSortItems.test.ts):Both flipped green with the one-line fix; the predicted failing pair and the predicted
received values matched the run exactly.
Which of these assertions would still pass on a revert
Stated plainly, because most of them would:
does NOT read the retired direction spelling — it falls back to asc'desc'.ignores direction even when it is the only key that could answer['desc','desc'].reads orderorderwas always read firstdefaults a sort entry with no direction key at all to asc|| 'asc'tail is unchangedpreserves an explicit id and mints one otherwisereturns an empty list for a non-array draftTwo of six assertions grade the change. The other four are regression cover for the parts
deliberately not touched, and a revert leaves them green. Every other suite in the repo
also stays green on a revert — that is the point of the trap above, not an oversight.
Also in this diff
packages/plugin-view/src/ObjectView.tsx— theviewsprop doc paragraph recordedthis export as folding
s.order || s.directionand "deliberately not retired here(objectui#6011)". True when
ObjectViewProps.views[].sortdeclaresdirection, but every sort consumer readsorder— a host-supplieddescview silently sorts ascending #5293 landed, false the moment this does. Rewritten to say theretirement has since happened on its own card.
packages/plugin-view/src/__tests__/ObjectView.filterSources.test.tsx— the stalefixture the card recorded, which wrote
directionbehind anas anyontable.defaultSort,a surface declaring
order(packages/types/src/objectql.ts, zod mirror agreeing).Re-spelled after reading what it pins: the assertion pins verbatim pass-through of the
forwarded value and never reads the direction key, so any word passed and re-spelling changes
nothing it grades. A comment now records that the surviving
as anyis about arity(
defaultSortis declared a single{ field, order }, the fixture passes an array onpurpose), not spelling — so the canonical key is not misread as declaration conformance.
packages/plugin-view/README.md:91— the documented export line described the returnshape but not the accepted one. Now names
{ field, order }.packages/plugin-view/src/SortUI.tsx— deliberately untouched. Its file-localconst toSortItemsis a different symbol that mapsSortEntry[], anddirectionis the keySortUISchemalegitimately declares. A grep fortoSortItemsreads it as a caller; it is notone.
.changesetentry isminorand namesdirectionin words, so a host grepingthe retired key lands on it. It also records that
ObjectViewProps.views[].sortdeclaresdirection, but every sort consumer readsorder— a host-supplieddescview silently sorts ascending #5293's scope note — which said this exportwas not retired by that change — described that release's scope correctly and is superseded
here.
Verification
All of the following on final head
b0d186b79, with the working tree clean(
git diff HEAD --statempty).vitest run packages/plugin-view apps/console/…/insecure-origin-crypto.test.tsTest Files 24 passed (24) / Tests 237 passed (237)pnpm --filter @object-ui/plugin-view type-checktsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/console type-checkpnpm --filter @object-ui/plugin-view lint✖ 247 problems (0 errors, 247 warnings)— all pre-existingcheck-changeset-presence.mjs✅ 4 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major.mjs✅ No changeset declares a major bump.check-changeset-fixed.mjs✅ All workspace packages are in the changeset fixed group.check:control-bytes✅ check-control-bytes: OK (scanned 5140 tracked text file(s))check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:self-import✅ No package names itself inside its own src/.check:vi-mock-specifiers✅ check-vi-mock-specifiers: OKcheck:shell-escape-residue✅ check-shell-escape-residue: OKlint:coverage✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).type-check:coverage✅ type-check coverage: 45/46 via type-check … 0 errors outstandingNote on the console type-check. It first failed with five
Cannot find moduleerrors for@object-ui/app-shell/plugin-view/plugin-list/plugin-detail/auth— theunbuilt-dependency shape, not this diff. After
pnpm --filter '@object-ui/console^...' buildit is exit 0. Recorded because that red reads identically to "your change broke an import".
Declared narrowing.
pnpm lint(turbo run lint, 46 packages) was not run whole; the@object-ui/plugin-viewlint task was run directly, which is the complete unit turbo would runfor this diff. Evidence the narrowing excludes nothing: (1) the population comes from eslint's
own config resolution, not a hand-picked file list; (2)
--format jsonreports 38 fileslinted, 0 errors, 247 warnings; (3)
eslint.config.jsdeclares noparserOptions.projectand no
projectService, so type-aware linting is off and a change inside this packagecannot move the verdict on any file outside it — and each package's
lintscript iseslint .rooted in its own directory.
lint:coverageindependently reports 46/46 packages at 0 errors.CI runs the full farm regardless.
Generated by Claude Code