Repository navigation
fix(admin): start new entries with each field's default value - #3758
Conversation
A field's defaultValue was stored on its field row but never reached the editor: the admin manifest's field descriptor left it out, and the new-entry form started from empty data. A boolean field with defaultValue: true rendered switched off, and if the field was optional, saving the entry without touching it left its column NULL. The manifest now carries each field's default, except on a reference field bound to a relation, whose value create refuses in data. The new-entry form starts from those defaults and sends them with the first save. Existing entries still load their stored values.
🦋 Changeset detectedLatest commit: d17a277 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR takes the right approach for the stated scope: it forwards the registry’s stored defaultValue through the admin manifest, seeds new-entry formData from those defaults, and correctly omits defaults for relation-bound (storageless) reference fields. The added tests cover new entries, existing entries, an unloaded edit form, and core manifest forwarding including falsy defaults like false and 0.
I traced the admin route props and the ContentEditor state/dirty logic and the change behaves correctly for registry/manual collections. The one gap I found is that config-defined collections using Zod schemas (e.g. z.boolean().default(true)) are not covered: extractFieldType unwraps ZodDefault and discards its default value, so those collections’ admin new-entry forms will still ignore defaults. Since the PR title/description emphasizes stored defaults, this is likely out of the immediate scope, but it leaves the fix inconsistent across collection definition paths. I’d suggest addressing it (or documenting it) before merging if config schemas are a supported way to declare defaults.
Findings
-
[suggestion]
packages/core/src/api/handlers/manifest.ts:274The
ZodDefaultbranch recurses intodef.innerTypebut never reads the default value. That means config-defined collections whose schemas usez.boolean().default(true)(or similar) will not carrydefaultValuein the generated manifest, so the admin new-entry form will still start those fields empty. The DB-field path added below covers registry/manual defaults; this path leaves config-defined collections uncovered.Consider extending
extractFieldTypeto capture the value from the Zod default descriptor and set it on the returned field descriptor:case "ZodDefault": case "ZodOptional": { const inner = def?.innerType ? extractFieldType(name, def.innerType) : { kind: "string", label: formatLabel(name) }; if (typeName === "ZodDefault") { // Zod stores the default as a function in `_def.defaultValue`. // The result should already be JSON-serializable for valid schemas. const defaultValue = typeof def?.defaultValue === "function" ? def.defaultValue() : def?.defaultValue; if (defaultValue !== undefined) { (inner as ManifestFieldDescriptor).defaultValue = defaultValue; } } return inner; }
If config-defined defaults are intentionally out of scope, a short note in the changeset would help set expectations.
emdashbot
Bot
added
review/awaiting-author
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
danielmlr
added
bot:review
github-actions
Bot
removed
the
ci:run
There was a problem hiding this comment.
This re-review confirms the PR correctly implements its stated scope: stored field defaultValues now flow from the registry through the admin manifest into the new-entry ContentEditor form, while existing entries and unloaded edit forms continue to ignore them.
What I checked:
packages/core/src/api/handlers/manifest.tsforwardsfield.defaultValueto the manifest descriptor, guarded by!isStoragelessField(field)so relation-bound references stay default-free.ContentEditorseedsformDatafromdefaultFieldValues(fields)only for new entries (isNewwith noitem.data), using??so existing/hydrated entries never get overridden.lastSavedDatainitializes fromitem?.data || {}, so a new entry with defaults starts dirty (Save enabled) as intended.- Tests cover new-entry defaults, existing-entry preservation of falsy stored values (
false,0,""), unloaded edit forms, core manifest forwarding including falsy defaults, and bound-reference omission on both SQLite and Postgres. - Changeset is clear, user-focused, and names the affected packages.
The only item from my prior review remains open: config-defined collections whose schemas use ZodDefault (e.g. z.boolean().default(true)) still do not carry defaultValue in the generated manifest because extractFieldType unwraps ZodDefault without reading its value. The PR description explicitly scopes this out — “collections passed to generateManifest as Zod schemas are unchanged” — which is a reasonable boundary for this fix, though it does leave default handling inconsistent across collection-definition paths.
No new blocking issues; the implementation is clean for the chosen scope.
emdashbot
Bot
added
review/approved
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
swissky
left a comment
swissky
left a comment
There was a problem hiding this comment.
Thanks, Daniel, nice fix. Forwarding defaultValue through the manifest and seeding only new entries is the right layer, and the tests cover the edit and failed-load cases too. Approving.
Two small, non-blocking suggestions:
defaultFieldValuesputs default values into form state by reference, and those objects live in the react-query cache. For array/object defaults (repeater, multiSelect, json, blocks), an in-place change would leak into the next new entry.structuredClone(field.defaultValue)would match whatlib/block-field-state.tsalready does.- The changeset only describes the admin form, but the manifest field descriptors now also include
defaultValue. One sentence about that would help anyone reading the API.
Heads-up: #3791 adds a property at the same spot in three files, so whichever lands second needs a quick rebase (keep both lines).
Clone each default when creating form state so nested entry data cannot mutate the manifest defaults used by later entries. Cover the save callback boundary with a nested object and array regression, and document the manifest defaultValue field in the changeset.
github-actions
Bot
added
size/L
review/needs-rereview
github-actions
Bot
removed
the
ci:run
There was a problem hiding this comment.
This re-review confirms the implementation matches its stated bug-fix scope and the prior approved state.
Approach and scope — The PR is the right fix for #2230: new admin entries now start from stored field defaultValues, while existing entries and unloaded edit forms keep their current behavior. Seeding defaults only when isNew && !item?.data, and cloning them with structuredClone, prevents shared-mutation bugs across new-entry sessions. Excluding relation-bound references from the manifest is also correct, since API create rejects data values for those fields.
What I checked:
packages/admin/src/components/ContentEditor.tsxseedsformDatafromdefaultFieldValues(fields)only for new entries, uses??so existing/hydrated data wins, and clones defaults before storing in React state.lastSavedDatastill initializes fromitem?.data || {}, so new entries with defaults are dirty and the Save button is enabled as intended.packages/core/src/api/handlers/manifest.tsforwardsfield.defaultValueonly when it is notundefinedand the field is not storage-less (i.e., not a relation-bound reference).- The type definitions in
packages/core/src/api/types.ts,packages/core/src/astro/types.ts, andpackages/admin/src/lib/api/client.tswere all updated consistently. - New tests in
ContentEditor.test.tsxcover new-entry defaults, falsy stored-value preservation, unloaded edit forms, and cross-entry default isolation. Core tests cover manifest forwarding and bound-reference omission. - The changeset names both affected packages and describes the user-facing behavior.
- No new user-facing strings, RTL/Tailwind changes, SQL changes, authorization changes, or logged-out-route query changes.
Known scope boundary (unchanged): The PR deliberately does not propagate Zod schema ZodDefault values into the generated manifest for config-defined collections. That inconsistency remains, but it is explicitly scoped out in the PR description and was the only open item from the previous review.
No new blocking issues. The implementation is clean for the chosen scope.
emdashbot
Bot
added
review/approved
What does this PR do?
Starts the admin's new-entry form with each field's
defaultValue. Onmain, a boolean field withdefaultValue: truerenders switched off, and saving the entry untouched leaves an optional field's columnNULL.The default is stored on the field, but the manifest's field descriptor leaves it out and the form starts from empty data. The manifest now carries each field's default, except on a relation-bound reference field, whose value create refuses in
data. The form seeds a new entry from a deep copy of those defaults, so the first save sends them and mutations to entry data cannot change defaults shared with later new entries. Existing entries still load their stored values, and API creates are unchanged. The admin's manifest takes its collections from the database alone, so collections passed togenerateManifestas Zod schemas are unchanged.The bot's candidate, 0bef4c6, seeded the form but left the manifest alone; with only that half applied, the switch still starts off.
Closes #2230
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: no new strings)AI-generated code disclosure
Screenshots / test output
The New Post form in
demos/simple, with an optionalfeaturedboolean field (defaultValue: true) added to Posts. Before the change, the switch starts off.After the change, it starts on, and saving the untouched entry stores
1wheremainstoredNULL.ContentEditor.test.tsx, "field defaults": a new entry shows and sends its boolean, select, number and string defaults (fails onmain, the switch stays unchecked); an existing entry that storesfalse,0, an empty string or nothing keeps those values and stays saved; an edit form whose entry failed to load gets no defaults.ContentEditor.test.tsx, "keeps nested defaults unchanged across new entries": changes to the first entry's saved nested object and array leave the manifest defaults and the next entry unchanged (fails without cloning).manifest-build.test.ts: the manifest carriestrue,false,0and"blue"defaults (fails onmain:expected undefined to be true).manifest-reference.test.ts: no default on a bound reference field, on SQLite and Postgres.Initial validation:
pnpm format,pnpm build,pnpm typecheck,pnpm lint,pnpm lint:jsonwith 0 diagnostics, the admin suite (2984 tests) and corevitest --changed(1287 tests).Follow-up validation:
pnpm format, rootpnpm build,pnpm typecheck,pnpm lint:quick,pnpm lint,pnpm lint:jsonwith 0 diagnostics, all 142 ContentEditor tests, 25 manifest tests, and an Arabic editor check with the translated catalog and RTL layout.Overlaps with #3791 in the shared field descriptors; both
defaultValueandintegermust be preserved when integrating the second PR.