feat: reference field type (storage-less edges + admin UI)#1928
feat: reference field type (storage-less edges + admin UI)#1928MA2153 wants to merge 28 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`handleContentCreate`/`handleContentUpdate` already accept a `references` body key, but the zod schemas didn't list it, so `parseBody` silently stripped it before it reached the handler.
Creating a reference field now creates its backing relation def transactionally, updating the field's label PATCHes the relation's childLabel, and deleting the field deletes the relation and its edges. The admin no longer has to orchestrate these multi-step writes itself. Also fixes withTransaction to short-circuit when already inside a transaction (db.isTransaction), rather than attempting an illegal nested .transaction() call — required for the field-create/update/ delete handlers to nest their relation writes with SchemaRegistry's own internally-transacted field writes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Type STORAGELESS_FIELD_TYPES as ReadonlySet<string> so membership checks against DB-sourced field types need no cast, resolving the lint diagnostic introduced with stripStoragelessDataKeys. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reference fields are storage-less, so a seed defining one now creates the backing relation (like the schema handler) and writes a $ref value in the field's data as a content-reference edge instead of a table column. Previously applySeed threw "no such column" for any seed using a reference field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- resolveEntries now attaches a display `title` (entry `title`, then `name`, else null) to every EntryRef, so picked entries and backlinks show a readable label instead of a slug; hydrated through content GET and the admin editor/backlinks sidebar. - Wire the relation and reference-edge API routes into injectCoreRoutes; they existed but were never registered, so /_emdash/api/relations 404'd and the "Referenced by" panel silently hid itself. - Preserve `targetCollection` and `multiple` on reference field validation so the create handler no longer rejects the field for a missing target. - Backlinks sidebar resolves relations by translation_group, not name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # packages/core/src/api/handlers/content.ts
🦋 Changeset detectedLatest commit: bd97d0a The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
Scope checkThis PR changes 3,014 lines across 34 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@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-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@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: |
# Conflicts: # packages/admin/src/components/ContentEditor.tsx # packages/admin/src/router.tsx
There was a problem hiding this comment.
This PR implements the end-to-end reference field feature in a way that fits EmDash’s architecture: storage-less via _emdash_content_references, atomic writes inside the content create/update transaction, opt-in hydration on read, and admin picker/backlink UI. The approach is sound and matches the approved Discussion. I reviewed the diff, the changed handler/repository/component files, and the new route wiring without running the test suite or tooling.
There are a few real issues to fix before merge:
- The
ReferenceEntryRefZod response schema does not include the newtitlefield thatresolveEntriesreturns, so OpenAPI/generated types drift from the runtime response. - The reference-field auto-loader in
ContentEditorwill retry forever whenfetchReferenceChildrenfails because the error path clearsloadingbut leavesnextCursorset. createFieldRelationvalidates the parent collection exists but never checks the target/child collection, allowing invalid reference fields via seeds or direct API calls.- The content picker does not filter by or preserve the active locale, which causes duplicate translation rows in multi-locale collections and means newly picked references lose their locale context.
Tests look thorough for the lifecycle and write paths, and the changeset wording is appropriate.
Findings
-
[needs fixing]
packages/core/src/api/schemas/relations.ts:89-98EntryRef/ReferenceEntryRefnow carries atitlefield populated byresolveEntries, and the admin client type has it, but the Zod response schema was not updated. This causes the OpenAPI contract and any generated types to disagree with the runtime response.// The actual locale of the resolved variant. When no variant matches the // requesting entry's locale, the ref falls back to another locale's row; // this field makes that substitution explicit instead of silently // presenting a wrong-locale entry under the requested context. locale: z.string().nullable(), title: z.string().nullable(), sortOrder: z.number().int().optional(), -
[needs fixing]
packages/admin/src/components/ContentEditor.tsx:1684-1688ReferenceFieldRendererauto-loads remaining pages whenevernextCursor && !loading. WhenhandleLoadMoreReferencesfails, it resetsloadingtofalse(line ~576) but leavesnextCursorintact, so this effect fires again immediately and can loop indefinitely on a failing API.Fix the error path so a failed load does not auto-retry. A minimal safe change is to track an error state (
loadError) and include!loadErrorin this condition, then render a manual retry affordance when the load fails. Simply clearingnextCursorwould stop the loop but would also unlock editing on a partial list and risk truncation on save. -
[needs fixing]
packages/core/src/api/handlers/schema.ts:47-55createFieldRelationchecks that the parent collection exists, but it never validatestargetCollection. A reference field can therefore be created pointing to a non-existent collection (via seed or direct API), leaving a relation row and an admin picker that loads a missing collection.The standalone
handleRelationCreatealready validates both collections;createFieldRelationshould do the same.const parent = await registry.getCollection(collectionSlug); if (!parent) { throw new SchemaError(`Collection "${collectionSlug}" not found`, "COLLECTION_NOT_FOUND"); } const child = await registry.getCollection(targetCollection); if (!child) { throw new SchemaError( `Target collection "${targetCollection}" not found`, "COLLECTION_NOT_FOUND", ); } -
[suggestion]
packages/admin/src/components/ContentPickerModal.tsx:32-39PickedContentEntrydoes not carry the chosen entry’slocale, so freshly picked references lose locale context: the editor can’t show the cross-locale badge and the link falls back to the active locale instead of the actual entry locale. Also, the content-list query below does not pass a locale filter, which makes multi-locale collections show duplicate translation rows. Consider threading the editor locale intofetchContentListand addinglocale?: stringhere so the reference row can preserve the picked variant.
|
Item 4 (duplicate loses reference edges) confirmed and fixed. Added |
There was a problem hiding this comment.
This re-review confirms the reference field implementation remains the right approach for EmDash: storage-less edges, atomic transaction writes, opt-in hydration, and admin picker/backlink UI. It matches the approved Discussion and the broader architecture.
The only prior-review concern that is clearly resolved is the auto-paging retry loop in ContentEditor.tsx: the error state now halts fetchReferenceChildren retries when a page fails, so a broken API no longer spins.
Three issues from the previous review are still unaddressed in this push and need attention before merge:
- The
entryRefSchemainpackages/core/src/api/schemas/relations.tsdoes not include thetitlefield thatresolveEntriesreturns at runtime. This creates an OpenAPI/generated-type contract drift from the actual response. createFieldRelationinpackages/core/src/api/handlers/schema.tsstill validates only the parent collection and not the target/child collection, leaving a defense-in-depth gap.PickedContentEntryinpackages/admin/src/components/ContentPickerModal.tsxstill doesn't carry locale, andfetchContentListisn't scoped to one, so multi-locale collections show duplicate translation rows and newly picked references lose locale context.
Tests and changesets look appropriate. The first two are blocking enough to hold a clean merge; the third is a UX/localization suggestion that aligns with EmDash's per-locale content model.
Findings
-
[needs fixing]
packages/core/src/api/schemas/relations.ts:89-93The
entryRefSchemaobject still omits thetitlefield thatresolveEntriespopulates from the referenced entry'stitle/namedata. TheEntryRefTS type inapi/handlers/relations.ts, the admin client type, and theContentItem.referencesshape all include it, but this Zod response schema does not. That drifts the OpenAPI contract and any generated clients from the runtime response, and a parse/serialize pass through the schema would strip the display title.Add
titleaftercollectionto match the runtimeEntryReforder:id: z.string(), slug: z.string().nullable(), collection: z.string(), title: z.string().nullable(), // The actual locale of the resolved variant. When no variant matches the // requesting entry's locale, the ref falls back to another locale's row; // this field makes that substitution explicit instead of silently // presenting a wrong-locale entry under the requested context. locale: z.string().nullable(), sortOrder: z.number().int().optional(), -
[needs fixing]
packages/core/src/api/handlers/schema.ts:48-50The
createFieldRelationhelper still validates thatcollectionSlugexists but never checkstargetCollection. Because this helper is also called from seeds and direct registry code paths, a missing or invalid target collection leaves a relation row pointing at a non-existent child collection. The standalonehandleRelationCreatealready validates both collections;createFieldRelationshould too.const parent = await registry.getCollection(collectionSlug); if (!parent) { throw new SchemaError(`Collection "${collectionSlug}" not found`, "COLLECTION_NOT_FOUND"); } if (!(await registry.getCollection(targetCollection))) { throw new SchemaError( `Target collection "${targetCollection}" not found`, "COLLECTION_NOT_FOUND", ); } const baseName = `${collectionSlug}_${fieldSlug}`.slice(0, 63); -
[suggestion]
packages/admin/src/components/ContentPickerModal.tsx:32-33PickedContentEntrystill doesn't carry the selected entry'slocale, and thefetchContentListcall at line 116 isn't scoped to one either. In multi-locale collections this means duplicate translation rows appear in the picker, and a freshly picked reference loses its locale context (soContentEditorcan't show the cross-locale badge or link to the actual picked variant until hydration refreshes it).Consider:
- Adding
locale?: stringtoPickedContentEntryandlocale?: stringtoContentPickerModalPropsso callers can pass the editor's active locale. - Passing
locale: item.localewhen building picked objects (around lines 136 and 147). - Threading that locale into
fetchContentListso the list is scoped to the locale the editor is working in.
- Adding
- Add `title` to entryRefSchema so the response contract matches the runtime EntryRef (no more OpenAPI/generated-type drift). - Validate the target collection in createFieldRelation, rejecting a reference field whose target does not exist (rolls back in-transaction, leaving no orphan field or relation row). Adds a lifecycle test. - Remove the misleading cross-locale badge from the reference list and backlinks sidebar: edges are keyed by translation_group, so labelling a resolved variant's locale implies a per-locale binding that doesn't exist. Titles already resolve at the current entry's locale via pickVariant. Drops the now-dead entryLocale plumbing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed in 850285f.
|
There was a problem hiding this comment.
The reference field implementation continues to be the right approach for EmDash: storage-less edges, atomic transaction writes, opt-in hydration, and admin picker/backlink UI. It fits the approved Discussion and the broader per-locale content model.
Of the three issues raised in the previous review, two are clearly resolved in this push:
entryRefSchemanow includestitleandlocale, matching the runtimeEntryRefshape.createFieldRelationnow validates both the parent and target collections.
One previously-flagged issue is still unaddressed: ContentPickerModal does not carry or pass locale information, so multi-locale collections show duplicate translation rows and newly picked references lose their locale context.
This re-review also found two new contract/convention gaps:
handleContentGethydratesitem.references, butcontentItemSchemadoes not declare it, so OpenAPI / generated clients drift from the runtime response.referencesis now a runtime-hydrated top-levelContentItemproperty but is not inRESERVED_FIELD_SLUGS, so a user-defined field with that slug collides with the hydrated object.
Authorization on the new reference edge routes is correct (own + content:edit_own on writes, content:read on reads), the relation lifecycle mirrors the existing taxonomy translation pattern, seed $ref: values are routed to edges, and changeset/test coverage looks appropriate. None of the findings are security or data-loss blockers, so this is a comment review.
Findings
-
[needs fixing]
packages/core/src/api/schemas/content.ts:157-179handleContentGetsetsitem.referenceswhenreferenceOptionsis passed (seepackages/core/src/api/handlers/content.ts), and the runtimeContentItemtype already includes it. The publiccontentItemSchemadoes not, so the OpenAPI/generated client contract drifts from the actual single-item response — a pass through this schema would strip reference hydration.Import
entryRefSchemafrom./relations.jsand addreferencesafterseo:import { bylineSummarySchema, bylineCreditSchema, contentBylineInputSchema } from "./bylines.js"; import { cursorPaginationQuery, httpUrl, localeCode } from "./common.js"; import { entryRefSchema } from "./relations.js";locale: z.string().nullable(), translationGroup: z.string().nullable(), seo: contentSeoSchema.optional(), references: z.record( z.string(), z.object({ children: z.array(entryRefSchema), nextCursor: z.string().optional(), }), ).optional(), }).meta({ id: "ContentItem" }); -
[needs fixing]
packages/admin/src/components/ContentPickerModal.tsx:32This is the same locale concern from the previous review and is still unaddressed.
PickedContentEntryhas nolocale,ContentPickerModalPropshas no locale prop, andfetchContentListis called without one. On a multi-locale install the target collection's content list returns every translation, so the picker shows duplicate translation rows; when an entry is picked, the editor can't know its locale, so links lose the?locale=context and the cross-locale badge/behavior cannot work until the next hydration round-trip.Make three small changes:
- Add
localetoPickedContentEntryandContentPickerModalProps. - Pass the active locale into
fetchContentListso the result is scoped to the editor locale. - Preserve the picked item's
localewhen building thePickedContentEntryobjects (aroundtogglePickedandhandleSingleChoose).
export interface PickedContentEntry { collection: string; id: string; slug: string | null; title: string; locale?: string; } interface ContentPickerModalProps { open: boolean; onOpenChange: (open: boolean) => void; collection?: string; multiple?: boolean; selectedIds?: ReadonlySet<string>; onConfirm: (rows: PickedContentEntry[]) => void; title?: string; /** Active editor locale. When set, the list is scoped to that locale. */ locale?: string; }const { data, isLoading, fetchNextPage, hasNextPage, isFetchingNextPage } = useInfiniteQuery({ queryKey: ["content-picker", activeCollection, trimmedSearch, locale], queryFn: ({ pageParam }) => fetchContentList(activeCollection, { limit: 50, cursor: pageParam, search: trimmedSearch || undefined, locale, }),next[item.id] = { collection: activeCollection, id: item.id, slug: item.slug, title: getItemTitle(item), locale: item.locale, }; - Add
-
[needs fixing]
packages/core/src/schema/types.ts:317-334handleContentGetnow hydratesitem.referencesas a top-levelContentItemproperty, alongsideterms,bylines, andbyline. Those existing runtime-hydrated fields are reserved so user-defined fields cannot shadow them, butreferencesis missing. A collection with a non-reference field namedreferenceswould return bothdata.referencesandreferencesin the response, causing confusion and making the schema contract impossible to satisfy cleanly.Add
referencesto the reserved list afterbyline:// Runtime-hydrated fields "terms", "bylines", "byline", "references",
|
Re: review — addressed two of the three, declining the third. #1 — #2 — #3 — reserve |
There was a problem hiding this comment.
Re-review of the reference field feature.
The storage-less reference-field approach is still the right fit: edges live in _emdash_content_references, writes are atomic inside the content transaction, hydration is opt-in, and the admin picker/backlink UI follows the existing patterns. The previously-flagged schema gaps are closed in this push:
contentItemSchemanow declares the runtime-hydratedreferencesshape viareferenceChildrenResponseSchema.entryRefSchemacarriestitleandlocale.createFieldRelationvalidates both the parent and target collections.
Two issues from the last review remain partially or fully unaddressed:
-
Content picker still fetches across all locales.
ContentPickerModalnow acceptslocaleand threads it into the picked row, but it does not sendlocaletofetchContentListor include it in the query key. The server returns every translation; the modal only collapses them client-side. That still produces duplicate rows when translations exist, and switching locales while the modal is cached can show the wrong variant. It also meansselectedIds(which holds a concrete entry id) can miss a translation-group sibling that the picker happens to render. -
referencesis still not a reserved field slug.handleContentGetnow places resolved reference children atitem.references, alongsideterms,bylines, andbyline. Those siblings are reserved, butreferencesis missing. A collection with a non-reference field namedreferenceswould emit bothdata.referencesand a top-levelreferences, shadowing the hydrated value and making the contract impossible to satisfy.
I also found one new low-severity item: the docstring at the top of the manifest test file describes the pre-fix behavior and contradicts the test it introduces.
Security/auth, transaction routing, seed edge writes, relation lifecycle, and changeset/test coverage are otherwise in good shape. No blockers, so this is a comment review.
Findings
-
[needs fixing]
packages/admin/src/components/ContentPickerModal.tsx:124-131The
localeprop is now carried onPickedContentEntryand used for client-side translation-group collapsing, but it is not passed to the server request or included in the query key.fetchContentListsupports alocalefilter, so the picker currently fetches every translation and then collapses them in memory. That still produces duplicate translation rows in multi-locale collections and makes the cached result wrong when the active locale changes. It also meansselectedIds(concrete entry ids) can miss sibling variants rendered by the locale collapse.Add
localeto the query key and forward it tofetchContentList:const { data, isLoading, fetchNextPage, hasNextPage, isFetchingNextPage } = useInfiniteQuery({ queryKey: ["content-picker", activeCollection, trimmedSearch, locale], queryFn: ({ pageParam }) => fetchContentList(activeCollection, { limit: 50, cursor: pageParam, search: trimmedSearch || undefined, locale, }), initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.nextCursor, enabled: open && !!activeCollection, }); -
[needs fixing]
packages/core/src/schema/types.ts:333-334handleContentGetnow hydrates references as a top-levelContentItemproperty (alongsideterms,bylines, andbyline). Those existing runtime-hydrated properties are reserved so user-defined fields cannot shadow them, butreferencesis missing. A collection with a non-reference field slug ofreferenceswould return bothdata.referencesand a top-levelreferences, making the response contract and any generated types impossible to satisfy cleanly.Add
referencesto the runtime-hydrated reserved list:// Runtime-hydrated fields "terms", "bylines", "byline", "references", ]; -
[suggestion]
packages/core/tests/integration/manifest-reference.test.ts:1-8The docstring at the top of the file says
_buildManifestcopies validation for repeater/file/image fields "but notreferencefields", but the test below asserts that reference-field validation is included after this PR. The stale explanation is confusing and contradicts the test.Update the comment to match the current behavior, e.g.:
/** * `_buildManifest` copies `field.validation` into the manifest descriptor * for repeater/file/image/reference fields so the admin editor can drive * the reference picker widget with the field's `relation`, * `targetCollection`, and `multiple` config. */
|
References are keyed by translation groups; they step-aside the entry's locale. We decide which title to show based on the current entry's locale (the one being edited). Once you translate the current entry to a different locale, its references automatically show titles in the new locale if they have that translation. Navigating to references determines the correct ID based on the same mechanism as the title. |
What does this PR do?
Adds a reference field type end-to-end — a storage-less field that links entries via content-reference edges rather than a column on the collection's table.
Core (
emdash)_emdash_content_referencesinstead of anec_*column. Existing reference columns keep their data but are no longer written.referenceskey and are written atomically with the entry in a single transaction; the content GET hydrates them alongside SEO and bylines.title(from the entry'stitle, thenname), so backlinks and picked entries show a readable label rather than a slug.$ref:value as an edge (seed shape unchanged).injectCoreRoutes.Admin (
@emdash-cms/admin)Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpasses (0 diagnostics)pnpm testpasses (core: 4672 passed / 3 skipped; admin: 1065 passed)pnpm formathas been runmessages.pochanges included in this PRemdash: minor,@emdash-cms/admin: minor)AI-generated code disclosure
Screenshots / test output
🤖 Generated with Claude Code
Remains to be fixed
Remains to be verified