Uh oh!
There was an error while loading. Please reload this page.
refactor(app-shell): bind DeclaredActionsBar's row through usePredicateRecordContext (#4080) - #4268
Merged
Conversation
…teRecordContext (#4080) objectui#4077 fixed the bar's root-only predicate binding with an inline `{ ...row, record: row, data: row }`. objectui#4079 fixed the same fault on the four generic action renderers and, rather than repeating the expression, gave the rule one name — `usePredicateRecordContext`, exported from `@object-ui/react` beside `useCondition`. The bar kept its copy. Two implementations of one binding rule is the shape objectui#3367 / #3842 rule against, and this family has already paid for it once at the `toPredicateInput` level (objectui#3314 — two normalizations drifted and the same `visible:` predicate reached different verdicts depending on which path surfaced the action). Nothing had drifted here; the migration is a substitution, not a fix. The helper's semantics are KEPT, not the inline ones. On no row it binds NOTHING, where the inline copy bound `{ record: {}, data: {} }` — and since `useCondition` merges this context OVER the ambient predicate scope, the inline shape blanks out a `record` the host put in the scope itself. No host mounts this bar without a row, so nothing user-visible changes; the point is that the bar no longer holds a second answer to the question. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 10:06
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#4080
DeclaredActionsBarheld an inline copy of the three-way row binding —{ ...row, record: row, data: row }— added by #4077 to fix its root-only predicate scope. #4079 fixed the same fault on the four generic action renderers and, rather than repeating the expression a fifth time, gave the rule one name:usePredicateRecordContext, exported from@object-ui/reactbesideuseCondition. The bar kept its copy.This is that migration: one import, one substitution, the inline
useMemodeleted. The long rationale in the bar's comment moves to a pointer at the definition, keeping only what is specific to this bar (every declared action onsys_approval_requestgates onrecord.viewer.*, so a root-only bag made the whole server-declared decision set invisible here).The one semantic subtlety — the helper's semantics are kept
The two copies differ in exactly one place, which is why this is a migration rather than a rename. With no row,
usePredicateRecordContextbinds NOTHING; the inline copy bound{ record: {}, data: {} }.useConditionevaluates on the local context merged OVER the ambient predicate scope, so the inline shape blanks out arecorda host had put in the scope itself — a legitimate pattern, and howaction-group-dropdown-visible.test.tsxdrives the group's dropdown leaves.Per the card, the helper's semantics are the ones kept. That corner is unreachable in the bar today (it is always mounted over a record), so no user-visible behaviour changes; the point is that the bar no longer holds a second answer to the question.
Premise check
Verified on the branch tip before implementing, all three still true:
useMemolive atDeclaredActionsBar.tsx:148;usePredicateRecordContextexported atpackages/react/src/hooks/useExpression.ts:142;useConditionfrom@object-ui/react, so no new dependency edge.Tests
Two pins on the bar, deliberately different in kind, plus one at the helper level.
record.status, barestatus,data.status) reach the same verdict, both halves per spelling with the ungated companion asserting the bar itself rendered. Green before and after; it is not a mutation detector for this change and the comment says so.PredicateScopeProviderthat suppliesrecord: the host's row must still reach the predicate. This is the one place the copies part.useExpression.test.ts) — the return shape itself: no row yields an empty bag, andrecord/dataare absent rather than present-and-undefined, so the ambientrecordsurvives the merge. Not a duplicate of existing coverage: the no-row semantics is pinned behaviourally through the renderers inaction-record-predicate-root.test.tsxand incidentally by the dropdown suite, but nothing pinned the contract one level below a rendered verdict, which is what the bar now depends on.Reverse verification
Direction predicted before running, and it matched. The fix was taken out with
git checkout origin/main -- DeclaredActionsBar.tsx(nevergit stash— shared stack), then restored from HEAD.The convergence pin went red, alone. The equivalence pin stayed green in both directions — honestly, that is the expected result and not a weak test: the two copies agree on every row that exists, so nothing about a row-present predicate can distinguish them. The helper-level pins stayed green both ways too; they do not touch the bar.
Changeset
The card said "no changeset".
scripts/check-changeset-presence.mjsarbitrates and disagrees —@object-ui/app-shellis a released package and this is a change under itssrc/— so the gate is followed:@object-ui/app-shell: patch. The empty-frontmatter form was considered and rejected; it asserts "no published behaviour changes", and the no-row binding of an exported component did change, even where no host reaches it.check-changeset-no-majoris green, and noskip-changesetlabel is requested (#3724).Generated by Claude Code