fix(plugin-map): thread props.data and seed clustering zoom from the applied camera

.changeset/objectmap-data-prop-and-zoom-seed-5003.md

@@ -0,0 +1,26 @@
+---
+"@object-ui/plugin-map": patch
+---
+
+Fix two pieces of `ObjectMap` state that stopped tracking their source after mount
+(objectui#5003):
+
+- **`data` prop threading**: the fetch effect preferred records passed via `props.data`,
+ but `data` was read off the `rest` spread and was not one of the effect's dependencies.
+ A host rendering `<ObjectMap data={[]} .../>` while its own query is in flight, then
+ re-rendering with the resolved rows, kept showing the empty map — the prop changed, the
+ effect never re-ran to notice. `data` is now a declared prop (`data: dataProp`), tracked
+ directly in the effect's dependency array; the whole `rest` object is intentionally
+ **not** added there (it is a fresh object every render, which would refetch on every
+ render instead).
+- **Clustering zoom seed**: `currentZoom` — what `clusterMarkers` uses for its grid cell
+ size — was seeded with a nominal `mapConfig.zoom || 3` and updated only by `onZoom`.
+ MapLibre applies the initial camera (including a `bounds` fit) via its constructor,
+ before react-map-gl attaches React's event handlers, so no `onZoom` ever fired for that
+ first camera — the seed stayed nominal until the user's first zoom. It is now also
+ seeded from `onLoad`, which fires once the initial camera has settled, so clustering at
+ first paint reflects the camera MapLibre actually applied.
+
+Both were dormant on the console path (never exercised in the example apps) — see the
+issue for why — so this ships with dedicated tests exercising the prop-update path and
+the pre-`onZoom` clustering state directly.

packages/plugin-map/README.md

@@ -147,6 +147,7 @@ import { ObjectMap } from '@object-ui/plugin-map';
 | `schema` | The map schema — the keys above. |
 | `dataSource` | Resolves the `object` provider. Not needed for `staticData` or an inline `data` array. |
 | `className` | Classes for the wrapper around the map. |
+| `data` | Records to render directly, bypassing the component's own fetch — the shape `ListView` passes when it already holds the rows. Tracked live: passing a new array after mount (e.g. once a host's own in-flight query resolves) updates the map. |
 | `onMarkerClick` | Called with the clicked record. |
 | `onRowClick` | Record click handler; takes priority over the `navigation` overlay. |
 | `onEdit` / `onDelete` | Passing either adds that button to the marker popup (and to the mobile record sheet). |

packages/plugin-map/src/ObjectMap.dataProp.test.tsx

@@ -0,0 +1,86 @@
+/**
+ * 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.
+ *
+ * Regression (objectui#5003): `props.data` was read by the fetch effect (via
+ * the `rest` spread) but was not one of its dependencies. A host rendering
+ * `<ObjectMap data={[]} .../>` while its own query is in flight, then passing
+ * the resolved rows, saw the effect's copy of `data` stay empty forever — the
+ * prop changed, but the effect never re-ran to notice.
+ *
+ * This is UNREACHABLE through the console path: `ElementDataSourceGate` (see
+ * `ObjectMap.elementDataSource.test.tsx`) always drives the map through its
+ * own `dataSource.find`, never through this prop. So the tests below mount
+ * with `data` directly — the only way to exercise the buggy dependency list.
+ */
+import React from 'react';
+import { render, screen, waitFor } from '@testing-library/react';
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectMap } from './ObjectMap';
+
+// Mock react-map-gl/maplibre the same way the sibling ObjectMap tests do — no
+// WebGL canvas in the test env, and every assertion here is about the marker
+// count reaching the DOM, not the map surface itself.
+vi.mock('react-map-gl/maplibre', () => ({
+ default: (props: any) => <div aria-label="Map">{props.children}</div>,
+ Map: ({ children }: any) => <div aria-label="Map">{children}</div>,
+ NavigationControl: () => <div data-testid="nav-control" />,
+ Marker: ({ children, longitude, latitude }: any) => (
+ <div data-testid="map-marker" data-lat={latitude} data-lng={longitude}>
+ {children}
+ </div>
+ ),
+ Popup: ({ children }: any) => <div data-testid="map-popup">{children}</div>,
+}));
+
+const MAP_CONFIG = { latitudeField: 'latitude', longitudeField: 'longitude', titleField: 'name' };
+
+const rows = [
+ { id: '1', name: 'Loc 1', latitude: 40, longitude: -74 },
+ { id: '2', name: 'Loc 2', latitude: 41, longitude: -75 },
+];
+
+describe('ObjectMap — props.data threading (objectui#5003)', () => {
+ it('picks up rows passed as a prop AFTER mount, not just at mount', async () => {
+ const schema: any = { type: 'map', map: MAP_CONFIG };
+
+ const { rerender } = render(<ObjectMap schema={schema} data={[]} />);
+
+ await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull());
+ expect(screen.queryAllByTestId('map-marker')).toHaveLength(0);
+
+ // The host's own query resolves later and re-renders with the rows —
+ // exactly the sequence the issue describes (a query in flight at first
+ // paint). Nothing else about the schema changes.
+ rerender(<ObjectMap schema={schema} data={rows} />);
+
+ await waitFor(() => {
+ expect(screen.getAllByTestId('map-marker')).toHaveLength(2);
+ });
+ const markers = screen.getAllByTestId('map-marker');
+ expect(markers[0]).toHaveAttribute('data-lat', '40');
+ expect(markers[1]).toHaveAttribute('data-lat', '41');
+ });
+
+ it('does not refetch on every render — a stable `data` prop is read once', async () => {
+ const schema: any = { type: 'map', map: MAP_CONFIG };
+
+ const { rerender } = render(<ObjectMap schema={schema} data={rows} className="a" />);
+ await waitFor(() => expect(screen.getAllByTestId('map-marker')).toHaveLength(2));
+
+ // Re-render with the SAME `data` array identity but a different unrelated
+ // prop. If the effect depended on the whole `rest` object (a fresh object
+ // every render) instead of the declared `data` prop, this would be
+ // indistinguishable from a genuine data change and would flip back
+ // through the `loading` gate, unmounting `MapGL` for a beat.
+ rerender(<ObjectMap schema={schema} data={rows} className="b" />);
+
+ // Give any spurious effect a tick to fire before asserting steady state.
+ await new Promise((r) => setTimeout(r, 0));
+ expect(screen.queryByText('Loading map...')).toBeNull();
+ expect(screen.getAllByTestId('map-marker')).toHaveLength(2);
+ });
+});

packages/plugin-map/src/ObjectMap.tsx

@@ -50,6 +50,15 @@ export interface ObjectMapProps {
  schema: ObjectMapSchema;
  dataSource?: DataSource;
  className?: string;
+ /**
+ * Records to render directly, bypassing this component's own fetch — the
+ * shape `ListView` passes when it already holds the rows. Declared as its
+ * own prop (not read off the `rest` spread) so the fetch effect can depend
+ * on this one value: naming the whole `rest` object in that effect's deps
+ * instead would refetch on every render, since `rest` is a fresh object
+ * each render (objectui#5003).
+ */
+ data?: any[];
  onMarkerClick?: (record: any) => void;
  onRowClick?: (record: any) => void;
  onEdit?: (record: any) => void;
@@ -474,13 +483,13 @@ export const ObjectMap: React.FC<ObjectMapProps> = ({
  schema,
  dataSource,
  className,
+ data: dataProp,
  onMarkerClick,
  onRowClick,
  onEdit,
  onDelete,
  enableClustering,
  clusterRadius = 50,
- ...rest
 }) => {
  const [data, setData] = useState<any[]>([]);
  const [loading, setLoading] = useState(true);
@@ -542,14 +551,14 @@ export const ObjectMap: React.FC<ObjectMapProps> = ({
  try {
  setLoading(true);
 
- // Prioritize data passed via props (from ListView)
-if ((rest as any).data) { // Check props.data directly first
- const passed = (rest as any).data;
- if (Array.isArray(passed)) {
- setData(passed);
- setLoading(false);
- return;
- }
+ // 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
+// refetch-every-render trap (objectui#5003).
+if (Array.isArray(dataProp)) {
+setData(dataProp);
+setLoading(false);
+return;
  }
 
  // Check schema.data next
@@ -597,7 +606,7 @@ export const ObjectMap: React.FC<ObjectMapProps> = ({
  };
 
  fetchData();
- }, [dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema]);
+ }, [dataProp, dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema]);
 
  // Fetch object schema for field metadata
  useEffect(() => {
@@ -663,6 +672,28 @@ export const ObjectMap: React.FC<ObjectMapProps> = ({
 
  const [currentZoom, setCurrentZoom] = useState(mapConfig.zoom || 3);
 
+ /**
+ * Seed `currentZoom` from the camera MapLibre actually applies at mount,
+ * instead of leaving it at the nominal `mapConfig.zoom || 3` above until
+ * the user's first zoom. `initialViewState` (computed below) — including a
+ * `bounds` fit — is resolved by the constructor before react-map-gl attaches
+ * its React event handlers, so no `onZoom` fires for that first camera.
+ * `onLoad` fires once the style has loaded and the initial camera has
+ * settled (fit-bounds included), so reading the zoom off that event
+ * captures the real applied value without waiting on user interaction
+ * (objectui#5003).
+ */
+ const handleMapLoad = useCallback((e: { target?: { getZoom?: () => number } }) => {
+ try {
+ const zoom = e?.target?.getZoom?.();
+ if (typeof zoom === 'number' && Number.isFinite(zoom)) {
+ setCurrentZoom(Math.round(zoom));
+ }
+ } catch {
+ /* ignore — falls back to the nominal seed / next onZoom */
+ }
+ }, []);
+
  const navigation = useNavigationOverlay({
  navigation: schema.navigation,
  objectName: schema.objectName,
@@ -789,6 +820,7 @@ export const ObjectMap: React.FC<ObjectMapProps> = ({
  touchZoomRotate={true}
  dragRotate={true}
  touchPitch={true}
+ onLoad={handleMapLoad}
  onZoom={(e) => setCurrentZoom(Math.round(e.viewState.zoom))}
  onError={handleMapError}
  >

packages/plugin-map/src/ObjectMap.zoomSeed.test.tsx

@@ -0,0 +1,107 @@
+/**
+ * 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.
+ *
+ * Regression (objectui#5003): `currentZoom` — the value `clusterMarkers` uses
+ * for its grid cell size — was seeded with the NOMINAL `mapConfig.zoom || 3`
+ * and updated only by `onZoom`. MapLibre applies the initial camera
+ * (`initialViewState`, including a `bounds` fit) via its constructor, before
+ * react-map-gl attaches React's event handlers, so no `onZoom` ever fires for
+ * that first camera — the seed stayed nominal until the user's first zoom.
+ *
+ * This was equally true before objectui#4941 (the old nominal seed was `10`);
+ * that PR did not introduce it. Recorded here so it is not rediscovered as a
+ * regression of that PR.
+ *
+ * Dormant on the console path: clustering only engages above 100 markers (or
+ * explicit opt-in), so a stale seed has no visible effect on the small sets
+ * in the examples. These tests opt in explicitly and use a marker pair whose
+ * grid-cell membership flips between the nominal seed (3) and a zoomed-in
+ * camera (12), so the cluster/no-cluster split is a direct, observable proxy
+ * for what `currentZoom` actually holds.
+ */
+import React from 'react';
+import { act, render, screen, waitFor } from '@testing-library/react';
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { ObjectMap } from './ObjectMap';
+
+let capturedProps: any = null;
+
+vi.mock('react-map-gl/maplibre', () => ({
+ default: (props: any) => {
+ capturedProps = props;
+ return <div aria-label="Map">{props.children}</div>;
+ },
+ Map: ({ children }: any) => <div aria-label="Map">{children}</div>,
+ NavigationControl: () => <div data-testid="nav-control" />,
+ Marker: ({ children, longitude, latitude }: any) => (
+ <div data-testid="map-marker" data-lng={longitude} data-lat={latitude}>
+ {children}
+ </div>
+ ),
+ Popup: ({ children }: any) => <div data-testid="map-popup">{children}</div>,
+}));
+
+// ~1 degree apart in both axes: at the nominal seed (zoom 3, cell size
+// 50/2^3 = 6.25deg) both fall in the same grid cell and cluster into one; at
+// zoom 12 (cell size 50/2^12 ≈ 0.0122deg) they land in different cells and
+// render as two separate markers. The split is the observable signal.
+const twoNearbyCities = [
+ { id: '1', name: 'Loc 1', latitude: 40, longitude: -74 },
+ { id: '2', name: 'Loc 2', latitude: 41, longitude: -75 },
+];
+
+const schema: any = {
+ type: 'map',
+ map: { latitudeField: 'latitude', longitudeField: 'longitude', titleField: 'name' },
+ // No declared `zoom` — the nominal `mapConfig.zoom || 3` seed applies.
+ data: { provider: 'value', items: twoNearbyCities },
+};
+
+describe('ObjectMap — clustering zoom seed (objectui#5003)', () => {
+ beforeEach(() => {
+ capturedProps = null;
+ });
+
+ it('starts clustered under the nominal seed, then splits once the applied camera is read', async () => {
+ render(<ObjectMap schema={schema} enableClustering />);
+
+ await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull());
+ await waitFor(() => expect(capturedProps).not.toBeNull());
+
+ // Before `onLoad` fires: the nominal seed (3) groups the pair into one
+ // cluster — this is the bug's starting state, present on both legs. The
+ // cluster pin is itself wrapped in a `Marker`, so `map-marker` reads 1
+ // here too (one wrapper, holding the one `map-cluster` div).
+ await waitFor(() => {
+ expect(screen.getAllByTestId('map-cluster')).toHaveLength(1);
+ });
+ expect(screen.getAllByTestId('map-marker')).toHaveLength(1);
+
+ // MapLibre's 'load' event: the camera has settled. `onLoad` is undefined
+ // on the unfixed component (no handler wired), so this is a no-op there —
+ // predicted red assertion below is what tells the two legs apart.
+ act(() => {
+ capturedProps.onLoad?.({ target: { getZoom: () => 12 } });
+ });
+
+ await waitFor(() => {
+ expect(screen.getAllByTestId('map-marker')).toHaveLength(2);
+ });
+ expect(screen.queryAllByTestId('map-cluster')).toHaveLength(0);
+ });
+
+ it('ignores an onLoad event carrying no readable zoom', async () => {
+ render(<ObjectMap schema={schema} enableClustering />);
+ await waitFor(() => expect(capturedProps).not.toBeNull());
+ await waitFor(() => expect(screen.getAllByTestId('map-cluster')).toHaveLength(1));
+
+ // No `target`, or a `target` without `getZoom` — must not throw, and must
+ // leave the seed (and therefore the clustering) unchanged.
+ expect(() => act(() => { capturedProps.onLoad?.({}); })).not.toThrow();
+ expect(screen.getAllByTestId('map-cluster')).toHaveLength(1);
+ });
+});