Uh oh!
There was an error while loading. Please reload this page.
fix(console): do not seed a runtime-default defaultValue into the form control - #5885
Merged
Merged
Conversation
`readPrefill` in apps/console/src/components/FormPage.tsx seeded every declared `defaultValue` unconditionally. A `defaultValue` may be a literal, or an instruction the server resolves per insert — a DEFAULT_VALUE_TOKENS token (`NOW()`, `current_user`) or a CEL Expression envelope. Seeding those literally put the text `NOW()` into a datetime input on both console form routes, and the submit sent that string as the value; it is neither absent nor null, so `ObjectQL.applyFieldDefaults` never resolved the declared default. The seed is now guarded by `isRuntimeDefault` from `@object-ui/core` — the same published classifier `@object-ui/plugin-form`'s `schemaDefaults.ts` guards its seeding with, and the one this file already reads once removed via `isServerOwnedValue`. A runtime default leaves the key absent rather than empty, because absent is the case the engine resolves. Skipping is unconditional rather than create-gated: on an edit form a record that names the field already outranks the default, while a record leaving the column unset is exactly where the literal would still reach a control. The later precedence rungs are untouched: a stored value and an explicit `prefill_` param still fill a runtime-default field. Fixes#5727 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 17:42
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#5727
readPrefillinapps/console/src/components/FormPage.tsxseeded every declareddefaultValueunconditionally. AdefaultValuemay be a literal, or an instruction the server resolves per insert — aDEFAULT_VALUE_TOKENStoken (NOW(),current_user) or a CEL Expression envelope. Seeding one of those literally put the textNOW()into a datetime input on both console form routes, and the submit sent that string as the field's value; it is neither absent nor null, soObjectQL.applyFieldDefaultsnever resolved the declared default and the column stored the token text.The seed is now guarded by
isRuntimeDefault, imported from@object-ui/core.Premise re-measured
The card is static evidence taken at
c7cd2b60b. Re-measured on today'sorigin/main(1939c9610) before writing anything — the premise still holds:readPrefillstill seeds unconditionally:if (f.defaultValue !== undefined) out[f.name] = f.defaultValue;isRuntimeDefaultstill returns zero hits inFormPage.tsx(grep -c= 0).#5627 has since landed, and its
serverOwnedValuecarve-out is present — but it governsrequired, not seeding, exactly as the card predicted.The triage question is the PM's call, not this PR's
The card asked whether this hand-rolled prefill should import the classifier or grow a shared helper. The dispatching PM ruled: import
isRuntimeDefaultfrom@object-ui/coredirectly, as@object-ui/plugin-form'sschemaDefaults.tsalready does. Restating the rationale rather than re-deciding it: the classifier is already the single published authority for this distinction, a third helper wrapping it would be a new indirection with one consumer, and the sibling chain's shape is the precedent this repo has been converging on (#4047 / #4068 / #4069 / #4085).Two consequences of that ruling worth recording, because they are not free choices:
isRuntimeDefault, notschemaDefaults.ts'sisSeedableDefaultwrapper. The wrapper additionally rejectsnull, which would quietly change what a declareddefaultValue: nulldoes on this route. The card is about runtime instructions; the null contract stays as it was (!== undefined). Pinned as a control below.NOW()would still reach a control. One unconditional rule means a runtime token never reaches an input on this route.No dependency change was needed:
@object-ui/coreis already declared byapps/console, and this file already importsisServerOwnedValuefrom it.check:phantom-depsverdict below.Expectations, with controls
Every specimen is the spec's own declaration, put through the spec's own three-way classifier (
discriminateDefaultValueShapefrom@objectstack/spec/data) before the behaviour is asserted — a hand-typed'NOW()'would keep passing after the token family moved on.defaultValue'NOW()'(fromDEFAULT_VALUE_TOKENS)token'current_user'(fromDEFAULT_VALUE_TOKENS)token{ dialect: 'cel', source: 'today()' }expression'now()'/' NOW() 'token'Acme'literal'NOW'(no parens)literal{ source: 'today()' }(nodialect)literalnullnull)isSeedableDefaultnote above'NOW()'?prefill_x=, default'NOW()'The key is asserted absent (
not.toHaveProperty), not empty: absent is precisely the caseapplyFieldDefaultsresolves.Verification
All legs below ran on
d62fb621d, which is this PR's head.Reverse verification — discriminating. The guard alone was ablated back to the unconditional seed (the mutation was confirmed on disk by grep counts on both the injected and the deleted text: injected marker 1, removed guard 0 — an editor's exit code proves nothing). No rebuild step is involved or claimed: the test imports the subject as
./FormPage, a relative source specifier, so nothing resolves through a packageexportstodist. The mutation script carried atrap ... EXIT INT TERMrestore.Tests 5 failed | 70 passed (75)— the two token legs, the envelope leg, and both tolerated spellings went red. Predicted direction, observed direction.git diff HEAD --name-onlyempty andsha256sumbyte-identical togit show HEAD:...; re-runTest Files 1 passed (1)/Tests 75 passed (75).prefill_rows pin precedence, and the later rungs overwrite whatever source 1 put there either way. Thenullrow does not discriminate this ablation either; it discriminates a different candidate implementation (isSeedableDefault).Tests and types
pnpm exec vitest run apps/console/src/components/FormPage.test.ts->Test Files 1 passed (1)/Tests 75 passed (75)pnpm exec vitest run apps/console/->Test Files 72 passed (72)/Tests 805 passed (805)pnpm --filter @object-ui/console type-check-> exit 0, script echoed (> @object-ui/console type-check->tsc --noEmit && tsc -b tsconfig.node.json --force)Gates — each quoted from the gate's own verdict line, with exit codes captured before any pipe:
check-changeset-presence.mjs->2 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-control-bytes.mjs->OK (scanned 4891 tracked text file(s); skipped 85 binary)check-phantom-dependencies.mjs->Every in-scope import is declared by the package that publishes it.Re-derived from the paths actually touched rather than trusting the dispatch list — the new
@objectstack/spec/dataimport in the test pulls in more than the four named:check-spec-symbol-derivation.mjs->spec symbol derivation: 1300 files scanned against 4959 spec export namescheck-package-self-import.mjs->No package names itself inside its own src/.Lint, and the narrowing declared. Ran the whole
@object-ui/consoleturbo lint unit (eslint .fromapps/console, the package's own script verbatim) rather than all 40 units: 171 files, 0 errors, 202 warnings — the file count read from--format json, and the population from eslint's own resolution of., not from a hand-picked list. No warning on any line this PR adds; the ones on the touched files are pre-existing (react-refresh/only-export-componentson this file's many exported helpers,no-explicit-anyon existing casts). Soundness of not running the other units:eslint.config.jsextendstseslint.configs.recommended, notrecommendedTypeChecked, and sets noparserOptions.project/projectService; the localeslint-rules/*are per-file AST rules with no filesystem reads. No rule reads across file boundaries, and this diff touches no config file — so it cannot move the verdict of any file it does not contain.One gate deliberately not run, declared:
check:eager-closureneedsapps/console/dist/eager-closure.json, emitted byvite build; without it the script reports a broken gauge, not a verdict. It cannot move here, and that is checkable rather than assumed: this PR adds a named symbol to an import statement that already existed, so the bundled module's package-specifier set is byte-identical toorigin/main's (diffof the sortedfrom '...'sets is empty). CI builds the console and runs it.Out of scope, filed rather than fixed
''— noomitServerResolvedDefaultsequivalent, so the declared runtime default is still suppressed on that path #5883 — the submit half. A runtime-default control the user focuses and clears reaches the payload as'', which is neither absent nor null, so the declaration is still defeated on that path. The sibling chain closes this withomitServerResolvedDefaults;FormPage.tsxcalls no equivalent. A different call site and predicate from this card's fence.current_userseed the wayschemaDefaults.tshas since #5683 — correct, but the operator cannot see the attribution before submitting #5884 — observation: this renderer does not resolve thecurrent_userseed the wayschemaDefaults.tshas since Create-form does not resolve the current_user defaultValue token — spec-correct "default applicant = me" renders as an empty field until save #5683. After this PR the behaviour is correct (server stamps the id), just less informative than the sibling.Neither is touched here. The conditional-rule surfaces (#5627's territory) and the
required-ness half (#4069 / #4085) are likewise untouched.Generated by Claude Code