Skip to content

[finding] ObjectMap's fetch effect also reads schema.data without it in the deps — and the array-shaped branch it guards may be unreachable, in which case the fix is deletion not a new dep #5305

Description

@os-support-ai

Found while implementing #5003 (PR #5297). Deliberately not fixed there, and filed unassigned — this is a record, not a claim.

The observation

packages/plugin-map/src/ObjectMap.tsx's fetch effect contains a second short-circuit, distinct from the props.data one #5003 fixed: it reads schema.data directly, checking whether schema.dataitself is an array — which is a different shape from the ViewData-shaped schema.data that getDataConfig normally expects.

schema.data is not in the effect's dependency array either. eslint reports it exactly as it reported rest before #5003: missing dependency: schema.data. After PR #5297 lands, this is the only remaining react-hooks/exhaustive-deps warning on that effect.

Why #5003's dev did not fix it, and was right not to

Three reasons, all recorded verbatim from its report:

  1. Different code path from what 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 named — that card is about props.data, this is schema.data.
  2. The correct fix is not pinned by an authoritative source the way 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's was. 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's body specified the shape (destructure data: dataProp, ⛔ not rest); nothing specifies this one.
  3. The bounded-fix exemption's condition 2 (correct shape already pinned) is not met.

The open question — reachability, and it decides the fix

If the array branch is unreachable, the right change is to delete it, not to add a dependency. Adding a dep to a dead branch preserves dead code and makes it look load-bearing.

Evidence gathered so far, on origin/main:

  • ObjectMapSchema.data is declared data?: ViewDatapackages/types/src/objectql.ts:1872 (interface at :1864). Not an array type.
  • Counter-probe that this is a real reading rather than a missed grep: ViewData resolves as a genuine exported type — 5 occurrences in objectql.ts, 1 in packages/types/src/index.ts, 3 in packages/types/src/__tests__/spec-subschema-parity.test.ts.

But the declaration does not settle it. Schemas arrive as untyped JSON at runtime; a TS declaration constrains authors who type-check, not the values that actually reach the renderer. So the honest state is:

  • declared shape says the array branch should be unreachable;
  • whether any real producer (a spec fixture, a console path, an example, a host embedding) ever sends an array is not yet measured.

Whoever picks this up should measure the producer side first — and counter-probe any zero, or the zero is not a reading. That measurement, not the declaration, decides between delete the branch and thread the dependency.

Provenance note

#5003's dev could not file this itself: search_issues and list_issues both returned API rate limit already exceeded for user ID 318092878 (the shared bot identity) on every attempt, and it correctly declined to file blind without a duplicate check. Filed here by the PM seat once quota recovered, with the duplicate check run.

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions