Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): derive a marker-title binding in defaultMapFromObject - #5955
Merged
Conversation
`defaultMapFromObject` bound only `locationField`, so an ADR-0047 interface page that whitelists `map` reached `ObjectMap` with no `titleField`. `getMapConfig` fills that gap with the literal `'name'` and the marker title is a plain `record[titleField]` read, so every marker popup on an object whose display field is not `name` titled itself `undefined`. Bind the object's display field, resolved with the field-name half of ADR-0079's precedence: the declared `nameField` (and its `displayNameField` / `NAME_FIELD_KEY` aliases), else the shared `deriveTitleField` scan — imported from `@object-ui/core` rather than reimplemented, so this binding and the sibling renderers can never rank fields differently. The key is omitted, not defaulted, when nothing resolves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZyKZejBWZoCSj1NP35wcp
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019ZyKZejBWZoCSj1NP35wcp
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-warren
marked this pull request as ready for review
August 24, 2026 04:40
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#5909
The defect, and which path produces it
defaultMapFromObjectbound onlylocationField. An ADR-0047 interface page that whitelistsmaptherefore reachedObjectMapwith notitleField, andgetMapConfigfills that gap with a string literal (packages/plugin-map/src/ObjectMap.tsx:377):The marker title is then a bare property read (
ObjectMap.tsx:667), sorecord['name']on an object whose display field is notnameisundefined— every marker popup titles itselfundefined.The call site was reshaped by #5908 the day before, so the two paths were traced before touching either:
Both paths converge on the same flat branch of
getMapConfig:ListView'smapcase emits the flat form deliberately (a nestedmapkey would win outright atgetMapConfigper the #5018 ruling), so a declared block and a derived one alike land ontitleField: schema.titleField || 'name'. The difference is the escape hatch — an author-supplied block can declaretitleField; the derivation had no way to supply one at all. That is the path fixed here.titleFieldis onFLAT_MAP_CONFIG_KEYS, so the derived key survivespickFlatMapConfig's whitelisted flatten and reachesgetMapConfigahead of the literal — checked, because a key outside that whitelist would have made this a silent no-op.The card's "unlike every sibling deriver" premise is false as stated — measured, not assumed
The card argues this deriver was the odd one out. It was not. No deriver in
InterfaceListPage.tsxbinds a title; each binds its viz's own required field and stops:defaultKanbanFromObject{ groupByField }defaultCalendarFromObject{ startDateField }defaultGalleryFromObject{ coverField }defaultGanttFromObject{ startDateField, endDateField, progressField? }defaultMapFromObject{ locationField }— its own required fieldThe real asymmetry is one layer down, at the renderers:
ObjectKanban(:301),ObjectCalendar(:356) andObjectGantt(:600) all resolve their item title through@object-ui/core#getRecordDisplayName(ADR-0079), so they need nothing derived.ObjectMapnever imports the resolver. That is why the siblings look like they "bind their display field" — they resolve it, at render time, and map alone does not.So this PR fixes the user-visible failure at the seam available to it, and does not claim to be the general fix. The renderer asymmetry is filed separately as #5953 — that one covers a hand-declared block omitting
titleField, maps outside interface pages entirely, and thetitleFormat/record-probe steps a static field-name binding structurally cannot carry. #5953 is not addressed here.The rule picked, and why that one
Rather than invent a heuristic, the binding reuses the field-name half of the ADR-0079 precedence the sibling renderers already resolve through:
objectDef.nameField, then the deprecateddisplayNameField/NAME_FIELD_KEYaliases;deriveTitleField(objectDef)— the shared type-aware scan, imported from@object-ui/core, not reimplemented, so this binding and the renderers can never rank fields differently.Steps deliberately not taken: step 0 (
objectDef.titleField) is the caller's explicit choice, which is what is being computed here; step 3 (titleFormat) is a render-only template, not a field name; steps 4b/5 read a record, which an object-derived binding has none of.deriveTitleField's own eligibility filter is used as-is rather than being re-screened through this file'shidden/system-managed filter — screening differently here would reintroduce exactly the per-view dialect ADR-0079 removed.When nothing resolves the key is omitted, not defaulted: an absent key leaves current
ObjectMapbehaviour standing, whereas a fabricated one is indistinguishable from a declared choice at the read site.Tests
packages/app-shell/src/views/InterfaceListPage.mapConfig.test.tsx. The discriminating arm pins the not-namecase specifically — an object keyed bytitle— since an arm on an object whose display field isnamepasses on the defect (that arm is present too, labelled CONTROL, and is not alone). Also pinned: the declarednameFieldoutranking the scan (load-bearing —deriveTitleFieldalone rankstitleaboveheadlinethere, so a scan-only implementation fails it), the deprecated aliases, the*_nameaffix convention, omission when no field is title-eligible, and an anti-drift arm assertingrecord[derivedTitleField] === getRecordDisplayName(objectDef, record)so a map and a board over one object cannot disagree about what a record is called.Two pre-existing arms from #5042 changed expectation: their fixture object is
showcase_task(title+location), i.e. exactly the not-namecase, so the derivation product is now{ locationField: 'location', titleField: 'title' }.Evidence — all on final head
adb8cec0eReverse-verification, run from the committed fix and restored by an
EXIT/INT/TERMtrap. Mutation confirmed on disk by grepping the text meant to be removed (displayFieldOfObject0,deriveTitleField0, old signature 1), and the restore leg re-confirmed (2 / 4) plus a cleangit status. Predicted direction was RED with the two "nothing resolves" arms surviving; observed exactly that:A second reverse-check proves the new cross-package import is genuinely type-checked rather than silently
any— pasting a value the real signature must reject turnstscred by name:Union on final head (dependency closure
pnpm --filter '@object-ui/app-shell^...' buildbuilt first, exit 0):vitest(4 files, path-filtered)Test Files 4 passed (4)/Tests 101 passed (101)pnpm --filter @object-ui/app-shell type-check> tsc --noEmit && tsc -p tsconfig.test.json)eslint .inpackages/app-shellturbo run lintdoes, not a narrowing (105 s)check-changeset-presence✅ 2 source file(s) … declares 1 changeset(s)check-control-bytes✅ scanned 4940 tracked text file(s)check-changeset-no-major✅ No changeset declares a 'major' bumpcheck-phantom-dependencies✅ Every in-scope import is declared by the package that publishes itcheck-eager-closure-budgetreports a broken gauge locally, not a failure — it needsapps/console/dist/eager-closure.jsonfrom a fullvite build. No new eager edge is added regardless:InterfaceListPage.tsxalready importsListViewfrom@object-ui/plugin-list, whoseListView.tsx:24imports@object-ui/core, so core was already in this module's eager closure. CI runs the real gauge.Repo-wide scans (
pnpm check, the rest of the farm) are CI's run.Generated by Claude Code
Generated by Claude Code