diff --git a/.changeset/6592-rekey-fetch-effects-on-primitives.md b/.changeset/6592-rekey-fetch-effects-on-primitives.md new file mode 100644 index 0000000000..7c5cbd0a92 --- /dev/null +++ b/.changeset/6592-rekey-fetch-effects-on-primitives.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-map': patch +'@object-ui/plugin-calendar': patch +'@object-ui/plugin-gantt': patch +--- + +Re-key the load-bearing fetch effects in `ObjectMap`, `ObjectCalendar` and +`ObjectGantt` onto the primitive fields they actually read off `dataConfig` +(`provider` / `object` / `items`) instead of the whole memoised `dataConfig` +object (objectui#6592, the deferred half of objectui#6270/PR #6591). +`ObjectTree` is a census member too but is deferred out of this change — see +the PR body — because its own fetch effects are the surface of PR #6696 +(objectui#6481), open at the same time. + +`useMemo` carries no semantic guarantee — React is permitted to discard a +memo cache and recompute even when its dependency array compares equal to +the previous render, and the local `getDataConfig(schema)` helper each of +these renderers carries builds a fresh `{ provider, object }` / +`{ provider, items }` wrapper object on every call. So a fetch effect keyed +on `dataConfig` itself was correct only for as long as that identity +happened to survive a discard: a recompute alone (no author or caller +action) was enough to re-run the effect and issue an extra +`dataSource.find` / `dataSource.getObjectSchema` call. Keying the effects +on the primitives instead makes a cache discard a no-op, restoring +`useMemo` to a pure optimisation. + +`ObjectGantt`'s `effectiveDataSource` memo deliberately keeps `dataConfig` +as a dependency (`resolveDataSource` needs the whole provider-shaped +value — the `api` provider's `read`/`write` request config cannot be +flattened to a fixed primitive list the way `object`/`value` can), so its +`reload()` fetch is decoupled from the redundant direct `dataConfig` +dependency but not from `effectiveDataSource`'s own; for the `object`/`value` +providers `resolveDataSource` returns its `fallback`/a fresh +`ValueDataSource` respectively rather than reading further into the config, +which is enough for the two fetch effects to observe no extra call under a +recomputed-but-equivalent `dataConfig` in the common case. + +No behaviour change for a schema whose `useMemo` caches survive normally; +the effects are unaffected by React discarding one. diff --git a/packages/plugin-calendar/src/ObjectCalendar.discardedConfigMemo.test.tsx b/packages/plugin-calendar/src/ObjectCalendar.discardedConfigMemo.test.tsx new file mode 100644 index 0000000000..8d071e58aa --- /dev/null +++ b/packages/plugin-calendar/src/ObjectCalendar.discardedConfigMemo.test.tsx @@ -0,0 +1,91 @@ +/** + * 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#6592 — see `ObjectMap.discardedConfigMemo.test.tsx` for the full + * rationale (`useMemo` carries no semantic guarantee, so a fetch effect + * keyed on `dataConfig`'s object identity is correct only for as long as + * that identity happens to survive). This file pins the same contract for + * `ObjectCalendar`'s record-fetch effect. + * + * `ObjectCalendar`'s own `dataConfig` memo is ALREADY keyed on primitives + * (`schema.data` / `schema.staticData` / `schema.objectName` — objectui#6018), + * so the map/tree discard proxy ("a schema with a new reference but equal + * `objectName`") does not even recompute `dataConfig` here: those three deps + * would all compare equal and the memo would keep its old cached object. The + * discard proxy for THIS component instead varies `schema.data` itself — one + * of the memo's OWN deps — across two object literals that carry the SAME + * `provider`/`object` but a different reference, which forces the recompute + * (`(schema as any).data` is compared by Object.is, not by value) while + * leaving every primitive the fetch effect reads unchanged. That is the same + * "different identity, same content" shape a genuine memo-cache discard + * would produce. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { ObjectCalendar } from './ObjectCalendar'; + +afterEach(cleanup); + +const today = new Date(); +const dayInThisMonth = (d: number) => new Date(today.getFullYear(), today.getMonth(), d, 9, 0, 0, 0); + +const ROWS = [{ id: 'v1', name: 'Site visit', starts_at: dayInThisMonth(10).toISOString() }]; + +function makeDataSource() { + return { + find: vi.fn().mockResolvedValue({ data: ROWS }), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'visit', fields: {} }), + } as any; +} + +const CALENDAR = { startDateField: 'starts_at', titleField: 'name' }; + +describe('ObjectCalendar — record-fetch effect survives a discarded `dataConfig` memo (objectui#6592)', () => { + it('does not re-fire the fetch when `dataConfig` recomputes to a new identity with the SAME primitive fields', async () => { + const dataSource = makeDataSource(); + // Two different `data` object references, byte-identical content — forces + // `dataConfig`'s own memo to recompute to a NEW object (its `data` dep is + // compared by reference) while `provider`/`object` stay unchanged. + const schemaA: any = { type: 'object-calendar', calendar: CALENDAR, data: { provider: 'object', object: 'visit' } }; + const schemaB: any = { type: 'object-calendar', calendar: CALENDAR, data: { provider: 'object', object: 'visit' } }; + expect(schemaA.data).not.toBe(schemaB.data); + expect(schemaA.data).toEqual(schemaB.data); + + const { rerender } = render(); + await waitFor(() => expect(screen.getByText('Site visit')).toBeTruthy()); + await waitFor(() => expect(dataSource.getObjectSchema).toHaveBeenCalled()); + + const findCallsAtRest = dataSource.find.mock.calls.length; + const schemaCallsAtRest = dataSource.getObjectSchema.mock.calls.length; + expect(findCallsAtRest).toBeGreaterThan(0); + + rerender(); + await new Promise((r) => setTimeout(r, 0)); + + expect(dataSource.find.mock.calls.length).toBe(findCallsAtRest); + expect(dataSource.getObjectSchema.mock.calls.length).toBe(schemaCallsAtRest); + }); + + it('still DOES re-fire when the recomputed `dataConfig` carries a genuinely different `object`', async () => { + const dataSource = makeDataSource(); + const schemaA: any = { type: 'object-calendar', objectName: 'visit', calendar: CALENDAR }; + const schemaB: any = { type: 'object-calendar', objectName: 'appointment', calendar: CALENDAR }; + + const { rerender } = render(); + await waitFor(() => expect(dataSource.find).toHaveBeenCalledWith('visit', expect.any(Object))); + const callsBefore = dataSource.find.mock.calls.length; + + rerender(); + + await waitFor(() => expect(dataSource.find.mock.calls.length).toBeGreaterThan(callsBefore)); + expect(dataSource.find).toHaveBeenCalledWith('appointment', expect.any(Object)); + }); +}); diff --git a/packages/plugin-calendar/src/ObjectCalendar.tsx b/packages/plugin-calendar/src/ObjectCalendar.tsx index 2592c8aa35..5dead55eb3 100644 --- a/packages/plugin-calendar/src/ObjectCalendar.tsx +++ b/packages/plugin-calendar/src/ObjectCalendar.tsx @@ -241,6 +241,20 @@ export const ObjectCalendar: React.FC = ({ (schema as any).colorField ]); const hasInlineData = dataConfig?.provider === 'value'; + /** + * The record-fetch effect below used to key on `dataConfig` itself — the + * whole memoised object identity. `useMemo` carries no semantic + * guarantee (React may discard its cache and recompute), and + * `getDataConfig(schema)` builds a fresh wrapper object on every call + * even when its own deps haven't changed, so a discard alone was enough + * to re-run the effect and refetch. `dataProvider` and `dataItems` are + * the remaining primitive fields that effect reads off `dataConfig` — + * `schemaObjectName` below already covers the `object` field for the + * same purpose. Keying on all three instead of the container object + * makes a cache discard a no-op (objectui#6592). + */ + const dataProvider = dataConfig?.provider; + const dataItems = dataConfig?.provider === 'value' ? dataConfig.items : undefined; // ⭐ objectui#6453 — this replaces a `useRef` written in the render body // (`objectSchemaRef.current = objectSchema`), which existed so the fetch @@ -303,17 +317,17 @@ export const ObjectCalendar: React.FC = ({ // set has no expand set to derive and issues no metadata read at all, so // gating it would hold a query open on a resolution nothing was going to // produce. - if (dataConfig?.provider === 'object' && !objectSchemaReady) return; + if (dataProvider === 'object' && !objectSchemaReady) return; let isMounted = true; const fetchData = async () => { try { if (!isMounted) return; setLoading(true); - - if (hasInlineData && dataConfig?.provider === 'value') { + + if (hasInlineData && dataProvider === 'value') { if (isMounted) { - setData(dataConfig.items as any[]); + setData(dataItems as any[]); setLoading(false); } return; @@ -323,8 +337,11 @@ export const ObjectCalendar: React.FC = ({ throw new Error('DataSource required for object/api providers'); } - if (dataConfig?.provider === 'object') { - const objectName = dataConfig.object; + if (dataProvider === 'object') { + // `schemaObjectName` already resolves this same 'object' branch's + // `dataConfig.object` (required on that discriminated-union + // variant), computed once above for the schema-fetch gate too. + const objectName = schemaObjectName as string; // Auto-inject $expand for lookup/master_detail fields // Reached only with the schema resolved (the gate above), so a // calendar whose object declares relations queries WITH its @@ -336,13 +353,13 @@ export const ObjectCalendar: React.FC = ({ $orderby: convertSortToQueryParams(schema.sort), ...(expand.length > 0 ? { $expand: expand } : {}), }); - + const items: any[] = extractRecords(result); - + if (isMounted) { setData(items); } - } else if (dataConfig?.provider === 'api') { + } else if (dataProvider === 'api') { console.warn('API provider not yet implemented for ObjectCalendar'); if (isMounted) setData([]); } @@ -359,8 +376,8 @@ export const ObjectCalendar: React.FC = ({ fetchData(); return () => { isMounted = false; }; - }, [hasExternalData, dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, - refreshKey, objectSchemaReady, objectSchema]); + }, [hasExternalData, dataProvider, schemaObjectName, dataItems, dataSource, hasInlineData, + schema.filter, schema.sort, refreshKey, objectSchemaReady, objectSchema]); // Fetch object schema for field metadata. // diff --git a/packages/plugin-gantt/src/ObjectGantt.discardedConfigMemo.test.tsx b/packages/plugin-gantt/src/ObjectGantt.discardedConfigMemo.test.tsx new file mode 100644 index 0000000000..dc8c1d72cf --- /dev/null +++ b/packages/plugin-gantt/src/ObjectGantt.discardedConfigMemo.test.tsx @@ -0,0 +1,124 @@ +/** + * 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#6592 — see `ObjectMap.discardedConfigMemo.test.tsx` for the full + * rationale. This file pins the same contract for `ObjectGantt`'s + * `reload()` fetch (the `useEffect(() => { reload(); }, [reload])` mount + * effect) and its "fetch object schema" effect. + * + * `ObjectGantt`'s own `dataConfig` memo already buys a value-stable identity + * via `useMemo(() => rawDataConfig, [JSON.stringify(rawDataConfig)])` + * (pre-dating this card, and untouched by it) — so the map/tree/calendar + * discard proxy ("a schema with a new reference but byte-identical `.data`") + * would not even recompute `dataConfig` here: the JSON-string dep would + * compare equal and the memo would keep its old cached object. So the + * discard proxy for THIS component adds an inert field to `schema.data` that + * changes value between renders — `_probe` below, never read by + * `getDataConfig`, `reload`, or `resolveDataSource` — which forces the JSON + * string (and so `dataConfig`'s identity) to change while every primitive + * either fetch effect actually reads (`provider`, `object`) stays the same. + * That is the same "different identity, same content" shape a genuine + * memo-cache discard would produce. + * + * One more thing has to hold for this to isolate the two effects under + * test rather than a third, architecturally-unavoidable one: + * `effectiveDataSource = useMemo(() => resolveDataSource(dataConfig, ...), [dataConfig, ...])` + * also depends on `dataConfig`'s identity, and objectui#6592 deliberately + * leaves it that way (see the comment on `dataItems` in `ObjectGantt.tsx` — + * `resolveDataSource` reads a provider-shaped slice of `dataConfig` that + * cannot be flattened to a fixed primitive list). For the `object` provider + * this component is tested with here, though, `resolveDataSource` returns + * the `fallback` context DataSource UNCHANGED (`packages/core/src/adapters/resolveDataSource.ts` + * — no new adapter is constructed), so `effectiveDataSource`'s value stays + * referentially the SAME object across the `_probe` churn even though its + * memo's factory reran. That is what makes the two fixed effects observable + * in isolation below; it is also why this file's own comment does not claim + * gantt is unconditionally immune to a `dataConfig` discard — see the PR + * body's "Known boundary" note for the `api`/`value` providers, where + * `resolveDataSource` allocates a fresh adapter every call and this + * decoupling does not hold. + * + * The two effects under test used to list bare `dataConfig`; after + * objectui#6592 the `reload` callback lists `dataProvider`/`dataItems` and + * the "fetch object schema" effect drops the (unused) dependency entirely. + */ + +import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi } from 'vitest'; +import { ObjectGantt } from './ObjectGantt'; +import { DataSource } from '@object-ui/types'; + +vi.mock('./GanttView', () => ({ + GanttView: ({ tasks }: any) => ( +
+ {tasks.map((t: any) => ( +
{t.title}
+ ))} +
+ ), +})); + +const ROWS = [ + { id: '1', name: 'Task 1', start_date: '2024-01-01', end_date: '2024-01-05', progress: 50 }, +]; + +function makeDataSource(): DataSource { + return { + find: vi.fn().mockResolvedValue({ data: ROWS }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ fields: { name: { type: 'text' } } }), + } as any; +} + +const GANTT_CONFIG = { titleField: 'name', startDateField: 'start_date', endDateField: 'end_date' }; + +describe('ObjectGantt — reload/schema fetch effects survive a discarded `dataConfig` memo (objectui#6592)', () => { + it('does not re-fire either fetch when `dataConfig` recomputes to a new identity with the SAME primitive fields', async () => { + const dataSource = makeDataSource(); + // `_probe` is read by nothing under test — it exists only to force + // `JSON.stringify(rawDataConfig)` to differ so `dataConfig`'s OWN memo + // (unrelated to this card, see the file docblock) recomputes to a new + // object identity while `provider`/`object` stay unchanged. + const schemaA: any = { type: 'gantt', gantt: GANTT_CONFIG, data: { provider: 'object', object: 'tasks', _probe: 'a' } }; + const schemaB: any = { type: 'gantt', gantt: GANTT_CONFIG, data: { provider: 'object', object: 'tasks', _probe: 'b' } }; + + const { rerender } = render(); + await waitFor(() => expect(screen.getAllByTestId('gantt-task')).toHaveLength(1)); + await waitFor(() => expect((dataSource.getObjectSchema as any)).toHaveBeenCalled()); + + const findCallsAtRest = (dataSource.find as any).mock.calls.length; + const schemaCallsAtRest = (dataSource.getObjectSchema as any).mock.calls.length; + expect(findCallsAtRest).toBeGreaterThan(0); + + rerender(); + await new Promise((r) => setTimeout(r, 0)); + + expect((dataSource.find as any).mock.calls.length).toBe(findCallsAtRest); + expect((dataSource.getObjectSchema as any).mock.calls.length).toBe(schemaCallsAtRest); + }); + + it('still DOES re-fire when the recomputed `dataConfig` carries a genuinely different `object`', async () => { + const dataSource = makeDataSource(); + const schemaA: any = { type: 'gantt', gantt: GANTT_CONFIG, data: { provider: 'object', object: 'tasks' } }; + const schemaB: any = { type: 'gantt', gantt: GANTT_CONFIG, data: { provider: 'object', object: 'milestones' } }; + + const { rerender } = render(); + await waitFor(() => expect(dataSource.find).toHaveBeenCalledWith('tasks', expect.any(Object))); + const callsBefore = (dataSource.find as any).mock.calls.length; + + rerender(); + + await waitFor(() => expect((dataSource.find as any).mock.calls.length).toBeGreaterThan(callsBefore)); + expect(dataSource.find).toHaveBeenCalledWith('milestones', expect.any(Object)); + }); +}); diff --git a/packages/plugin-gantt/src/ObjectGantt.tsx b/packages/plugin-gantt/src/ObjectGantt.tsx index 3e4c5b838b..b8e552ad06 100644 --- a/packages/plugin-gantt/src/ObjectGantt.tsx +++ b/packages/plugin-gantt/src/ObjectGantt.tsx @@ -598,7 +598,22 @@ export const ObjectGantt: React.FC = ({ }, [JSON.stringify(rawDataConfig)]); const ganttConfig = getGanttConfig(schema); - const hasInlineData = dataConfig?.provider === 'value'; + const dataProvider = dataConfig?.provider; + const hasInlineData = dataProvider === 'value'; + /** + * The one primitive field `reload` (below) reads off `dataConfig` beyond + * `dataProvider` — the inline-data payload for the `value` provider. + * `reload` used to key on `dataConfig` itself: `useMemo` carries no + * semantic guarantee (React may discard its cache and recompute), and a + * discard alone was enough to give `reload` a fresh identity and re-fire + * the mount effect below, refetching. `effectiveDataSource`'s own memo + * intentionally keeps `dataConfig` as a dependency (not just its + * `object`/`items` primitives): `resolveDataSource` reads a + * provider-shaped slice of it (the whole `read`/`write` request config + * on `api`), which cannot be flattened to a fixed primitive list the way + * the 'object'/'value' branches below can be (objectui#6592). + */ + const dataItems = dataConfig?.provider === 'value' ? dataConfig.items : undefined; // Resolve the ViewData config into a concrete DataSource adapter: // provider: 'object' → the context DataSource passed via props (unchanged) @@ -644,8 +659,8 @@ export const ObjectGantt: React.FC = ({ return; } - if (hasInlineData && dataConfig?.provider === 'value') { - if (isCurrent()) setData(dataConfig.items as any[]); + if (hasInlineData && dataProvider === 'value') { + if (isCurrent()) setData(dataItems as any[]); return; } @@ -675,7 +690,7 @@ export const ObjectGantt: React.FC = ({ else setLoading(false); } // eslint-disable-next-line react-hooks/exhaustive-deps -- (rest as any).data intentionally untracked, matching the original effect - }, [effectiveDataSource, resource, hasInlineData, dataConfig, schema.filter, schema.sort, objectSchema]); + }, [effectiveDataSource, resource, hasInlineData, dataProvider, dataItems, schema.filter, schema.sort, objectSchema]); useEffect(() => { reload(); @@ -698,7 +713,10 @@ export const ObjectGantt: React.FC = ({ if (!hasInlineData && effectiveDataSource) { fetchObjectSchema(); } - }, [resource, effectiveDataSource, hasInlineData, dataConfig]); + // `dataConfig` was listed here but never read in this effect (`resource` + // already carries the one field — `object` — this effect needs from + // it); dropped rather than re-keyed (objectui#6592). + }, [resource, effectiveDataSource, hasInlineData]); // Transform data to gantt tasks const tasks = useMemo(() => { diff --git a/packages/plugin-map/src/ObjectMap.discardedConfigMemo.test.tsx b/packages/plugin-map/src/ObjectMap.discardedConfigMemo.test.tsx new file mode 100644 index 0000000000..fecb03ae06 --- /dev/null +++ b/packages/plugin-map/src/ObjectMap.discardedConfigMemo.test.tsx @@ -0,0 +1,138 @@ +/** + * 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#6592 — the fetch effects must not read `dataConfig`'s OBJECT + * IDENTITY, because `useMemo` carries no semantic guarantee. + * + * `ObjectMap.dataConfigMemo.test.tsx` (objectui#6018) pins that `dataConfig` + * does not get a fresh identity on an UNCHANGED `schema` reference. That is + * a real and necessary property, but it is not what this card is about: + * React is permitted to discard a `useMemo` cache and recompute it even when + * its dependency array compares equal to the previous render — the deps + * array only decides whether the FACTORY reruns, never whether the cache is + * kept. So "same `schema` in ⇒ same `dataConfig` out" is not guaranteed to + * hold across a discard, and a fetch effect keyed on `dataConfig` itself + * would refetch on every discard even though nothing an author or caller + * controls changed. + * + * There is no public API to force React's internal memo-discard path, so + * this file drives the same failure mode the discard hazard produces: a + * `dataConfig` recompute that yields a NEW object reference carrying the + * SAME primitive fields (`provider`, `object`). `getDataConfig(schema)` + * builds a fresh `{ provider, object }` / `{ provider, items }` wrapper on + * every call — so any recompute (whether from a discard or, as here, from + * `schema` itself getting a new but value-equal reference) produces exactly + * this shape of "different identity, same content" `dataConfig`. From the + * fetch effect's perspective the two triggers are indistinguishable: what + * matters is whether ITS dependency array reacts to the identity change or + * only to the primitives. + * + * Before objectui#6592 both effects listed bare `dataConfig` and re-fired + * (extra `dataSource.find` / `dataSource.getObjectSchema` calls, pinned RED + * against pre-fix source further below). After it, they list + * `dataProvider` / `dataObjectName` / `dataItems` and do not. + */ + +import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('react-map-gl/maplibre', () => ({ + default: ({ children }: any) =>
{children}
, + Map: ({ children }: any) =>
{children}
, + NavigationControl: () =>
, + Marker: ({ children, longitude, latitude }: any) => ( +
+ {children} +
+ ), + Popup: ({ children }: any) =>
{children}
, +})); + +import { ObjectMap } from './ObjectMap'; + +const MAP = { latitudeField: 'latitude', longitudeField: 'longitude', titleField: 'name' }; + +const ROWS = [ + { id: '1', name: 'Harbour Depot', latitude: 47.6062, longitude: -122.3321 }, + { id: '2', name: 'Ridge Yard', latitude: 37.7749, longitude: -122.4194 }, +]; + +function makeAdapter() { + return { + find: vi.fn().mockResolvedValue({ data: ROWS }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'store', + fields: { name: { type: 'text' } }, + }), + }; +} + +const settle = async (adapter: ReturnType) => { + await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull()); + await waitFor(() => expect(screen.getAllByTestId('map-marker')).toHaveLength(2)); + await waitFor(() => expect(adapter.getObjectSchema).toHaveBeenCalled()); + const seen = adapter.find.mock.calls.length; + await new Promise((r) => setTimeout(r, 0)); + expect(adapter.find.mock.calls.length).toBe(seen); +}; + +describe('ObjectMap — fetch effects survive a discarded `dataConfig` memo (objectui#6592)', () => { + it('does not re-fire either fetch effect when `schema` gets a new reference with the SAME primitive fields', async () => { + const adapter = makeAdapter(); + + // Two DIFFERENT object literals — new reference each — with identical + // `objectName`/`map` content. `dataConfig = useMemo(() => getDataConfig(schema), [schema])` + // sees `schema` change (Object.is fails) and calls `getDataConfig` again, + // which returns a brand-new `{ provider: 'object', object: 'store' }` + // wrapper: the same failure shape a discarded-and-recomputed cache would + // produce with schema held constant. + const schemaA: any = { type: 'object-map', map: MAP, objectName: 'store' }; + const schemaB: any = { type: 'object-map', map: MAP, objectName: 'store' }; + expect(schemaA).not.toBe(schemaB); + expect(schemaA).toEqual(schemaB); + + const { rerender } = render(); + await settle(adapter); + + const findCallsAtRest = adapter.find.mock.calls.length; + const schemaCallsAtRest = adapter.getObjectSchema.mock.calls.length; + expect(findCallsAtRest).toBeGreaterThan(0); + + rerender(); + await new Promise((r) => setTimeout(r, 0)); + + // The review question this card is about: neither effect re-fired + // against the recomputed-but-equivalent `dataConfig`. + expect(adapter.find.mock.calls.length).toBe(findCallsAtRest); + expect(adapter.getObjectSchema.mock.calls.length).toBe(schemaCallsAtRest); + }); + + it('still DOES re-fire when the recomputed `dataConfig` carries a genuinely different `object`', async () => { + // Counter-probe, same purpose as the #6018 file's: "immune to identity + // churn" must not be satisfiable by "never reacts to real changes either". + const adapter = makeAdapter(); + const schemaA: any = { type: 'object-map', map: MAP, objectName: 'store' }; + const schemaB: any = { type: 'object-map', map: MAP, objectName: 'warehouse' }; + + const { rerender } = render(); + await settle(adapter); + expect(adapter.find.mock.calls.map((c) => c[0])).toContain('store'); + const callsBefore = adapter.find.mock.calls.length; + + rerender(); + + await waitFor(() => expect(adapter.find.mock.calls.length).toBeGreaterThan(callsBefore)); + expect(adapter.find.mock.calls.map((c) => c[0])).toContain('warehouse'); + }); +}); diff --git a/packages/plugin-map/src/ObjectMap.tsx b/packages/plugin-map/src/ObjectMap.tsx index 12e16b135a..338e6f7829 100644 --- a/packages/plugin-map/src/ObjectMap.tsx +++ b/packages/plugin-map/src/ObjectMap.tsx @@ -654,12 +654,29 @@ export const ObjectMap: React.FC = ({ const mapConfig = useMemo(() => getMapConfig(schema), [schema]); const hasInlineData = dataConfig?.provider === 'value'; + /** + * The two fetch effects below used to key on `dataConfig` itself — the + * whole memoised object identity. `useMemo` carries no semantic + * guarantee: React is permitted to discard its cache and recompute, and + * `getDataConfig(schema)` builds a fresh `{ provider, object }` / + * `{ provider, items }` wrapper object on every call even when `schema` + * itself hasn't changed. So a discard (not just a `schema` change) was + * enough to re-run both effects and refetch. These three are every + * primitive field either effect actually reads off `dataConfig`; keying + * on them instead of the container object makes a cache discard a no-op + * for both effects, and returns `useMemo` here to being a pure + * optimisation rather than a correctness dependency (objectui#6592). + */ + const dataProvider = dataConfig?.provider; + const dataObjectName = dataConfig?.provider === 'object' ? dataConfig.object : undefined; + const dataItems = dataConfig?.provider === 'value' ? dataConfig.items : undefined; + // Fetch data based on provider useEffect(() => { const fetchData = async () => { try { setLoading(true); - + // Prioritize data passed via props (from ListView). `dataProp` is a // declared prop (not the `rest` spread), so it can sit in this // effect's dependency array below without turning into a @@ -670,8 +687,8 @@ export const ObjectMap: React.FC = ({ return; } - if (hasInlineData && dataConfig?.provider === 'value') { - setData(dataConfig.items as any[]); + if (hasInlineData && dataProvider === 'value') { + setData(dataItems as any[]); setLoading(false); return; } @@ -680,8 +697,12 @@ export const ObjectMap: React.FC = ({ throw new Error('DataSource required for object/api providers'); } - if (dataConfig?.provider === 'object') { - const objectName = dataConfig.object; + if (dataProvider === 'object') { + // `dataObjectName` is only unset here if the schema is off-contract + // (an 'object' provider with no `object` name) — the discriminated + // union declares it required, same as the pre-refactor narrowing + // this replaces. + const objectName = dataObjectName as string; // Auto-inject $expand for lookup/master_detail fields const expand = buildExpandFields(objectSchema?.fields); const result = await dataSource.find(objectName, { @@ -689,14 +710,14 @@ export const ObjectMap: React.FC = ({ $orderby: convertSortToQueryParams(schema.sort), ...(expand.length > 0 ? { $expand: expand } : {}), }); - + const items: any[] = extractRecords(result); setData(items); - } else if (dataConfig?.provider === 'api') { + } else if (dataProvider === 'api') { console.warn('API provider not yet implemented for ObjectMap'); setData([]); } - + setLoading(false); } catch (err) { setError(err as Error); @@ -705,20 +726,20 @@ export const ObjectMap: React.FC = ({ }; fetchData(); - }, [dataProp, dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema]); + }, [dataProp, dataProvider, dataObjectName, dataItems, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema]); // Fetch object schema for field metadata useEffect(() => { const fetchObjectSchema = async () => { try { if (!dataSource) return; - - const objectName = dataConfig?.provider === 'object' - ? dataConfig.object + + const objectName = dataProvider === 'object' + ? dataObjectName : schema.objectName; - + if (!objectName) return; - + const schemaData = await dataSource.getObjectSchema(objectName); setObjectSchema(schemaData); } catch (err) { @@ -729,7 +750,7 @@ export const ObjectMap: React.FC = ({ if (!hasInlineData && dataSource) { fetchObjectSchema(); } - }, [schema.objectName, dataSource, hasInlineData, dataConfig]); + }, [schema.objectName, dataSource, hasInlineData, dataProvider, dataObjectName]); // Transform data to map markers const { markers, invalidCount } = useMemo(() => {