fix(fields): lookup multi-value hydration batches via $in and shows loading instead of the empty placeholder (#3108)

baozhoutao · claude · baozhoutao · commit 2c7b4e5521e4 · 2026-07-31T05:03:52.000-07:00
A multi-value lookup hydrated its committed ids with a SERIAL findOne per
id — 41 selected users meant 41 sequential round-trips, and for those
seconds the trigger rendered the bare "Select…" placeholder,
indistinguishable from having no value at all (downstream users read a
populated field as empty and could clear it by hand).

- Several unresolved ids now hydrate with one `$in` query per
  LOOKUP_PAGE_SIZE chunk (chunks in parallel); the single-id case keeps
  the pre-existing cheap paths (primary-id findOne GET / equality filter
  for id_field, #3508).
- While any committed bare id is still unresolved, the trigger (and the
  readonly renderer) shows a spinner + "N selected" instead of the empty
  placeholder. A settled-set keeps the spinner from sticking forever on
  ids that will never resolve (deleted records, fetch failures) — those
  fall back to the pre-existing drop behaviour.

The committed value itself is never rewritten during hydration
(handleSelect/handleRemove operate on the raw value), so submitting
mid-hydration was already lossless; the loading state removes the visual
lie that invited manual clearing.

Co-Authored-By: Claude Fable 5 &lt;noreply@anthropic.com&gt;
diff --git a/packages/fields/src/widgets/LookupField.hydration.test.tsx b/packages/fields/src/widgets/LookupField.hydration.test.tsx
@@ -0,0 +1,136 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Regression (#3108): a multi-value lookup hydrated its committed ids with a
+ * SERIAL `findOne` per id — 41 selected users meant 41 sequential round-trips,
+ * during which the trigger rendered the bare "Select…" placeholder,
+ * indistinguishable from having no value at all.
+ *
+ * Fixed behaviour under test:
+ *  1. unresolved ids are fetched in ONE batched `$in` query (chunked), not
+ *     one findOne per id;
+ *  2. while hydration is in flight the trigger shows a loading indicator and
+ *     the committed-value count instead of the empty placeholder;
+ *  3. the loading state settles (falls back to placeholder) when hydration
+ *     fails, instead of spinning forever.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, cleanup, waitFor, screen } from '@testing-library/react';
+import { LookupField } from './LookupField';
+
+afterEach(cleanup);
+
+const USERS = [
+  { id: 'u1', name: 'Ada Lovelace' },
+  { id: 'u2', name: 'Grace Hopper' },
+  { id: 'u3', name: 'Margaret Hamilton' },
+];
+
+describe('LookupField — multi-value hydration batches and shows loading (#3108)', () => {
+  it('hydrates several ids with one $in query instead of a findOne per id', async () => {
+    const find = vi.fn(async (_obj: string, params: any) => {
+      const wanted: any[] = params?.$filter?.id?.$in ?? [];
+      return { data: USERS.filter((u) => wanted.includes(u.id)) };
+    });
+    const findOne = vi.fn(async () => null);
+    render(
+      <LookupField
+        value={['u1', 'u2', 'u3']}
+        onChange={() => {}}
+        dataSource={{ find, findOne } as never}
+        field={{ reference_to: 'sys_user', multiple: true } as never}
+      />,
+    );
+
+    await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy());
+    expect(screen.getByText('Grace Hopper')).toBeTruthy();
+    expect(screen.getByText('Margaret Hamilton')).toBeTruthy();
+
+    // One batched round-trip, zero per-id GETs.
+    expect(findOne).not.toHaveBeenCalled();
+    const hydrationCalls = find.mock.calls.filter((c) => c[1]?.$filter?.id?.$in);
+    expect(hydrationCalls).toHaveLength(1);
+    expect(hydrationCalls[0][1].$filter.id.$in).toEqual(['u1', 'u2', 'u3']);
+  });
+
+  it('shows the committed count + spinner while hydration is in flight, never the empty placeholder', async () => {
+    let resolveFind!: (v: any) => void;
+    const find = vi.fn(
+      () => new Promise((resolve) => { resolveFind = resolve; }),
+    );
+    render(
+      <LookupField
+        value={['u1', 'u2', 'u3']}
+        onChange={() => {}}
+        dataSource={{ find } as never}
+        field={{ reference_to: 'sys_user', multiple: true } as never}
+      />,
+    );
+
+    // In-flight: loading indicator + raw committed count, no "Select…".
+    await waitFor(() => expect(screen.getByTestId('lookup-hydrating')).toBeTruthy());
+    expect(screen.queryByText('Select...')).toBeNull();
+    expect(screen.getByText(/3/)).toBeTruthy();
+
+    resolveFind({ data: USERS });
+    await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy());
+    expect(screen.queryByTestId('lookup-hydrating')).toBeNull();
+  });
+
+  it('settles (no infinite spinner) when the hydration fetch fails', async () => {
+    const find = vi.fn(async (_obj: string, params: any) => {
+      if (params?.$filter?.id?.$in) throw new Error('boom');
+      return { data: [] };
+    });
+    render(
+      <LookupField
+        value={['u1', 'u2']}
+        onChange={() => {}}
+        dataSource={{ find } as never}
+        field={{ reference_to: 'sys_user', multiple: true } as never}
+      />,
+    );
+
+    await waitFor(() => expect(find).toHaveBeenCalled());
+    // The failed attempt is marked settled — the spinner clears instead of
+    // spinning forever (pre-existing fallback: unresolvable ids drop).
+    await waitFor(() => expect(screen.queryByTestId('lookup-hydrating')).toBeNull());
+  });
+
+  it('readonly mode shows the loading count instead of an empty value while hydrating', async () => {
+    let resolveFind!: (v: any) => void;
+    const find = vi.fn(
+      () => new Promise((resolve) => { resolveFind = resolve; }),
+    );
+    render(
+      <LookupField
+        value={['u1', 'u2']}
+        onChange={() => {}}
+        readonly
+        dataSource={{ find } as never}
+        field={{ reference_to: 'sys_user', multiple: true } as never}
+      />,
+    );
+
+    await waitFor(() => expect(screen.getByTestId('lookup-hydrating')).toBeTruthy());
+
+    resolveFind({ data: USERS.slice(0, 2) });
+    await waitFor(() => expect(screen.getByText('Ada Lovelace')).toBeTruthy());
+  });
+
+  it('keeps the cheap findOne GET for a single unresolved primary id', async () => {
+    const find = vi.fn(async () => ({ data: [] }));
+    const findOne = vi.fn(async () => ({ id: 'u1', name: 'Ada' }));
+    render(
+      <LookupField
+        value={['u1']}
+        onChange={() => {}}
+        dataSource={{ find, findOne } as never}
+        field={{ reference_to: 'sys_user', multiple: true } as never}
+      />,
+    );
+    await waitFor(() => expect(findOne).toHaveBeenCalledWith('sys_user', 'u1'));
+  });
+});
diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx
@@ -205,6 +205,12 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
   // even when the record wasn't part of the Level 1 popover fetch.
   const [pickerResolvedRecords, setPickerResolvedRecords] = useState<LookupOption[]>([]);
 
+  // Ids whose hydration attempt has FINISHED — found or not — keyed by
+  // String(id). Distinguishes "still loading" from "unresolvable" (deleted
+  // record, fetch failure) so the trigger's hydrating state can't stick
+  // forever on an id that will never resolve (#3108).
+  const [hydrationSettled, setHydrationSettled] = useState<Set<string>>(() => new Set());
+
   // Arrow-key active index (-1 = none)
   const [activeIndex, setActiveIndex] = useState(-1);
   const listRef = useRef<HTMLDivElement>(null);
@@ -476,11 +482,12 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
     (async () => {
       try {
         const fetched: LookupOption[] = [];
-        for (const id of unresolved) {
-          // `findOne` resolves by PRIMARY id only. When the field commits a
-          // different column (`id_field: 'name'` — e.g. position machine
-          // names, objectstack #3508), hydrate by filtering on that column or
-          // the stored value would never resolve to a label.
+        // Single id: the pre-existing cheap paths — a primary-id `findOne`
+        // GET, or an equality filter when the field commits a different
+        // column (`id_field: 'name'` — e.g. position machine names,
+        // objectstack #3508).
+        if (unresolved.length === 1) {
+          const id = unresolved[0];
           if (typeof (dataSource as any).findOne === 'function' && idField === 'id') {
             const rec = await (dataSource as any).findOne(referenceTo, id);
             if (rec) fetched.push(recordToOption(rec, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema));
@@ -489,9 +496,35 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
               $filter: { [idField]: id },
               $top: 1,
             } as QueryParams);
-            const rows = res?.data ?? res ?? [];
+            const rows = (res as any)?.data ?? res ?? [];
             if (rows[0]) fetched.push(recordToOption(rows[0], displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema));
           }
+        } else {
+          // SEVERAL unresolved ids: one `$in` query per chunk. A multi-value
+          // field can hold dozens of ids, and the old serial findOne-per-id
+          // took seconds while the trigger sat on the empty "Select…"
+          // placeholder (#3108).
+          // Chunked to LOOKUP_PAGE_SIZE so no request exceeds the page size
+          // a server may cap `$top` at; chunks run in parallel.
+          const chunks: any[][] = [];
+          for (let i = 0; i < unresolved.length; i += LOOKUP_PAGE_SIZE) {
+            chunks.push(unresolved.slice(i, i + LOOKUP_PAGE_SIZE));
+          }
+          const results = await Promise.all(
+            chunks.map((chunk) =>
+              dataSource.find(referenceTo, {
+                $filter: { [idField]: { $in: chunk } },
+                $top: chunk.length,
+              } as QueryParams),
+            ),
+          );
+          for (const res of results) {
+            const rows = (res as any)?.data ?? res ?? [];
+            if (!Array.isArray(rows)) continue;
+            for (const row of rows) {
+              fetched.push(recordToOption(row, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema));
+            }
+          }
         }
         if (!cancelled && fetched.length) {
           setPickerResolvedRecords((prev) => {
@@ -502,6 +535,17 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
         }
       } catch {
         // Ignore — chip will fall back to showing the raw id.
+      } finally {
+        // Mark the attempt settled — found or not — so `hydrating` below
+        // clears even for ids that no longer resolve (deleted records,
+        // fetch failures) instead of spinning forever.
+        if (!cancelled) {
+          setHydrationSettled((prev) => {
+            const next = new Set(prev);
+            for (const id of unresolved) next.add(String(id));
+            return next;
+          });
+        }
       }
     })();
     return () => {
@@ -580,6 +624,31 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
     [findOption, findOptionLoose, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema],
   );
 
+  // A value can hold bare ids that no option list resolves YET — the batch
+  // hydration above is still in flight. While any such id is pending, the
+  // trigger must not present the field as empty: a 41-value lookup rendered
+  // the bare "Select…" placeholder for seconds, indistinguishable from
+  // having no value at all (#3108).
+  const hydrating = useMemo(() => {
+    if (!hasDataSource) return false;
+    const raw: any[] = multiple
+      ? Array.isArray(value) ? value : []
+      : value != null && value !== '' ? [value] : [];
+    return raw.some(
+      (v) =>
+        v != null && v !== '' && typeof v !== 'object' && !parseReferenceObjectString(v) &&
+        !findOption(v) && !findOptionLoose(v) &&
+        !hydrationSettled.has(String(v)),
+    );
+  }, [hasDataSource, multiple, value, findOption, findOptionLoose, hydrationSettled]);
+
+  // Committed-value count. During hydration the resolved-option count
+  // undercounts (unresolved ids are filtered out below), so the trigger
+  // shows this raw count instead.
+  const rawSelectedCount = multiple
+    ? (Array.isArray(value) ? value : []).filter((v) => v != null && v !== '').length
+    : value != null && value !== '' ? 1 : 0;
+
   const selectedOptions = multiple
     ? (Array.isArray(value) ? value : []).map(resolveSelectedOption).filter(Boolean)
     : value ? [resolveSelectedOption(value)].filter(Boolean) : [];
@@ -801,6 +870,17 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
 
   if (readonly) {
     if (!selectedOptions.length) {
+      if (hydrating) {
+        return (
+          <span
+            className="inline-flex items-center gap-1.5 text-sm text-muted-foreground"
+            data-testid="lookup-hydrating"
+          >
+            <Loader2 className="size-3.5 animate-spin" />
+            {multiple ? t('table.selected', { count: rawSelectedCount }) : t('lookup.loading')}
+          </span>
+        );
+      }
       return <EmptyValue />;
     }
 
@@ -846,15 +926,28 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
         ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') })
         : undefined}
     >
-      <Search className={cn('size-4 shrink-0 text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')} />
+      {hydrating ? (
+        <Loader2
+          className={cn('size-4 shrink-0 animate-spin text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')}
+          data-testid="lookup-hydrating"
+        />
+      ) : (
+        <Search className={cn('size-4 shrink-0 text-muted-foreground', compact ? 'mr-1.5' : 'mr-2')} />
+      )}
       <span className={cn('truncate', compact && selectedOptions.length === 0 && 'text-muted-foreground')}>
         {dependenciesMissing
           ? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') })
-          : compact && !multiple && selectedOptions.length > 0
-            ? singleSelectedLabel
-            : selectedOptions.length === 0
-              ? lookupField?.placeholder || t('common.select')
-              : multiple ? t('table.selected', { count: selectedOptions.length }) : t('common.select')}
+          : hydrating
+            // The value EXISTS but its labels are still loading — say so
+            // instead of the empty placeholder (#3108).
+            ? multiple
+              ? t('table.selected', { count: rawSelectedCount })
+              : t('lookup.loading')
+            : compact && !multiple && selectedOptions.length > 0
+              ? singleSelectedLabel
+              : selectedOptions.length === 0
+                ? lookupField?.placeholder || t('common.select')
+                : multiple ? t('table.selected', { count: selectedOptions.length }) : t('common.select')}
       </span>
     </Button>
   );
