Skip to content

Commit 186f5a1

Browse files
os-zhuangclaude
andauthored
perf+feat(core): memoize derived title field + make nameField canonical (ADR-0079 Phase 2) (#2091)
record-title resolver follow-ups: 1. perf: memoize deriveTitleField per objectDef via a module-level WeakMap<object, string|undefined>, so a 1000-row list scans objectDef.fields once instead of 1000x. Behavior byte-identical; only the field scan is cached, and only for non-null object defs (split into computeDerivedTitleField + a memoizing wrapper). 2. Phase 2 (conservative): promote objectDef.nameField to the canonical record-title pointer and de-prioritize the legacy render-only titleFormat template. New precedence in getRecordDisplayName: options.titleField -> objectDef.nameField -> displayNameField alias -> titleFormat (LEGACY, now AFTER the declared field) -> type-aware derivation -> record name-ish keys -> Record #<id>. An object that sets nameField resolves via it even with a stale titleFormat. titleFormat is KEPT for back-compat (not removed); added an ADR-0079 deprecation note on its branch. Tests: +nameField-wins-over-stale-titleFormat, titleFormat-only back-compat, and memoization (same field across calls + no re-scan via a read-counting Proxy + cached-undefined + cross-objectDef isolation). record-title suite 66/66 green; @object-ui/core, app-shell, plugin-list build green. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3622f6a commit 186f5a1

2 files changed

Lines changed: 130 additions & 2 deletions

File tree

packages/core/src/utils/__tests__/record-title.test.ts

Lines changed: 130 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,26 @@ describe('getRecordDisplayName — *_name on the RECORD when objectDef is unusab
6363
});
6464

6565
describe('getRecordDisplayName — precedence', () => {
66-
it('1. titleFormat wins over everything', () => {
66+
// ADR-0079 Phase 2: an explicitly-declared field outranks the legacy
67+
// render-only `titleFormat` template. The object declares BOTH a
68+
// `displayNameField` and a `titleFormat`; the declared field wins.
69+
it('1. declared displayNameField wins over a legacy titleFormat', () => {
6770
const obj = {
6871
titleFormat: '{first} {last}',
6972
displayNameField: 'name',
7073
fields: { name: { type: 'text' }, first: { type: 'text' }, last: { type: 'text' } },
7174
};
72-
const rec = { id: '1', name: 'Ignored', first: 'Ada', last: 'Lovelace' };
75+
const rec = { id: '1', name: 'Declared', first: 'Ada', last: 'Lovelace' };
76+
expect(getRecordDisplayName(obj, rec)).toBe('Declared');
77+
});
78+
79+
// titleFormat is still honored when no declared field resolves (back-compat).
80+
it('1b. titleFormat still renders when no nameField/displayNameField is declared', () => {
81+
const obj = {
82+
titleFormat: '{first} {last}',
83+
fields: { first: { type: 'text' }, last: { type: 'text' } },
84+
};
85+
const rec = { id: '1', first: 'Ada', last: 'Lovelace' };
7386
expect(getRecordDisplayName(obj, rec)).toBe('Ada Lovelace');
7487
});
7588

@@ -143,6 +156,121 @@ describe('getRecordDisplayName — precedence', () => {
143156
});
144157
});
145158

159+
describe('getRecordDisplayName — ADR-0079 Phase 2 (nameField canonical)', () => {
160+
// (a) nameField is the NEW canonical pointer and must win even when the object
161+
// also carries a stale render-only titleFormat.
162+
it('resolves via nameField even when a stale titleFormat is present (nameField wins)', () => {
163+
const obj = {
164+
nameField: 'activity_name',
165+
titleFormat: '{wrong}',
166+
fields: { activity_name: { type: 'text' }, wrong: { type: 'text' } },
167+
};
168+
const rec = { id: '1', activity_name: '夏日城市骑行夜', wrong: 'STALE TEMPLATE' };
169+
expect(getRecordDisplayName(obj, rec)).toBe('夏日城市骑行夜');
170+
});
171+
172+
it('nameField outranks the displayNameField alias', () => {
173+
const obj = {
174+
nameField: 'activity_name',
175+
displayNameField: 'legacy_name',
176+
fields: { activity_name: { type: 'text' }, legacy_name: { type: 'text' } },
177+
};
178+
const rec = { id: '1', activity_name: 'Canonical', legacy_name: 'Alias' };
179+
expect(getRecordDisplayName(obj, rec)).toBe('Canonical');
180+
});
181+
182+
it('falls through to displayNameField when nameField is empty on the record', () => {
183+
const obj = {
184+
nameField: 'activity_name',
185+
displayNameField: 'legacy_name',
186+
fields: { activity_name: { type: 'text' }, legacy_name: { type: 'text' } },
187+
};
188+
const rec = { id: '1', activity_name: ' ', legacy_name: 'Alias' };
189+
expect(getRecordDisplayName(obj, rec)).toBe('Alias');
190+
});
191+
192+
// (b) An object with ONLY a titleFormat (no nameField/displayNameField) still
193+
// resolves via the template — back-compat preserved.
194+
it('back-compat: titleFormat-only object still resolves via the template', () => {
195+
const obj = {
196+
titleFormat: '{first} · {last}',
197+
fields: { first: { type: 'text' }, last: { type: 'text' } },
198+
};
199+
const rec = { id: '1', first: 'Ada', last: 'Lovelace' };
200+
expect(getRecordDisplayName(obj, rec)).toBe('Ada · Lovelace');
201+
});
202+
});
203+
204+
describe('deriveTitleField — memoization (per objectDef, not per record)', () => {
205+
it('returns the same derived field across repeated calls for one objectDef', () => {
206+
const obj = { fields: { activity_name: { type: 'text' }, start_date: { type: 'date' } } };
207+
const first = deriveTitleField(obj);
208+
expect(first).toBe('activity_name');
209+
// Repeated calls (the per-record hot path) return the identical result.
210+
for (let i = 0; i < 5; i++) expect(deriveTitleField(obj)).toBe('activity_name');
211+
});
212+
213+
it('does NOT re-scan objectDef.fields after the first call (cached)', () => {
214+
// Lightweight spy: a `fields` map whose property reads are counted. After the
215+
// first deriveTitleField the cache must serve subsequent calls WITHOUT
216+
// touching `fields` again (zero further reads).
217+
let reads = 0;
218+
const fields = new Proxy(
219+
{ activity_name: { type: 'text' }, start_date: { type: 'date' } } as Record<string, any>,
220+
{
221+
get(target, prop, receiver) {
222+
reads++;
223+
return Reflect.get(target, prop, receiver);
224+
},
225+
ownKeys(target) {
226+
reads++;
227+
return Reflect.ownKeys(target);
228+
},
229+
getOwnPropertyDescriptor(target, prop) {
230+
return Reflect.getOwnPropertyDescriptor(target, prop);
231+
},
232+
},
233+
);
234+
const obj = { fields };
235+
236+
expect(deriveTitleField(obj)).toBe('activity_name');
237+
const readsAfterFirst = reads;
238+
expect(readsAfterFirst).toBeGreaterThan(0); // the first scan touched fields
239+
240+
for (let i = 0; i < 10; i++) deriveTitleField(obj);
241+
// No additional reads — every later call hit the WeakMap cache.
242+
expect(reads).toBe(readsAfterFirst);
243+
});
244+
245+
it('memoizes an undefined result (object with no title-eligible field)', () => {
246+
let scans = 0;
247+
const obj = {
248+
get fields() {
249+
scans++;
250+
return { when: { type: 'date' }, qty: { type: 'number' } };
251+
},
252+
};
253+
expect(deriveTitleField(obj)).toBeUndefined();
254+
expect(deriveTitleField(obj)).toBeUndefined();
255+
// `fields` getter invoked exactly once → the cached `undefined` was reused.
256+
expect(scans).toBe(1);
257+
});
258+
259+
it('keeps distinct objectDefs independent (no cross-contamination)', () => {
260+
const a = { fields: { subject: { type: 'text' } } };
261+
const b = { fields: { full_name: { type: 'text' } } };
262+
expect(deriveTitleField(a)).toBe('subject');
263+
expect(deriveTitleField(b)).toBe('full_name');
264+
expect(deriveTitleField(a)).toBe('subject');
265+
});
266+
267+
it('does not cache primitives/null (computes uncached, no throw)', () => {
268+
expect(deriveTitleField(null)).toBeUndefined();
269+
expect(deriveTitleField(undefined)).toBeUndefined();
270+
expect(deriveTitleField('nope' as any)).toBeUndefined();
271+
});
272+
});
273+
146274
describe('formatTitleTemplate — handlebars / double-brace', () => {
147275
it('renders double-brace {{field}} placeholders', () => {
148276
const obj = { titleFormat: '{{first_name}} {{last_name}}' };
3.08 KB
Binary file not shown.

0 commit comments

Comments
 (0)