Repository navigation
ObjectMap: two pieces of state stop tracking their source after mount — the data prop is absent from the fetch effect's deps, and the clustering zoom seed never sees the initial camera #5003
Description
Activity
os-support-ai commented
on Aug 19, 2026 CollaboratorMore actionsTriage first-touch — concentrated round #2 (triage seat, session
session_019ZyCHcjXeTFPiS8RWRk4nN, 2026-08-19): promotedfinding→pm:queue· Task. Dormant on the console path but real for any embedding host passingdataas a prop (rows arriving after mount are never shown), and the fix shape is specified: destructuredata: dataPropand depend on that one value (⛔ notrest— refetch-every-render trap the body names); seed the clustering zoom from the applied initial camera or firstonZoom. Both items one PR inpackages/plugin-map/src/ObjectMap.tsx, with tests for the prop-update path. Serial constraint: #5020 edits the same file (doc comment) — claim whichever lands first and merge main before the second; note the overlap in the claim comment. Size/model suggestion: S–M, sonnet.
Generated by Claude Code
os-support-ai commented
on Aug 19, 2026 CollaboratorMore actionsClaim: PM loop round 13
Session:
session_01RV6yuVCxymHYE16PL9vQkE
Branch:claude/issue-5003-objectmap-prop-and-zoom-seed
Worktree:objectui-issue-5003
Domain:repo:objectui
File surface:packages/plugin-map/src/ObjectMap.tsx+ its tests (stop on breach; explain in the report)
Container & model: S–M,mode:subagent,model: sonnet(taking the triage seat's size/model suggestion)
Clause-②: no — neither item changes what the contract accepts or rejects, and nothing on the public surface widens. Item 1 makes an existing declared prop actually tracked after mount; item 2 corrects an internal seed. No schema key gains or loses acceptance.
Serial constraints cleared: #5020 is the one card triage named as editing this same file (itsclusterRadiusdoc comment) — it ispm:queue, unassigned, not in flight, so there is no live hold and this claim takes the file first. Open PRs hold disjoint surfaces, each measured bygit diff --name-only origin/main...: PR #5290 →apps/console/**; PR #5292 →eslint-rules/**+eslint.config.js; PR #5294 →packages/plugin-view/**. No in-flight card is known to touchpackages/plugin-map; if one turns out to, stop and report.Scope — both items, one PR,
Fixes #5003Triage promoted this
finding→pm:queue· Task on 2026-08-19 and specified the fix shape. Carried through:Item 1 —
props.datais read by the fetch effect but is not one of its dependencies. ⛔ Do not addrestto the deps array:restis a fresh object every render, so naming it converts the bug into a refetch-every-render trap. The narrow fix the card names is to destructure the prop (data: dataProp) and depend on that one value. The eslintreact-hooks/exhaustive-depswarnings already reported on this effect (forrestandschema) are the signal, not the specification — satisfying the linter is not the goal, threading the prop correctly is.Item 2 — the clustering zoom seed never sees the initial camera.
useState(mapConfig.zoom || 3)seeds whatclusterMarkersuses for grid cell size, andonZoomis the only writer. MapLibre's constructor applies the initial camera viainitialViewStatebefore react-map-gl attaches React handlers, so noonZoomever fires for it. Seed from the applied initial camera (or the firstonZoom).Verification the report must carry
The card documents why this is dormant, and that is exactly what makes it easy to "fix" without evidence:
- the console path never passes
dataas a prop (it goes throughElementDataSourceGateand the component's owndataSource.find); - every fetch sets
loadingback totrue, which unmounts and remountsMapGL, so a data change through the effect is always reflected; - clustering only engages above 100 markers (or explicit opt-in).
So a test that renders through the console path will pass before and after — it proves nothing. Test the prop-update path directly: mount with
data={[]}, then pass rows, and assert the rows are shown. Reverse-verify: predict which assertions go red with the fix reverted, name them before running, and report predicted-vs-observed. If an assertion is green on both legs, it is not pinning the fix — say so rather than counting it.Item 2's own note: this was equally true before #4941 (the seed was then the synthesized
10); that PR did not introduce it and the current3is closer to a fitted continental camera than10was. Recorded so it is not re-reported as a regression of #4941.
Generated by Claude Code
- the console path never passes
os-support-ai commented
on Aug 19, 2026 CollaboratorMore actions{ "issue": 5003, "status": "done", "branch": "claude/issue-5003-objectmap-prop-and-zoom-seed", "pr": "https://github.com/objectstack-ai/objectui/pull/5297", "premise_still_valid": true, "summary": "Fixed both dormant state-tracking bugs in ObjectMap: (1) declared `data` as its own prop (`data: dataProp`) instead of reading it off the untyped `rest` spread, and added that single value to the fetch effect's deps — the whole `rest` object is deliberately excluded to avoid a refetch-every-render trap; (2) added an `onLoad` handler that seeds `currentZoom` from the actually-applied initial camera (MapLibre resolves `initialViewState`, including a bounds fit, before react-map-gl attaches `onZoom`), so clustering at first paint no longer relies on a nominal `mapConfig.zoom || 3` seed. Also documented the newly-declared `data` prop in the package README (it was previously reachable but undocumented).", "tests": "New files ObjectMap.dataProp.test.tsx and ObjectMap.zoomSeed.test.tsx, mounting outside the console/schema path per the card's warning (console path never passes `data` as a prop; clustering needs an explicit opt-in or >100 markers to engage). `pnpm --filter @object-ui/plugin-map run type-check` (tsc --noEmit + tsconfig.test.json) clean; `pnpm exec vitest run packages/plugin-map --maxWorkers=2` = 11 files / 65 passed. Both run at final head d031db984. Reverse verification: committed the fix (32bef3bd0), predicted before running that `dataProp`'s 'picks up rows passed as a prop AFTER mount' and `zoomSeed`'s 'starts clustered under the nominal seed, then splits once the applied camera is read' would go red on pre-fix ObjectMap.tsx (checked out from origin/main, tests kept) and the other two new tests would stay green; ran the suite and observed exactly that (2 failed | 63 passed, the two predicted names in the failure list). Restored the fix from the branch tip (`git checkout <branch> -- ObjectMap.tsx`, clean diff against HEAD) and reran: 65 passed (65). The two non-discriminating tests (dataProp test 2, zoomSeed test 2) are called out as such in the PR body rather than counted as pins — both were green on both legs. No build artifact sits between edit and test: the test files import `./ObjectMap` by relative path within the package, and the root vitest.config.mts also aliases `@object-ui/plugin-map` to `packages/plugin-map/src`. Also ran at the same head: check-changeset-presence.mjs, check-changeset-no-major.mjs (patch, not major), check-control-bytes.mjs, and eslint on the touched files (0 errors; the pre-existing react-hooks/exhaustive-deps warning for `rest` on this effect is gone, leaving only the pre-existing, out-of-scope `schema.data` one — see out_of_scope_findings).", "open_questions": [], "out_of_scope_findings": [ "not filed: the same fetch effect also reads `schema.data` directly (a separate short-circuit checking whether `schema.data` itself is an array, distinct from the `ViewData`-shaped `schema.data` `getDataConfig` normally expects) without it in the dependency array either -- eslint's react-hooks/exhaustive-deps flags it (`missing dependency: 'schema.data'`) the same way it flagged `rest` before this fix. Left alone: it's a different code path from what #5003 named (props.data, not schema.data), the 'correct' fix isn't pinned by an authoritative source the way item 1's was (unclear whether the array-shaped-schema.data branch is reachable at all given `ObjectMapSchema.data: ViewData`, i.e. it may be dead code, in which case the right fix is deletion, not a new dep), and the file-surface/gate-family bounded-fix exemption's condition 2 (correct shape already pinned) isn't clearly met. Could not search open issues for a duplicate or file a new one before writing this report: `search_issues` and `list_issues` both returned 'API rate limit already exceeded for user ID 318092878' on every attempt (the shared bot identity) -- PM should file this once the org-wide GitHub API rate limit clears, or confirm no filing is warranted after inspecting whether the array branch is reachable." ] }
Generated by Claude Code
Observation-class, found while implementing #4941. Filed unassigned, not claiming. Neither item is reachable through the console path today (see "Why this is dormant"), which is why it is a
findingrather than a bug report.1.
props.datais read by the fetch effect but is not one of its dependenciespackages/plugin-map/src/ObjectMap.tsx— the effect starts by preferring records passed as a prop:but its dependency list is
[dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema].rest.datais not in it (eslint already reports the pair asreact-hooks/exhaustive-depswarnings on this effect, forrestandschema).A host that renders the map with
data={[]}while its own query is in flight and then passes the rows would keep showing the empty map: the prop changes, the effect does not re-run, and the copy in state stays empty.2. The clustering zoom seed never sees the initial camera
const [currentZoom, setCurrentZoom] = useState(mapConfig.zoom || 3)seeds the value thatclusterMarkersuses for its grid cell size, andonZoomis the only thing that updates it. The initial camera is applied by MapLibre's constructor (viainitialViewState), before react-map-gl attaches the React handlers, so noonZoomfires for it — the seed stays nominal until the user zooms for the first time.This was equally true before #4941 (the seed was then the synthesized
10); the fix did not introduce it, and it now starts from3, which is closer to a fitted continental camera than10was. Recorded so it is not rediscovered as a regression of that PR.Why this is dormant
dataas a prop; it goes throughElementDataSourceGateand the component's owndataSource.find.loadingback totrue, which unmountsMapGLand remounts it when the data lands, so a data change through the effect is always reflected (that is also why the camera fit in plugin-map: no fitBounds to data — markers land on an adjacent world copy, initial viewport shows an empty map #4941 needs no imperativefitBoundscall).If picked up
Item 1 wants the prop threaded properly rather than added blindly to the deps array —
restis a fresh object every render, so namingrestwould refetch on every render. The narrow fix is to destructure the prop (data: dataProp) and depend on that one value.