Repository navigation
fix(react): stop reading the retired navigation.view and substituting it for the navigation-MODE token - #9936
Merged
os-tesla merged 2 commits intoSep 18, 2026
Conversation
…ng it for the navigation-MODE token (objectui#9874) `view.list.navigation.view` was removed in `@objectstack/spec` 17.5.0 under ADR-0049 (enforce-or-remove). `useNavigationOverlay` was its one shipped consumer: it read `const view = navigation?.view` and spent it at two `onNavigate` call sites as `view ?? 'view'`, so an authored name landed in the argument that carries the navigation MODE token and matched no branch in a host reading it against `edit`/`view`. Both sites now pass the literal `'view'`. `NavigationOverlayState` loses its `view` member — it could only ever carry the retired key. Census before removing it: zero readers in this repository. Two pins that asserted the substitution are inverted rather than deleted, both keeping their authored `view` fixture because it is the input the old and new implementations disagree about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
Contributor
|
changeset-claim-re-read
|
os-tesla
added
bug
Claude
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
|
…elease fact, not an installed-pin claim (objectui#9874)
`check-installed-spec-pin-claims` flagged
`gridNavigationMembers-8071.test.tsx:31` for naming `@objectstack/spec` 17.5.0
while the artifact resolves 17.4.0.
The sentence is a "when it changed" claim — 17.5.0 is the release that retired
`view.list.navigation.view` under ADR-0049 — so the gate's other remedy is not
available to it: restamping to the resolved pin would assert that 17.4.0
retired the key, and 17.4.0 is precisely the version where the key still
exists. That is the fresh false premise the gate's own docblock warns
restamping plants.
Recorded as class `historical`, the same class and the same shape as the
existing `offline-nav-performance-spec-parity` entry ("retired that name in
17.0.0-rc.1"). The marker that puts the line in the population is the word
`pinned` in "the row that pinned this", where it means a test row pinning a
behaviour rather than a version pin.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq
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
|
os-tesla
marked this pull request as ready for review
September 18, 2026 19:34
os-tesla
deleted the
claude/issue-9874-navigation-view-retired-consumer
branch
September 18, 2026 19:53
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 free
to 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 #9874
useNavigationOverlayreadnavigation.view— removed in@objectstack/spec17.5.0 under ADR-0049 — and spent it in the argument that carries the navigation MODE token. This stops both.The premise, re-measured at source
Every reading below is mine, taken in a worktree off
origin/mainf5e2fcb4a.origin/maintip02bc27b08f5e2fcb4a— the dispatch's base was stale by the time I fetched:275 const view = navigation?.view;f5e2fcb4a· 18:16:06Z:323/:356onNavigate(recordId as string OR number, view ?? 'view')f5e2fcb4a· 18:16:06ZonNavigate(sites;:314/:337pass the literalnew_windowf5e2fcb4a· 18:16:06Z17.4.0pnpm-lock.yamlcarries exactly one@objectstack/specversion string,17.4.0; control in the same command, sibling package@objectstack/types@17.4.0f5e2fcb4a· 18:17:55Z^17.0.0, some^17.2.0/^17.3.0"^17.1.0(packages/react,packages/plugin-detail) and^17.4.0(packages/types)f5e2fcb4a· 18:17:55ZThe retirement text was read at source in the sibling repo rather than taken from the card's quotation, on
objectstackorigin/main07c6f822e(18:16:17Z),packages/spec/src/ui/view.zod.ts:1814-1822. It matches the card verbatim, and its prescription — the half the card did not quote — is load-bearing for this PR:⇒ the replacement route is page assignment, which lives nowhere near this hook. No fallback belongs here, and the fix is subtractive.
retiredKeyisz.never(...).optional()(objectstack:packages/spec/src/shared/retired-key.ts:106), so on the bump the key typesneverandview ?? 'view'would still compile — silently dead rather than loudly broken. That is the reason to remove the read now rather than let the bump find it.The census the card left open — and what it turned up
Question 1, "does any other surface read
navigation?.view". Corpus:git ls-filesatf5e2fcb4a, 8090 tracked files, working tree clean.Four lines, and only one is a read of the live key: the hook itself. The other three are two prose comments and one
packages/typesparity assertion that belongs to objectui#9667. The same command carried a lit control onnavigation?.mode, which fires across 40-plus lines. So: no other surface reads it.A second probe asked whether anything reads the member off the hook's RESULT — that is the published
NavigationOverlayState.view:Zero lines, against a lit control in the same command on the sibling members of the same object (
isOverlay,handleClick,selectedRecord,width), which fires 20-plus times. I also opened all 12 real call sites of the hook: none destructuresview. So the published member has zero readers in this repository. Hosts outside this repository were NOT measured — that is an unmeasured site, not a negative.⭐ Probe A had a blind spot, and stating it is what found the real work. It matches MEMBER READS, so an object literal authoring the key is invisible to it. A third probe over authoring sites turned up a surface neither the card nor the dispatch names:
packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsxpins the retired substitution as intended behaviour —clickRow({ view: 'summary_view' })assertingonNavigatewas called with('7', 'summary_view'), under a docblock bullet teaching it as the contract.packages/react/.../useNavigationOverlay.modeDefault.test.tsxpins the same thing. Both go red against the fix. ⛔ Neither is skipped, disabled or quarantined: both rows are INVERTED in place, each keeping its authoredviewfixture, because that fixture is the one input the old and new implementations disagree about.Question 2, "what is the correct behaviour once the key is gone". Not a product choice, so 回翻条款 does not fire: the spec names the replacement route and it is a different machinery (page assignment), while this hook's remaining job —
modeandsize— is untouched. Both call sites now pass the literal'view', which is what every config WITHOUT the key already dispatched. The one judgement call is flagged for the seat below.What changed
packages/react/src/hooks/useNavigationOverlay.ts— the read is gone; bothview ?? 'view'sites pass'view';NavigationOverlayStateloses itsviewmember; the module docblock gains the retirement, the prescription and the census; one authoring example that spelled the retired key is rewritten.packages/react/.../useNavigationOverlay.modeDefault.test.tsx— one row inverted, one row added (the state shape, asserted on the KEY rather than on the value, becausetoBeUndefined()passes just as happily against the member this PR removed).packages/plugin-grid/src/__tests__/gridNavigationMembers-8071.test.tsx— one row inverted, three comment blocks corrected.packages/app-shell/src/views/ObjectView.tsx— a comment my change falsifies. It claimed a custom name arrived here "forwarded fromnavigation.view" and was "resolved by RecordDetailView from its own config". No layer ever resolved a view by that name. ⭐ This branch already treated the name and the mode token identically, which is exactly what made the substitution inaudible on the Console: behaviour there is unchanged, and that is the finding rather than a caveat..changeset/9874-navigation-view-retired-consumer.md—minor, per this repo's policy that its own breaking changes ship asminorwith the semantics in the body.Ablation — two legs, both restored and proven
Direction predicted before running: turn red, exactly two rows. Both legs mutate the real source, prove the mutation by blob hash, run, restore, and prove the restore by hash equality AND an empty
git diff HEAD, withtrap ... EXIT INT TERMon absolute paths.⭐ The ablation mutates
packages/react/src, and the plugin-grid leg imports@object-ui/reactby package specifier — the rootvitest.config.mtsaliases that specifier to./packages/react/src, so nodistrebuild is interposed. The plugin-grid row going red is itself the proof that the alias is live.Leg 1 — the substitution. Value chosen because the two implementations DISAGREE about it: an authored
view: 'summary_view'with the mode defaulting.Leg 2 — the published state shape. Re-adding the
viewmember to the returned object:grep -c "view ?? 'view'"printed 3 where its own label said "expect 2". The third hit is prose in the docblock I added, which quotes the old spelling. The authoritative instrument is the anchored replacement inside the mutation script, which asserts exactly 2 code sites and aborts otherwise; the grep line was a loose observation and its label was wrong, not the mutation.Verification
All test and build runs serialized behind the seat lock (
OS_VERIFY_LOCK_SLOT=3); each reports its ownVERDICT command-exit. Read at final HEAD3f2ed65c5.Test Files 2 passed,Tests 27 passed— the two files were green, so their red is caused by the fixvitest run packages/react/plus the plugin-grid pin, the plugin-list navigation-forward test and thepackages/typesparity testTest Files 91 passed (91),Tests 1114 passed (1114)type-checkfor@object-ui/react,@object-ui/app-shell,@object-ui/plugin-griderror TS— both legs ran (tsc --noEmit && tsc -p tsconfig.test.json), so the test sources are inside the judged set rather than excludedcheck:control-bytes·check:changeset-claims·check:pending-changeset-literals·check:new-line-citations·check:spec-symbols·check:comment-mask-corpus·check:handler-key-readscheck:changeset-presence·check:changeset-no-majorcheck:governed-queue-guard --teston the five changed pathsNOT GOVERNED — 5 path(s) checked against 5 governed surface(s); none matched⭐ The
packages/typesparity test is objectui#9667's territory and was run deliberately: it stays green, so the two halves are not entangled and this PR does not pre-empt that one.check:readme-exports— NOT MEASURED, prerequisite not met. It needs every package'sdiston disk and I built only the dependency closure of the three packages I touched. All 87 of its complaint lines read "is not on disk -- runpnpm buildfirst", and 0 of them name any file in this diff. A whole-repo build is CI's run, not a local one.Lint — a narrowing, measured rather than asserted.
eslint --no-inline-configover the four changed source files: exit 0, 0 errors; the 174 warnings are pre-existingno-explicit-any/react-refreshnoise inObjectView.tsxand the grid test, none on a line this diff wrote. The three readings that make the narrowing a measurement:isPathIgnored, not guessed: 5129 lintable tracked files.--format json: 4.eslint.config.jshas zero hits forprojectService,project:,parserOptionsortsconfigRootDir, against a lit control in the same file onrules/languageOptions/ignores. Type-aware linting is off, so this diff cannot move the verdict on any untouched file.Acceptance notes
⭐ One judgement worth a maintainer's eye, called out rather than buried. Removing
viewfromNavigationOverlayStateis a breaking change to a published type. I read it as the mechanical consequence of the retirement rather than a product choice — the member could only ever carry the retired key, so keeping it would publish a field that is permanentlyundefined, which is the "declared, consumed, and wrong" state the retirement exists to end — and the in-repo census for it is a measured zero. But the alternative (keep it as a deprecated tombstone that is alwaysundefined) is a real option, and it is the seat's to take. Reverting just that half is a two-line change and does not disturb anything else here.Out of scope, noted, not filed:
objectstack:packages/spec/src/ui/react-blocks.ts:321declares the navigation-mode union as view-or-edit while this hook passesnew_windowat two sites. Untouched here and genuinely independent: after this PR the union still lacksnew_window. Handed on rather than filed — the triage seat already recorded it against this same file. Successor: that card.Worth its own card, handed to the seat with dedupe words rather than filed from here:
packages/types/src/objectql.ts:2546tells authors thatnavigation: { view: 'summary_view' }is legal authored metadata. On the bump this steers an author — very often an AI — to write a key the runtime rejects with the tombstone. Dedupe words:objectql.ts navigation.view doc comment·retired key authored example·spec 17.5.0 tombstone types·ViewNavigationConfig legal metadata.packages/typesparity test. My authoring-site probe finds the retired key ALSO authored atpackages/types/src/__tests__/gantt-declared-keys.test.ts:117and:191,packages/types/src/__tests__/object-view-unmirrored-keys-7779.test.ts:386, andpackages/plugin-list/src/__tests__/ListView.ganttNavigationForward-7334.test.tsx:183and:185— five more, in three files that card does not name, each typingneveron the bump. ⛔ Deliberately not touched: that repo area is the sibling half's, and widening into it is what this dispatch told me to stop and report instead of doing. Dedupe words:navigation.view authoring sites census·retired key never type bump·gantt-declared-keys navigation view·9667 scope narrower than census.Two fixtures in THIS diff also author the retired key and are kept on purpose — they are the disagreement inputs the inverted pins rest on, and they will need the same treatment as the five above when the bump lands.
What in the dispatch was wrong
02bc27b08was stale;origin/mainwasf5e2fcb4awhen I fetched.^17.1.0and^17.4.0).Clause-②:line from the claim comment into this body. The claim comment on objectui#9874 carries no such line, and no gate in this repository reads one from a PR body —check-changeset-presence.mjsreads no PR body at all, and the marker the changeset workflow hunts for is the first line of a COMMENT. Nothing was copied because there was nothing to copy.Everything else in the dispatch re-measured as stated — including the 4-call-site control, which is a real lit control and did its job.
🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code