test(plugin-map,plugin-timeline): pin what these two contractEnvelope-6839 waits were standing on

.changeset/pin-unpinned-wait-preconditions-8709.md

@@ -0,0 +1,10 @@
+---
+---
+
+Test-only (no release). `ObjectMap.contractEnvelope-6839` and
+`ObjectTimeline.contractEnvelope-6839` each waited on something that could not
+fail: the map's absence-shaped wait was satisfied by a mount that never showed
+the loading panel and by a `setData` that had not committed yet, and the
+timeline's `data-item-count` not-null clause was inert — `getByTestId` throwing
+was the whole gate, so `"0"` satisfied it. Both waits now observe the
+transition and gate on a settled row count. No package source changed.

packages/plugin-map/src/ObjectMap.contractEnvelope-6839.test.tsx

@@ -34,10 +34,30 @@
  * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns
  * `[]` for everything — an implementation strictly worse than the bug. The
  * `data` and bare-array cases refuse it: same rows, same mount.
+ *
+ * ## The two things this file's wait used to STAND ON without saying so (objectui#8709)
+ *
+ * This is the family's only ABSENCE-shaped wait, and an absence carries two
+ * unstated preconditions. Both are now assertions in `markersThrough`, and both
+ * were measured failing first:
+ *
+ *   1. **The panel was ever on screen.** An absence nothing entered is
+ *      satisfied by a mount that never started. With the component mutated to
+ *      `return null` unconditionally the refusal case PASSED — "render nothing,
+ *      ever" is strictly worse than the bug and cleared the old bar.
+ *   2. **`setData` and `setLoading(false)` commit TOGETHER.** The panel is a
+ *      settle signal, not a rows signal; it is only a usable proxy for "the
+ *      rows are on screen" because the two `setState` calls land in one commit.
+ *      With `setData` deferred by 50ms and the `records` arm restored to
+ *      `extractRecords`, the refusal case read zero markers and PASSED while
+ *      the settled map plotted two.
+ *
+ * ⭐ Neither gate subsumes the other: (1) alone still lets a split commit
+ * through, and (2) alone still passes a component that renders nothing.
  */
 
 import React from 'react';
-import { render, screen, waitFor, cleanup } from '@testing-library/react';
+import { act, render, screen, waitFor, cleanup } from '@testing-library/react';
 import { describe, it, expect, vi, afterEach } from 'vitest';
 
 // The MapLibre canvas, replaced by DOM the test can count. Copied from
@@ -72,6 +92,17 @@ const schema: any = {
   data: { provider: 'object', object: 'store' },
 };
 
+/**
+ * The window held open AFTER the loading panel clears, to prove no markers
+ * arrive behind it.
+ *
+ * ⚠️ 50ms, not 0ms. RTL's `asyncWrapper` drains one macrotask before it
+ * returns, so a commit scheduled with `setTimeout(…, 0)` lands INSIDE that
+ * drain window — a 0ms window cannot tell a deferred commit from a
+ * same-commit one, and reports "stable" for both (measured on objectui#8664).
+ */
+const POST_SETTLE_MS = 50;
+
 /** How one case wraps its rows on the way back out of `find()`. */
 type Envelope = (rows: unknown[]) => unknown;
 
@@ -105,6 +136,17 @@ async function markersThrough(envelope: Envelope): Promise<number> {
     })),
   };
   render(<ObjectMap schema={schema} dataSource={ds} enableClustering={false} />);
+  // ① The transition's START, observed — the half an ABSENCE-shaped wait cannot
+  // supply for itself. "The panel has cleared" and "nothing was ever rendered"
+  // are the SAME DOM, so without this line the wait below is satisfied by a
+  // mount that never started. MEASURED, not argued: with `ObjectMap` mutated to
+  // `return null` unconditionally — an implementation strictly worse than the
+  // bug — the refusal case below still PASSED (objectui#8709 leg B). It does
+  // not survive this line.
+  expect(
+    screen.getByText('Loading map...'),
+    'the loading panel must be on screen BEFORE the absence wait — an absence nothing entered is satisfied by never having started',
+  ).toBeInTheDocument();
   await waitFor(() => expect(find).toHaveBeenCalled());
   // `find`'s OWN answer, settled — a pure read of the mock's call record that
   // touches no DOM. Without it "no markers" is satisfied by the mount's
@@ -114,7 +156,29 @@ async function markersThrough(envelope: Envelope): Promise<number> {
   // rather than a rows signal, which is exactly what makes it usable as the
   // one wait shared by the live cases and the refusal case.
   await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull());
-  return screen.queryAllByTestId('map-marker').length;
+  const atPanelClear = screen.queryAllByTestId('map-marker').length;
+  // ② The unstated precondition the line above STANDS ON, now pinned:
+  // `setData(capped.rows)` and `setLoading(false)` reach the DOM in ONE commit.
+  // React 18 batches them today because they sit in one `await` continuation —
+  // true, load-bearing, and asserted by nothing else in this repo. The day they
+  // split (a transition, an async boundary, a `startTransition`) the panel
+  // clears over an EMPTY `data` and the count read above is the INTERMEDIATE
+  // state. MEASURED: with `setData` deferred by 50ms and the `records` arm
+  // restored to `extractRecords`, the refusal case read zero markers and PASSED
+  // while the settled map plotted two — the bug went undetected (leg A).
+  await act(async () => {
+    await new Promise((resolve) => setTimeout(resolve, POST_SETTLE_MS));
+  });
+  // A dependency landing mid-window (`objectSchema`, `perms`) re-runs the fetch
+  // effect and re-raises the panel, so re-settle before re-reading: the claim is
+  // about the SETTLED count, not about a moment inside a refetch.
+  await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull());
+  const settled = screen.queryAllByTestId('map-marker').length;
+  expect(
+    settled,
+    'markers must not arrive AFTER the loading panel clears — the count read at the transition must already be the settled one',
+  ).toBe(atPanelClear);
+  return settled;
 }
 
 afterEach(() => {

packages/plugin-timeline/src/ObjectTimeline.contractEnvelope-6839.test.tsx

@@ -32,10 +32,27 @@
  * The `data` and bare-array cases are the ones that refuse it: they push the
  * SAME rows through the SAME mount, so an arm that delivers nothing delivered
  * nothing because the envelope was refused.
+ *
+ * ## What this file's wait used to STAND ON without saying so (objectui#8709)
+ *
+ * The wait named `data-item-count` and did not gate on it. `getByTestId`
+ * THROWING inside `waitFor` was the entire gate; by the time the attribute was
+ * read the node existed and the renderer double writes the attribute
+ * unconditionally, so `.not.toBeNull()` could never be the clause that failed.
+ * ⭐ That is worse than no clause: it reads like a row-count gate and is a
+ * mount signal, satisfied the instant the renderer appears — `"0"` included.
+ *
+ * MEASURED, not argued: with `setFetchedData` deferred by 50ms and the
+ * `records` arm restored to `extractRecords`, the wait was satisfied at
+ * `data-item-count="0"` and the refusal case PASSED while the settled timeline
+ * drew two rows. `itemsThrough` now gates on the count the wait names AND on
+ * that count surviving a settle window, with the loading skeleton observed
+ * first so a renderer mounted empty on the first paint is not read as a
+ * settled zero.
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { render, waitFor, cleanup, screen } from '@testing-library/react';
+import { act, render, waitFor, cleanup, screen } from '@testing-library/react';
 import React from 'react';
 
 // The timeline's own visualisation is orthogonal to what this file observes
@@ -55,6 +72,17 @@ const ROWS = [
   { id: 't2', subject: 'Ship it again', starts_at: '2026-01-02T09:00:00Z' },
 ];
 
+/**
+ * The window held open AFTER the renderer reports the expected count, to prove
+ * no further rows arrive behind it.
+ *
+ * ⚠️ 50ms, not 0ms. RTL's `asyncWrapper` drains one macrotask before it
+ * returns, so a commit scheduled with `setTimeout(…, 0)` lands INSIDE that
+ * drain window — a 0ms window cannot tell a deferred commit from a same-commit
+ * one, and reports "stable" for both (measured on objectui#8664).
+ */
+const POST_SETTLE_MS = 50;
+
 /** How one case wraps its rows on the way back out of `find()`. */
 type Envelope = (rows: unknown[]) => unknown;
 
@@ -69,8 +97,14 @@ const schema: any = {
   startDateField: 'starts_at',
 };
 
-/** Mount the timeline over a `find()` answering `envelope`, return rows drawn. */
-async function itemsThrough(envelope: Envelope): Promise<number> {
+/**
+ * Mount the timeline over a `find()` answering `envelope`, return rows drawn.
+ *
+ * `expectedItems` is the count this arm claims the envelope produces, and it is
+ * what the wait GATES on — see the comments in the body for why the count has
+ * to be named here rather than merely read at the end.
+ */
+async function itemsThrough(envelope: Envelope, expectedItems: number): Promise<number> {
   const ds: Record<string, any> = {
     find: vi.fn(async () => envelope(ROWS)),
     findOne: vi.fn(),
@@ -87,15 +121,56 @@ async function itemsThrough(envelope: Envelope): Promise<number> {
     })),
   };
   render(<ObjectTimeline schema={schema} dataSource={ds as any} />);
+  // ① The transition's START, observed. Without it a component that mounts the
+  // renderer EMPTY on its first paint and fetches afterwards clears every bar
+  // below on the refusal arm — `"0"`, stable, forever. The skeleton is the
+  // proof that this mount actually went through a loading phase.
+  //
+  // ⚠️ `toBeTruthy`, not `toBeInTheDocument`: this package's
+  // `tsconfig.test.json` does not name `@testing-library/jest-dom` in `types`
+  // (the sibling `plugin-map` one does), so that matcher is green under vitest
+  // and TS2339 under `tsc -p tsconfig.test.json`. `getByTestId` THROWING when
+  // the skeleton is absent is the assertion either way; the matcher only
+  // attaches the message.
+  expect(
+    screen.getByTestId('timeline-loading'),
+    'the loading skeleton must be on screen first — a renderer mounted empty from the first paint is not a settled zero',
+  ).toBeTruthy();
   await waitFor(() => expect(ds.find).toHaveBeenCalled());
   // `find`'s OWN answer, settled — a pure read of the mock's call record that
   // touches no DOM. Without it the assertion can be satisfied by the mount's
   // initial empty state, which every arm renders identically.
   await ds.find.mock.results[0].value;
+  // ② Gate on the count this wait NAMES, at the value this arm claims.
+  //
+  // The clause here used to be `.not.toBeNull()`, and it was INERT: the
+  // attribute is written unconditionally by the renderer double, so once
+  // `getByTestId` stops throwing the attribute is always a string. The THROW
+  // was the whole gate, which made this a bare mount signal wearing the
+  // vocabulary of a row count — `"0"` satisfied it, so an arm expecting rows
+  // and an arm expecting none waited on exactly the same event.
   await waitFor(() =>
-    expect(screen.getByTestId('timeline-renderer').getAttribute('data-item-count')).not.toBeNull(),
+    expect(
+      screen.getByTestId('timeline-renderer').getAttribute('data-item-count'),
+      'the renderer must report the row count this envelope produces, not merely exist',
+    ).toBe(String(expectedItems)),
   );
-  return Number(screen.getByTestId('timeline-renderer').getAttribute('data-item-count'));
+  // ③ … and it must STILL be that count after the settle window. This is the
+  // half ② cannot supply on the refusal arm, where the expected value is `"0"`
+  // and `"0"` is also what an unpopulated renderer reports. MEASURED: with
+  // `setFetchedData` deferred by 50ms and the `records` arm restored to
+  // `extractRecords`, the old wait was satisfied at `data-item-count="0"` and
+  // the refusal case PASSED while the settled timeline drew two rows
+  // (objectui#8709 leg A).
+  await act(async () => {
+    await new Promise((resolve) => setTimeout(resolve, POST_SETTLE_MS));
+  });
+  const settled = screen.getByTestId('timeline-renderer').getAttribute('data-item-count');
+  expect(
+    settled,
+    'rows must not arrive AFTER the count was read — a count that moves in the settle window was an intermediate state',
+  ).toBe(String(expectedItems));
+  return Number(settled);
 }
 
 beforeEach(() => {
@@ -113,18 +188,18 @@ describe('ObjectTimeline — the find() envelope it reads (objectui#6839)', () =
     // (objectui#7802): it renders, and `waitFor` re-runs its callback on DOM
     // mutations, so the predicate feeds itself and leaks a container div per
     // run. The render happens once, out here, and the case reads its answer.
-    expect(await itemsThrough(asData), 'the declared rows member must still draw').toBe(2);
+    expect(await itemsThrough(asData, 2), 'the declared rows member must still draw').toBe(2);
   });
 
   it('still reads a bare array — the live non-envelope shape fakes answer with', async () => {
-    expect(await itemsThrough(asBareArray), 'the bare-array arm must still draw').toBe(2);
+    expect(await itemsThrough(asBareArray, 2), 'the bare-array arm must still draw').toBe(2);
   });
 
   it('does NOT read `records` — not a QueryResult member', async () => {
     // Before the fix these two rows drew off a key `QueryResult` does not
     // declare, and did so AHEAD of `data`.
     expect(
-      await itemsThrough(asRecords),
+      await itemsThrough(asRecords, 0),
       'a `records` envelope must reach the rail as zero rows, not as the rows it names',
     ).toBe(0);
   });