Repository navigation
fix(console): honour section-level and object-level conditional rules in FormPage (#5627) - #5728
Merged
os-zhuang merged 1 commit intoAug 23, 2026
Conversation
…mPage (#5627) `FormPage` is the second form renderer in this repo and honoured exactly one of the four conditional-rule surfaces the sibling chain does — the view-level field predicate #5594 wired. The other three were dropped in the same place: keys the payload types did not admit, so nothing downstream could read them. Section-level `visibleWhen`/`visibleOn` (ADR-0089) is now evaluated: the render mapped every section unconditionally, so a section an author conditioned away drew in full — heading and every control — on both routes, including the anonymous `/f/:slug` one. The section type itself needed no widening: #5596 replaced this file's hand copy with the shared `FormSectionSpec`, which has declared both keys all along without anything evaluating them. Object-level `visibleWhen`/`readonlyWhen`/`requiredWhen` (ADR-0036) now reach the row: `ObjectFieldDef` admits them and `resolveRowState` resolves them through `@object-ui/core`'s shared `resolveFieldRuleState` — not a fourth consumer-side copy of the rule semantics. That reuse carries the settled rulings in unchanged, including the `serverOwnedValue` carve-out (#4069/#4085). Visibility stays a RENDERING rule at both granularities (triage ruling, 2026-08-22): a hidden section's fields still submit their values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
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-zhuang
marked this pull request as ready for review
August 23, 2026 01:26
os-zhuang
deleted the
claude/issue-5627-formpage-conditional-rule-surfaces
branch
August 23, 2026 01:26
This was referenced Aug 23, 2026
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 #5627
FormPageis the console's own form renderer — its ownbuildSections, its own JSX,serving the anonymous
/f/:slugroute and the internal/forms/:nameone. After #5594 ithonoured one of the four conditional-rule surfaces the sibling chain honours. This
wires the other three, through the shared machinery rather than a fourth copy of it.
What was measured before writing anything
The card's behavioural claim held; one of its two type citations did not.
c7cd2b60binterface ObjectFieldDefadmits none of the ADR-0036 rulesFormPage.tsx:95:1398):1403interface FormSectionSpec { ... }lives in this fileThe section type is no longer this file's to widen: #5596 already landed, replacing the
hand copy with the shared
FormSectionSpec(packages/app-shell/src/views/metadata-admin/form-spec.ts),imported here through
FormViewSpec. That type has declaredvisibleWhen/visibleOnall along — its own docstring says so while naming this card: "Declaring a key is not
honouring it: objectui#5627 tracks the console renderer still rendering every section
unconditionally, which this type makes declarable but does not evaluate." So the
section half of this fix is evaluation only; the local type work was the object-field
half alone.
The other measurement worth recording: the two
sections.mapsites are the builder(
buildSections,:393) and the render (:1403). Only the second one needed a guard.The fix
Sections.
buildSectionscarriessec.visibleWhen ?? sec.visibleOnonto the row(canonical-first, the precedence every sibling reader spells), and the render calls
isSectionVisible, which routes the predicate throughevalFieldPredicate— the sameengine, scope (
record.*live,previous.*stored) and fail-open-but-loud behaviour thefield predicate has used here since #5594.
The ruling asked for
readVisibilityfor sections. That helper is module-private toapp-shell's
SchemaForm.tsxand is not part of@object-ui/app-shell's published surface,so it cannot be imported; what it does is a two-key
??, which is done once at build timehere exactly as this file already does it for fields. The half that decides anything — the
engine — is the shared one.
Object-level field rules.
ObjectFieldDefadmitsvisibleWhen/readonlyWhen/requiredWhen,buildSectionscarries them onto the row as a group (RenderableField.rules), andresolveRowStatehands that group whole to@object-ui/core'sresolveFieldRuleState.No rule semantics are re-derived here. Three settled rulings come along with the resolver
rather than being restated:
readonly: trueis never weakened by a predicate resolving false;required: true;serverOwnedValueoutranks both spellings of required (A required field whosedefaultValueis a runtime token (NOW()/current_user) cannot be submitted from a create form #4069 /requiredWhen+ a runtimedefaultValuestill deadlocks a create form (the conditional half of #4069) #4085), so a createform does not require a producer-owned control.
isCreateFormis passed as a requiredparameter, read off the URL switch
readFormRecordTargetalready owns — the resolver maynot infer it, and a new call site must not get edit semantics by omission.
The two visibility layers are ANDed, never collapsed. A view-level predicate (#5594's
slot) and an object-level
visibleWhenare different authoring slots with different owners;sectionFields.tsroutes the view predicate intovisibleOnon the sibling chain preciselyso the object rule is never clobbered. Either layer resolving false hides the row, neither
can re-show what the other hid, and both are evaluated even when the first has already
decided — the one-time warning is the author's only diagnostic, and short-circuiting would
make a typo in one layer invisible whenever the other happened to hide the row.
One verdict per row.
FieldInputnow takes the resolved{ readonly, required }as aprop instead of reading the static flags, so the control and the asterisk beside it cannot
disagree — the two-layer split objectui#4201 removed on the sibling chain.
Hidden sections still submit. Triage's ruling, quoted: "Section-level visibility is a
rendering rule — a hidden section's fields still submit." Pinned as a positive assertion on
the POST body. The escape hatch in that clause was checked and did not fire: the only
client-side enforcement in this renderer is the DOM
requiredattribute, which travels withthe rendered control, so nothing can fire a validation on a value the user could never see.
Reverse verification — measured
FormPage.tsxreverted toc7cd2b60bwith the new pin file left in place. The mutation wasconfirmed on disk before the run (
resolveRowState0 hits,isSectionVisible0 hits, thepre-#5627 render line back at 1 hit) and restored by an
EXIT/INT/TERMtrap afterwards:Every red one fails by naming the defect — the conditioned-away section is on screen, the
locked control is editable, the required marker is missing, the predicate never reached the
row. The 4 green are the controls that have to be green on both sides, and they are the
whole set:
CONTROL: a section with no predicate at all still renders— without it, everynot.toBeInTheDocumentabove is equally satisfied by a renderer that draws nothing;CONTROL: a field with no object rules at all renders editable and optional;CONTROL: a static readonly is not weakened by a readonlyWhen that resolves false;does NOT require a server-owned field on a CREATE form— green pre-fix for the wrongreason (nothing read
requiredWhenat all), which is exactly why its twin,DOES require the same field on an EDIT form, is in the file and is red pre-fix.Tests — all on rendered output
Every failure mode in this card is invisible to
tscandeslint, so the 24 new casesassert the DOM: element presence for the two visibility surfaces,
toBeDisabled()forreadonlyWhen,toBeRequired()plus the marker read out of thelabelelement forrequiredWhen, and the POST body for the submit clause. Both routes are covered — thepublic
/f/:slugone included, since that is the user-reachable half.Gates run on
2eed0fe2a(the final commit)pnpm --filter @object-ui/console type-checkTYPECHECK_EXIT=0(after building the dependency closure)pnpm --filter @object-ui/console lintLINT_EXIT=0—202 problems (0 errors, 202 warnings)pnpm exec vitest run apps/consoleTest Files 72 passed (72)/Tests 782 passed (782)check:control-bytesOK (scanned 4776 tracked text file(s))check-changeset-presence/check-changeset-no-major/check-changeset-fixedcheck:phantom-deps,check:self-import,check:spec-symbols,check:esm-specifierscheck:i18n-keys,check:i18n-driftNo en value changed in this range)check:published-dist,check:eager-closure,check:action-forward-paritytype-check:coverage,lint:coverage46/46 packages linted,45/46 via type-checkNarrowing declared:
type-check/lint/testwere run for the only package this difftouches, whole (
eslint .overapps/consoleis that package's own gate command, not afile subset). Repo-wide
pnpm lint/pnpm type-check/pnpm testare CI's run.Out of scope, filed instead
#5727 —
readPrefillin this same file seeds a runtime-defaultdefaultValue(NOW(),current_user, a CEL envelope) literally into the control, suppressing the server-sideresolution the declaration asks for. Different defect class (seeding, not conditional-rule
evaluation); filed unassigned rather than folded in.
#5596 is adjacent and was not folded in — it is already landed for these two container
types, and this PR consumes its result rather than extending it.
Generated by Claude Code