Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-view): spell the views-prop sort direction key order, not direction - #6008
Merged
Merged
Conversation
…t `direction`
`ObjectViewProps.views[].sort` declared `{ field, direction }` while every
consumer of the resolved `activeView.sort` reads `order` — the shared sink
`convertSortToQueryParams`, the grid path via `ObjectGridSchema.sort`, and
`mergedSort`. A host writing the exact shape the prop declared got a silently
ascending list with no failure signal anywhere.
Renaming the key to `order` gives the repo one sort spelling and makes the
retired one a loud type error. Per the 2026-08-24 maintainer ruling this is
deliberately NOT a tolerant dual-read (`direction ?? order`) — that is the
tolerance layer objectui#4869 ruled against.
Producers corrected: the studio adapter round-trip fixtures, and two
`views`-prop fixtures inside plugin-view's own suite that the ruling did not
enumerate. `ObjectView.sortSink.test.tsx` held a pin asserting `{ name: 'asc' }`
for a `direction: 'desc'` fixture — green only because nothing read the key —
which is replaced by an assertion that the direction now survives, plus a
`@ts-expect-error` guard that turns red if the old spelling is ever readmitted.
NOT touched: `SortUI` and its `sort-ui` `defaultProps` in `index.tsx`, which
legitimately own `direction` on their own `SortUISchema`.
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
|
This was referenced Aug 24, 2026
… prop The changeset and the `ObjectViewProps.views` JSDoc both carried an unqualified repo-wide negative — "no reader in the repo has ever known the word" — that one grep in this same package falsifies: `toSortItems` (`src/config/view-config-utils.ts:328`) folds `s.order || s.direction || 'asc'`, and it is a published export (barrel `src/index.tsx:38`, README:91). The scoped claim in the same paragraph — all three consumers of the resolved `activeView.sort` read `order` — is the one that carries the argument and is correct. Both artifacts now make that claim instead, and both name the surviving site so a host migrating `views[].sort` who greps the old key does not read the hit in `toSortItems` as a partial retirement. Retiring that fallback is its own break on its own public export and is tracked separately; it is deliberately untouched here. Prose only: no declaration, no fixture and no runtime line changes. The `views[].sort` declaration, the `@ts-expect-error` guard and the whole code half of this branch are byte-identical to the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
Contributor
✅ 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
commented
Aug 24, 2026
CollaboratorAuthor
Rework pushed — prose only, |
| gate | verdict line |
|---|---|
pnpm --filter @object-ui/plugin-view type-check | exit 0, script name echoed (> @object-ui/plugin-view@17.6.0 type-check), runs tsc --noEmit && tsc -p tsconfig.test.json |
pnpm exec vitest run packages/plugin-view/src --maxWorkers=2 | Test Files 22 passed (22) / Tests 217 passed (217), exit 0 |
pnpm changeset:check | exit 0 — "All workspace packages are in the changeset fixed group" / "No changeset declares a major bump" |
pnpm check:control-bytes | exit 0 — "OK (scanned 4971 tracked text file(s); skipped 85 binary)" |
No test verdict moved, and the accounting is exact rather than eyeballed: the previous round ran packages/plugin-view/srcpluspackages/app-shell/src/views/view-config-adapter.test.ts and reported 23 files / 223 tests; that adapter file declares 6 tests, so 23−1 files and 223−6 tests is precisely this round's 22 / 217. The adapter fixtures are untouched by this commit.
Lint delta measured, not assumed. eslint's own config linted 1 of 1 changed source file (the changeset .md is not in eslint's set), count read from --format json, none ignored: 0 errors / 59 warnings at 54ee7f05b, and 0 errors / 59 warnings for the same file at 604c20366 — delta 0/0. Untouched files are invariant because no type-aware linting is configured (no project / projectService in eslint.config.*). The baseline leg checked the old file out, measured, and restored from the commit, with the injected text grepped at 0 before and 1 after; git status --porcelain empty afterwards.
Control-byte self-scan over both changed files with grep -naP returned no hits, in addition to the repo gate above.
Still draft, still Fixes #5293 in the body, still no tolerant dual-read on the views prop.
Generated by Claude Code
Generated by Claude Code
yinlianghui
marked this pull request as ready for review
August 24, 2026 13:09
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#5293
Implements the maintainer ruling of 2026-08-24 (Route 2), verbatim: 「四维分析一致的，接手你的建议。」
ObjectViewProps.views[].sortdeclared{ field, direction }while every consumer of the resolvedactiveView.sortreadsorder. The key is renamed toorder, matching every other sort surface in the repo and the shared sink. Verification union run at604c2036.Why the break costs nothing
Nothing has ever read
direction. Re-derived on the current tree rather than trusting the card (which was verified at12841b617, five days stale — line numbers had all moved and one consumer had changed mechanism):directionentry yieldsObjectView.tsx:786)convertSortToQueryParams(sort), whoseentry.order === 'desc'is false for a missing key{ name: 'asc' }:1364)ObjectGridSchema.sort→`${s.field} ${s.order}`"name undefined", andparseSchemaSortdraws an ascending arrowmergedSort(:1499)A host writing the exact shape the prop declared got an ascending list with no failure signal. The rename converts a silent wrong answer into a loud type error. That framing is in the changeset, which names the retired spelling so an external host can search for it.
Note the card's stated mechanism for the first consumer no longer holds: #4869 landed, so the non-grid fetch now lowers through the shared sink instead of passing
$orderbyraw. The outcome is identical (entry.orderis equally absent), so the conclusion survives — but it was re-measured, not assumed.⛔ Not a tolerant dual-read
No
direction ?? orderanywhere. That is the tolerance layer #4869 ruled against. A@ts-expect-errorguard inObjectView.sortSink.test.tsxnow turns red if the old key is ever readmitted, so the prohibition is mechanically enforced rather than only documented.The ruling's producer list was wrong in one place — corrected here
The ruling named two in-repo
directionproducers. Measured against the tree, one is misattributed and two more exist:packages/app-shell/src/views/view-config-adapter.test.ts(:18,:82) — round-trip fixtures, corrected. PM assumption 2 confirmed:view-config-adapter.tsitself has 0 hits fordirection(sort?: unknown[]), and the round-trip assertsexpect(back.sort).toEqual(view.sort), so it is spelling-agnostic behaviourally as well as by type. Blast radius is as the ruling priced it.packages/plugin-view/src/index.tsx:177— NOT touched, the ruling misattributed it. ThatdefaultPropsblock belongs toComponentRegistry.register('sort-ui', SortUI, …)(the enclosingregistercall opens at:158), i.e. it is SortUI's own default, wheredirectionis type-correct:SortUISchema.sortdeclaresdirection: 'asc' | 'desc'(packages/types/src/views.ts:862-871) andSortUI.tsx:188/:207readsingleSort.directionat runtime. Renaming it would have broken SortUI and contradicted PM assumption 3, which forbids touching that surface. The ruling's producer list and its own do-not-touch note were in conflict; the note is the half that is right.packages/plugin-view/src/__tests__/ObjectView.sortSink.test.tsx:208— not named by the ruling. This held a pin written for this card asserting$orderbyequals{ name: 'asc' }for adirection: 'desc'fixture — green only because nothing read the key, with a comment saying "Pinned so that whoever DOES fix that card is told this site exists." Fixture-triage case: replaced, not re-spelled, because it pinned the very branch being removed.packages/plugin-view/src/__tests__/ObjectView.test.tsx:490— not named by the ruling; would have become a hard type error.The last two are taken under the bounded in-place exemption (same defect class, mechanical with the correct form pinned by the ruling, no other claim on
packages/plugin-view/**, same gate family), and are named here rather than folded in silently.Reverse verification — direction predicted before running, and one prediction was wrong
Type leg (revert the declaration to
direction): predicted red, observed red — 3 errors, exit 2. The interesting one is inverted, which is what makes the guard non-vacuous:TS2578proves the@ts-expect-erroris genuinely suppressing a real error under the fix.Runtime leg: predicted red, observed green (12/12 passed) — the prediction was wrong, and the reason is sound. The declaration is type-only, so reverting it cannot change runtime; TypeScript is erased. Reported rather than dressed up: enforcement of this fix lives entirely at the type boundary.
To show the runtime assertion is not vacuous, the falsifying mutation is the fixture, not the declaration — swapping its key back to
directionfails with exactly the bug shape:Every mutation was confirmed on disk by grepping the injected and removed text separately (an editor's exit code proves nothing on a zero-hit anchor), and each leg ran under a
trap … EXIT INT TERMrestore. Tree confirmed byte-identical toHEADafterwards. No rebuild was needed for either leg: the suite imports../ObjectViewrelatively, so the subject resolves from source, not through the package'sexports/dist.Verification, run at
604c2036type-check@object-ui/plugin-view(tsc --noEmit && tsc -p tsconfig.test.json)type-check@object-ui/app-shelltype-check@object-ui/console(downstream)vitest run packages/plugin-view/src+ adapter suiteDownstream direction stated: the contract narrowed, so consumers were swept with the prefix form
pnpm --filter '...@object-ui/plugin-view'—app-shell,console,site, and three private examples.app-shellandconsoletypecheck clean;examples/andapps/were swept for the retired spelling and contain no producer of it.Both packages' first typecheck was red with
Cannot find module '@object-ui/…'— the unbuilt-closure signature, not a real failure; green after building each package's dependency closure with the suffix filter form (PKG^...). Recorded because that red reads exactly like a broken import.Lint — narrowed, and the narrowing is measured. The repo-wide
eslint .is CI's run. Locally: eslint's own config linted 4 of 4 changed source files (count read from--format json, none ignored); 0 errors, 88 warnings; and the identical run at the merge-base8d3a5294agives 0 errors, 88 warnings — delta 0/0, so every warning is pre-existing. Untouched files are invariant because no type-aware linting is configured (noproject/projectServiceineslint.config.*), so this diff cannot move another file's verdict.Out of scope, filed separately
#6006 — the published skill
skills/objectui/guides/data-integration.mdteaches aQueryParamsshape the adapters silently drop: the sort key spelleddirection(same defect class as this card, different surface), and every query option written without its$prefix — the class #5458 fixed in repo code, where an unprefixed key "reaches no branch and is dropped". Not fixed here: different surface, andskills/**is a customer-published package with its own expansion-budget discipline that this dispatch carried no budget for.Also left alone deliberately, each a different surface rather than an instance of this defect:
packages/types(ExportDownloadRequest.sort,designer.ts,data-protocol.tsorderBy— and claimed by a concurrent sibling), the export-boundary conversions inObjectGrid.tsx:2089andListView.tsx:2520(both convert fromorder), andObjectView.filterSources.test.tsx:161(table.defaultSort).Changeset:
minorper AGENTS.md §版本号策略 — objectui ships its own breaks asminor, nevermajor(CI-enforced bycheck-changeset-no-major.mjs).Generated by Claude Code