Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions .changeset/7895-timeline-settled-schema-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
'@object-ui/plugin-timeline': patch
---

`ObjectTimeline` waits for the object definition instead of querying twice
(objectui#7895).

It was the last member of the set objectui#6482 converged on the shared
settled-schema gate — `ObjectKanban`, `ObjectView`, `ObjectCalendar` and
`ObjectTree` were named there, `ObjectGantt` was ask 2 of objectui#7225 — and
nothing marked it a deliberate exclusion. It still held the object definition in
a local `useState` fed by its own metadata effect, and listed that definition in
the record-fetch effect's dependency array.

**User-visible.** Every object-bound timeline load issued **two** `find` calls
instead of one: the first before the definition landed, with `buildExpandFields`
seeing no fields and therefore carrying no `$expand` at all, and a second one
after. Whenever the metadata read is the slower of the two — the common case on
a cold metadata cache — the second call is not merely a wasted round trip but a
**three-step paint**: raw foreign-key ids, back to the loading skeleton (the
effect's re-run calls `setLoading(true)` and `loading` is an early return), then
the expanded rows. After this change the timeline paints once, from a query that
already carries its expansion.

Measured on the component with an instrumented renderer, one mount per hold,
`getObjectSchema` held 0/1/2/3/4/5/6/7/8/9/10/15/25/50/100 ms, with
`ObjectCalendar` and `ObjectGantt` as positive controls in the same run: before,
2 `find` calls with expand sets `[null, ['owner']]`, 1 paint at the readiness
predicate and 3 late writes after it at every hold from +3 ms up; after, 1 `find`
carrying `['owner']`, 1 paint, 0 late writes, and a first-paint time that tracks
the hold (8 ms at +3, 30 ms at +25, 105 ms at +100) where before it was a flat
3-7 ms at every hold. Both controls read 1 paint / 0 late writes before and
after.

The resolution half is now `useSettledSchema` from `@object-ui/react`, which
settles on **every** exit — no source, no `getObjectSchema`, no object name, and
a read that threw alike. That is what makes the gate safe: the replaced effect
returned without settling on all four, which cost nothing while nothing waited on
it and would have held a gated query open forever. Pinned by
`ObjectTimeline.fetchGate-7895.test.tsx`, including a timeline whose adapter
exposes no `getObjectSchema` and one whose definition read rejects — both still
query, unexpanded.

Unlike the two sibling conversions, the metadata read is **not** disabled for a
timeline whose items were authored inline: this component also reads the
definition's fields for option colours and field labels on that path, where no
record query is issued at all.
232 changes: 232 additions & 0 deletions packages/plugin-timeline/src/ObjectTimeline.fetchGate-7895.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#7895 — the timeline's DUPLICATE query is gated. It was the last
* member of the set objectui#6482 converged (`ObjectKanban`, `ObjectView`,
* `ObjectCalendar`, `ObjectTree`; `ObjectGantt` at objectui#7225 ask 2) still
* carrying the pre-gate shape, and nothing marked it a deliberate exclusion.
*
* Before this, `ObjectTimeline` held the object definition in a local
* `useState` and listed it in the record-fetch effect's dependency array, so a
* mount issued TWO `find` calls: one before the definition settled, with
* `buildExpandFields` seeing no fields and therefore NO `$expand` at all, and
* one after.
*
* ⛔ A GREEN RUN WITH NO HOLD IS ZERO EVIDENCE HERE. This container settles the
* metadata read and the record read together by default, so the defect is
* invisible unperturbed — that is the reading objectui#7466 paid for. The lever
* is the METADATA fetch, so `getObjectSchema` is HELD and the writes into an
* instrumented renderer are counted per SINGLE mount. Measured on this
* component with that instrument, `ObjectCalendar` and `ObjectGantt` as
* positive controls in the same vitest run, holds 0/1/2/3/4/5/6/7/8/9/10/15/
* 25/50/100 ms:
*
* component hold paints late writes first paint find calls
* ObjectTimeline before, +3ms and up 4 3 3-7ms 2
* ObjectTimeline before, +0 / +1ms 1 0 5ms 2
* ObjectTimeline after, every hold 1 0 tracks the hold 1
* ObjectCalendar before and after 1 0 tracks the hold 1
* ObjectGantt before and after 1 0 tracks the hold 1
*
* ⭐ "First paint tracks the hold" is the signature that the gate is LIVE, and
* it is what the last case below pins — not as a wall-clock threshold, which
* would be a timing test, but as an ORDER: the definition resolves, then the
* query goes out, then the renderer is written to, exactly once. Before the
* gate that order was find, paint, definition, find, paint. Measured after:
* 8ms at a +3ms hold, 19ms at +10, 30ms at +25, 105ms at +100, against a flat
* 3-7ms at every hold before it.
*
* ⚠️ The gate is only safe because the resolution now SETTLES ON EVERY EXIT
* (objectui#7232, via the shared `useSettledSchema`): the replaced effect
* returned without settling on `!dataSource`, on a missing `getObjectSchema`,
* on an absent object name and in its `catch`. That cost nothing while nothing
* waited on it and would hold a gated query open FOREVER. The two middle cases
* below are that trap, pinned: a timeline that never loads is the failure this
* file exists to make impossible.
*
* ⛔ GATING IS NOT EXPANDING. objectui#7429 is the separate, non-overlapping
* concern of whether the `$expand` set this query carries is FLS-gated; this
* file asserts only HOW MANY queries go out and whether the first one carries
* its expansion.
*/

import React from 'react';
import { render, screen, waitFor, act } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { ObjectTimeline } from './ObjectTimeline';

/** Every write into the renderer, in order — the paint counter. */
let paints: number[] = [];
/** Definition resolve / query / paint, interleaved in the order they happen. */
let sequence: string[] = [];

vi.mock('./renderer', () => ({
TimelineRenderer: ({ schema }: any) => {
const items = schema.items ?? [];
paints.push(items.length);
sequence.push('paint');
return <div data-testid="timeline-renderer" data-item-count={String(items.length)} />;
},
}));

/** A definition with a lookup, so a gated query has a real `$expand` to carry. */
const OBJECT_SCHEMA = {
name: 'duly_task',
fields: {
id: { name: 'id', type: 'text' },
subject: { name: 'subject', type: 'text' },
starts_at: { name: 'starts_at', type: 'datetime' },
owner: { name: 'owner', type: 'lookup', reference_to: 'user' },
},
};

const ROWS = [{ id: '1', subject: 'Ship it', owner: 'u1', starts_at: '2026-01-01T09:00:00Z' }];

const schema: any = {
type: 'timeline',
objectName: 'duly_task',
titleField: 'subject',
startDateField: 'starts_at',
};

function makeAdapter(getObjectSchema?: any) {
return {
find: vi.fn(async () => {
sequence.push('find');
return { data: ROWS, total: ROWS.length };
}),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
...(getObjectSchema === undefined ? {} : { getObjectSchema }),
} as any;
}

/** The expand sets of every issued query, in order. */
function expandSets(adapter: any): Array<unknown> {
return adapter.find.mock.calls.map(([, params]: [string, any]) => params?.$expand ?? null);
}

beforeEach(() => {
paints = [];
sequence = [];
});

describe('objectui#7895 — the timeline waits for the object definition instead of querying twice', () => {
it('issues ONE query per load, and it already carries the expansion', async () => {
const adapter = makeAdapter(vi.fn(async () => OBJECT_SCHEMA));

render(<ObjectTimeline schema={schema} dataSource={adapter} />);

await waitFor(() => expect(screen.getByTestId('timeline-renderer')).toBeTruthy());
await waitFor(() => expect(adapter.find).toHaveBeenCalledTimes(1));

// The old regime's signature was `[null, ['owner']]`. One expanded call.
expect(expandSets(adapter)).toEqual([['owner']]);
expect(adapter.getObjectSchema).toHaveBeenCalledTimes(1);
});

it('never issues an UNEXPANDED query for an object that declares a lookup', async () => {
const adapter = makeAdapter(vi.fn(async () => OBJECT_SCHEMA));

render(<ObjectTimeline schema={schema} dataSource={adapter} />);

await waitFor(() => expect(adapter.find).toHaveBeenCalled());
// The discarded round trip — and the raw-id frame it painted — is the thing
// gating removes. Not "fewer" unexpanded calls: none.
for (const params of adapter.find.mock.calls.map(([, p]: [string, any]) => p)) {
expect(params.$expand).toEqual(['owner']);
}
});

it('still queries when the adapter exposes NO `getObjectSchema` — the gate is on SETTLED, not on truthy', async () => {
// objectui#7232's trap: an exit that returns without settling would hold
// this query open forever, and the timeline would never load.
const adapter = makeAdapter(undefined);

render(<ObjectTimeline schema={schema} dataSource={adapter} />);

await waitFor(() => expect(screen.getByTestId('timeline-renderer')).toBeTruthy());
expect(adapter.find).toHaveBeenCalledTimes(1);
// Nothing to derive an expand set from, so the query is unexpanded — the
// same query this case produced before the gate.
expect(expandSets(adapter)).toEqual([null]);
});

it('still queries when the definition read REJECTS', async () => {
const adapter = makeAdapter(
vi.fn(async () => {
throw new Error('metadata endpoint down');
}),
);
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
render(<ObjectTimeline schema={schema} dataSource={adapter} />);

await waitFor(() => expect(screen.getByTestId('timeline-renderer')).toBeTruthy());
expect(adapter.find).toHaveBeenCalledTimes(1);
expect(expandSets(adapter)).toEqual([null]);
expect(String(error.mock.calls[0]?.[0] ?? '')).toContain('[useSettledSchema]');
} finally {
error.mockRestore();
}
});

it('an AUTHORED `items` timeline paints without issuing any record query', async () => {
const authored: any = {
...schema,
items: [{ title: 'Ship it', time: '2026-01-01T09:00:00Z' }],
};
const adapter = makeAdapter(vi.fn(async () => OBJECT_SCHEMA));

render(<ObjectTimeline schema={authored} dataSource={adapter} />);

await waitFor(() =>
expect(screen.getByTestId('timeline-renderer').getAttribute('data-item-count')).toBe('1'),
);
expect(adapter.find).not.toHaveBeenCalled();
// ⛔ The metadata read is NOT disabled on this path, unlike the sibling
// components' `hasInlineData ? undefined : dataSource`. `effectiveItems`
// reads `objectDef.fields` for option colours and field labels even when
// the items were authored, so taking the read away would be a second,
// unasked-for change riding on a fetch-sequencing fix.
expect(adapter.getObjectSchema).toHaveBeenCalledTimes(1);
});

it('under a HELD definition read: ONE paint, and it arrives AFTER the definition', async () => {
// ⛔ The hold is the whole instrument. Without it this container settles
// both reads together and the case passes on the unfixed component too.
const HOLD_MS = 25;
const adapter = makeAdapter(
vi.fn(async () => {
await new Promise((r) => setTimeout(r, HOLD_MS));
sequence.push('definition');
return OBJECT_SCHEMA;
}),
);

render(<ObjectTimeline schema={schema} dataSource={adapter} />);

await waitFor(() => expect(screen.getByTestId('timeline-renderer')).toBeTruthy());
// Leave the render MOUNTED and let anything still in flight land — the late
// writes are only observable on a render that is still there to receive
// them. Measured before the gate at this hold: 3 of them.
await act(async () => {
await new Promise((r) => setTimeout(r, HOLD_MS + 150));
});

expect(paints).toEqual([ROWS.length]);
// The ORDER is the signature, not a wall-clock threshold. Before the gate:
// find, paint, definition, find, paint (and two more writes after).
expect(sequence).toEqual(['definition', 'find', 'paint']);
expect(adapter.find).toHaveBeenCalledTimes(1);
expect(expandSets(adapter)).toEqual([['owner']]);
});
});
79 changes: 61 additions & 18 deletions packages/plugin-timeline/src/ObjectTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import React, { useEffect, useState, useCallback, useMemo } from 'react';
import type { DataSource, TimelineSchema, ListViewTimelineConfig } from '@object-ui/types';
import { useDataScope, useNavigationOverlay, useSafeFieldLabel } from '@object-ui/react';
import { useDataScope, useNavigationOverlay, useSafeFieldLabel, useSettledSchema } from '@object-ui/react';
import { NavigationOverlay } from '@object-ui/components';
import { extractRecords, buildExpandFields, convertSortToQueryParams, createFieldColorResolver } from '@object-ui/core';
import { usePullToRefresh } from '@object-ui/mobile';
Expand Down Expand Up @@ -173,7 +173,6 @@ export const ObjectTimeline: React.FC<ObjectTimelineProps> = ({
});
const [error, setError] = useState<Error | null>(null);
const [refreshKey, setRefreshKey] = useState(0);
const [objectDef, setObjectDef] = useState<any>(null);

// Resolve nested TimelineConfig (spec-compliant)
const timelineConfig = schema.timeline;
Expand All @@ -187,21 +186,50 @@ export const ObjectTimeline: React.FC<ObjectTimelineProps> = ({

const boundData = useDataScope(schema.bind);

// Fetch object definition for metadata
useEffect(() => {
let isMounted = true;
const fetchMeta = async () => {
if (!dataSource || typeof dataSource.getObjectSchema !== 'function' || !schema.objectName) return;
try {
const def = await dataSource.getObjectSchema(schema.objectName);
if (isMounted) setObjectDef(def);
} catch (e) {
console.warn('Failed to fetch object def for ObjectTimeline', e);
}
};
fetchMeta();
return () => { isMounted = false; };
}, [schema.objectName, dataSource]);
/**
* The object definition, and whether the read for THIS object has SETTLED —
* one piece of state, through the shared hook (objectui#7895).
*
* `ObjectTimeline` was the last member of the converged set still carrying
* the pre-gate shape: a local `useState` fed by its own metadata effect,
* with `objectDef` listed in the record-fetch effect's dependency array
* below. That shape issues the record query TWICE per mount — once before
* the definition lands, with `buildExpandFields` seeing no fields and so no
* `$expand` at all, and once after — and whenever the metadata read is the
* slower of the two the user sees the three-step paint `ObjectGantt`'s own
* conversion names: raw foreign-key ids, back to the loading skeleton (the
* re-run calls `setLoading(true)` and `loading` is an early return below),
* then the expanded rows. Measured on this component before the change,
* instrumented renderer, one mount per hold: 2 `find` calls with expand
* sets `[null, ['owner']]`, 1 paint at the readiness predicate and 3 late
* writes after it, at every hold from +3ms up.
*
* ⚠️ The gate below is only safe because this resolution SETTLES ON EVERY
* EXIT (objectui#7232) — no source, no `getObjectSchema`, no object name,
* and a read that threw alike. The hand-written effect it replaces returned
* WITHOUT settling on all four, which cost nothing while nothing waited on
* it and would hold a gated query open forever.
*
* ⛔ `dataSource` is passed unconditionally, NOT `hasInlineData ? undefined
* : dataSource` the way `ObjectCalendar` and `ObjectGantt` pass it. Those
* two read metadata only to expand a record query, so an inline data set has
* nothing to wait for. This component also reads `objectDef.fields` in
* `effectiveItems` below — option colours and field labels — on the AUTHORED
* items path, where no query is issued at all. Their recipe would stop that
* read happening; the conversion is a fetch-sequencing change and must not
* take a metadata read away from a path that still uses it.
*
* The key is `schema.objectName`, which is the object the record query
* itself names (`dataSource.find(schema.objectName, …)` below) and the one
* the replaced effect read. ⛔ Not `resolveRecordSourceObjectName`: this
* component has no resolved `data` block to read a second name from, and
* gating on a key the query does not use is exactly the stale-key mismatch
* `useSettledSchema`'s render-time comparison exists to make unrepresentable.
*/
const { ready: objectDefReady, def: objectDef } = useSettledSchema<any>(
schema.objectName ?? '',
dataSource,
);

// Content keys, not identities. `filter` / `sort` are fetch inputs from here on
// (objectstack#7137), and an inline array on a schema node is a NEW object every
Expand Down Expand Up @@ -245,13 +273,28 @@ export const ObjectTimeline: React.FC<ObjectTimelineProps> = ({
};

if (schema.objectName && !boundData && !schema.items && !(props as any).data) {
// ⭐ objectui#7895 — the object definition GATES this query; it does not
// refine it afterwards. `objectDef` stays in the dependency list below
// and the two are ONE mechanism, not two: the dependency is what makes
// this effect re-run when the definition lands, and this line is what
// stops the first run from spending a query before it has. Removing
// either half alone restores the double fetch — the reverse
// verification `ObjectGantt.fetchGate-7225.test.tsx` recorded on the
// sibling, re-measured here.
//
// Scoped to the branch that actually issues the query. The `else` below
// has authored or bound items and never queries, so gating it would
// hold nothing useful — and this component still reads the definition
// on that path (option colours in `effectiveItems`), which is why the
// resolution above is not disabled for it.
if (!objectDefReady) return;
fetchData();
} else {
// Have inline / bound items — won't fetch; clear loading.
setLoading(false);
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- `schema.filter`/`schema.sort` are tracked by CONTENT (filterKey/sortKey) on purpose; see above
}, [schema.objectName, dataSource, boundData, schema.items, (props as any).data, refreshKey, objectDef, filterKey, sortKey, schema.limit]);
}, [schema.objectName, dataSource, boundData, schema.items, (props as any).data, refreshKey, objectDefReady, objectDef, filterKey, sortKey, schema.limit]);

const rawData = (props as any).data || boundData || fetchedData;
const { t } = useTimelineTranslation();
Expand Down
Loading