Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-map): thread props.data and seed clustering zoom from the applied camera - #5297
Merged
os-support-ai merged 3 commits intoAug 19, 2026
Merged
Conversation
…the clustering zoom from the applied camera
ObjectMap had two pieces of state that stopped tracking their source after
mount (objectui#5003):
- The fetch effect preferred records passed via `props.data`, but read them
off the `rest` spread and didn't depend on them — a host re-rendering with
new rows (data={[]} then data={rows}) kept showing the empty map. `data` is
now a declared prop (`data: dataProp`) and a direct effect dependency; the
whole `rest` object is deliberately NOT added (fresh identity every render
would refetch on every render instead).
- `currentZoom` (the clustering grid-cell seed) stayed at the nominal
`mapConfig.zoom || 3` until the user's first zoom, because MapLibre applies
the initial camera via `initialViewState` before react-map-gl attaches
React's `onZoom` handler. It is now also seeded from `onLoad`, which fires
once the initial camera (including a bounds fit) has settled.
Both are dormant on the console path (ElementDataSourceGate always drives the
map through its own dataSource.find, never through props.data; clustering
only engages above 100 markers or explicit opt-in) — see the issue's "Why
this is dormant" section. Tests exercise the prop-update path and the
pre-onZoom clustering state directly, since a console-path test would pass
before and after.Add a row for it to the "Using ObjectMap directly" props table — it was only reachable, undocumented, off the `rest` spread before this fix.
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Aug 19, 2026
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Aug 19, 2026
os-support-ai
marked this pull request as ready for review
August 19, 2026 12:10
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-5003-objectmap-prop-and-zoom-seed
branch
August 19, 2026 12:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5003
What
Two pieces of
ObjectMapstate stopped tracking their source after mount. Both aredormant on the console path (see "Why untestable through the console path" below), so
neither has ever caused a visible bug in the example apps — this is a
finding,promoted to
pm:queueby triage, being fixed proactively.1.
props.datawas read but not depended on. The fetch effect preferred recordspassed via
props.data, checked as(rest as any).data, butrest(and thereforedata) was not in the effect's dependency array. A host renderingObjectMapwith adata={[]}prop while its own query is in flight, then re-rendering once it resolveswith
data={rows}, kept showing the empty map — the prop changed, the effect neverre-ran to notice.
Fix:
datais now a declared prop onObjectMapProps(data: dataPropin thedestructure), and the effect depends on that one value directly. The whole
restobjectis deliberately not added to the deps — it is a fresh object every render, so naming
it would turn this into a refetch-on-every-render trap instead.
2. The clustering zoom seed never saw the initial camera.
useState(mapConfig.zoom || 3)seedscurrentZoom, whichclusterMarkersuses for itsgrid cell size, and
onZoomwas its only writer. MapLibre applies the initial camera(
initialViewState, including aboundsfit) via its constructor, before react-map-glattaches React's event handlers — so no
onZoomever fires for that first camera, andthe seed stayed nominal until the user's first zoom.
Fix: also seed
currentZoomfromonLoad, which fires once the style has loaded and theinitial camera has settled (fit-bounds included), so clustering at first paint reflects
the camera MapLibre actually applied.
This was equally true before #4941 (the seed was then the synthesized
10); that PR didnot introduce it, and the current
3is closer to a fitted continental camera than10was. Not a regression of #4941 — recorded in the issue so it isn't rediscovered as one.
Why untestable through the console path
dataas a prop — it goes throughElementDataSourceGateand the component's owndataSource.find(
ObjectMap.elementDataSource.test.tsxpins this).loadingback totrue, which unmounts and remountsMapGL, so adata change through the effect is always reflected — masking the missing dependency.
has no visible effect on the small marker sets in the examples.
So the new tests exercise both paths directly rather than through the console/schema
flow, which would pass before and after the fix and prove nothing.
Tests
New files:
ObjectMap.dataProp.test.tsx,ObjectMap.zoomSeed.test.tsx.dataProptest 1 mounts with an emptydataarray, re-renders with rows, asserts therows appear — this is the one that actually pins the fix.
dataProptest 2 checks a stabledatareference plus an unrelated prop changedoesn't trigger a spurious refetch. Non-discriminating: green on both the fixed and
reverted legs (the pre-fix code never re-ran the effect off a prop change at all, by
identity or otherwise), kept as a regression guard for the "don't depend on the whole
restobject" constraint rather than as a pin.zoomSeedtest 1 uses a marker pair placed so grid-cell membership flips between thenominal seed (zoom 3, same cell) and a post-
onLoadzoom of 12 (different cells):asserts 1 cluster before
onLoad, 2 separate markers after. This is the one that pinsthe fix.
zoomSeedtest 2 checks anonLoadpayload with no readable zoom is a no-op.Non-discriminating: green on both legs (
onLoaddoesn't exist pre-fix, so callingit is already a no-op there).
Reverse verification: committed the fix (
32bef3bd0), then checked out pre-fixObjectMap.tsxfromorigin/mainwith the new tests kept, predicteddataProptest 1and
zoomSeedtest 1 would go red and the other two would stay green, ran the suite, andobserved exactly that (
2 failed | 63 passed, the two predicted names in the failurelist). Restored the fix from the branch tip (clean diff against HEAD afterward) and
reran —
65 passed (65).Both run at head
d031db984(post final commit).Also ran, at the same head:
node scripts/check-changeset-presence.mjs(declares thechangeset),
node scripts/check-changeset-no-major.mjs(nomajorbump — this is apatch),node scripts/check-control-bytes.mjs(clean), andpnpm exec eslinton thetouched files (0 errors; the pre-existing
react-hooks/exhaustive-depswarning forreston the fetch effect is gone — the remaining one isschema.data, a separate,out-of-scope read this issue didn't ask for).
Scope note
ObjectMapPropsgains one optional prop (data?: any[]); nothing on the schema/specsurface changes — no key gains or loses acceptance.
packages/plugin-map/README.mdgets a row for the newly-declareddataprop (it waspreviously reachable only off the untyped
restspread, undocumented).Changeset
patchon@object-ui/plugin-map.Generated by Claude Code