fix: stop re-serializing overlay props on unrelated re-renders

[jkasprzyk17]

Problem

Nitro diffs view props by reference identity (jsi::Value::strictEquals), but MapView rebuilt every overlay array, every callback(...) envelope and both entering-animation descriptors on each render. A re-render of the component holding the MapView — a timer, an unrelated state tick — therefore marked props dirty even when the map data was byte-identical.

Measured by rendering MapView twice with identical data and diffing the props exactly as Nitro does:

scenario	before	after
<Marker /> children + all handlers + entering animations	19 dirty props	0
<Marker /> children, nothing else	5	0
bulk markers, caller-memoized, one handler	2	0
bare <MapView style={STYLE} />	1	0

Each dirty prop cost a full JSIConverter pass on the JS thread. For markers that is 13 getProperty calls per descriptor plus nested reads (~27 for a marker with an image, anchor, offset and animation) and a std::string allocation per text field — then a C++→Swift array materialisation and a native re-apply. hybridRef being dirty also re-invoked the JS ref callback through the dispatcher on every render, even for a MapView with no props at all.

Approach

The whole fix is controlling when the reference changes:

• useStableValue holds the last returned value in a ref and hands it back when the new one is structurally equal. One primitive, N comparators — it covers the four overlay arrays and both entering-animation descriptors.
• descriptorEquality provides the comparators: field-by-field for markers, polylines, polygons and circles, plus coordinate lists and entering animations.
• useNitroCallback memoises the { f } envelope on the handler it wraps instead of allocating one per render; hybridRef's envelope is created once per mount.
• normalizeMarkerDescriptors drops its own identity-preservation (and the partly broken descriptorsEqual behind it) — that job now belongs to the stabilizer, which does it correctly for every field.

When something genuinely changes, the comparator returns false, the reference changes, and the chain runs exactly as before. Nothing is skipped or deferred.

Behaviour change

Descriptors and the objects inside them are now treated as immutable. Mutating a coordinate you already handed to MapView is not picked up, because the comparison sees the same object on both sides — with <Marker /> children that previously happened to work. Documented in the README with an example, and pinned by a test so it stays a deliberate contract.

Testing

• 73 new tests in descriptorEquality.test.ts. Every field of every descriptor has its own mutation case: a field a comparator misses is a map update that silently never reaches native, so the coverage is exhaustive by construction. Also pinned: the four near-identical list wrappers each reach their own item comparator, and the shared-nested-object contract above.
• normalizeMarkerDescriptors.test.ts reworked from identity assertions onto value assertions (require() resolution, resolution caching, animation normalisation) — the identity behaviour it pinned no longer exists.
• Separately verified with react-test-renderer that real changes still propagate: moving a marker, renaming a title, adding/removing a marker, editing a polyline coordinate, swapping a handler, changing an entering animation, and switching between bulk props and children.
• bun run lint, bun run typecheck, bun run build, 129 src tests and the plugin suite all pass.

Not included

Shape overlays (polylines / polygons / circles) are still torn down and rebuilt natively on every set — MapOverlayController has no equality guard for them the way markers do. That is now unreachable from MapView because this change never sets an unchanged shape prop, but it is worth fixing on its own. It should be built on the renderSignature() helper that landed in #54 rather than a bespoke fold; I have a draft that predates that commit and needs rewriting against it.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[github-actions]

React Doctor found 8 issues in 5 files · 2 errors & 6 warnings · score 64 / 100 (Needs work) · full project

Errors

• ❌ App.tsx:727 State updater has side effects no-impure-state-updater
• ❌ src/hooks/useCollectedOverlays.ts:239 Ref mutated during render no-ref-current-in-render

6 warnings

App.tsx

• ⚠️ L729 Side effect inside a state updater function no-side-effect-in-state-updater-function
• ⚠️ L734 Side effect inside a state updater function no-side-effect-in-state-updater-function
• ⚠️ L735 Side effect inside a state updater function no-side-effect-in-state-updater-function

package.json

• ⚠️ L0 unused-dev-dependency

src/hooks/index.ts

• ⚠️ L0 unused-file

src/utils/enteringAnimation.ts

• ⚠️ L33 unused-export

_{Reviewed by React Doctor for commit 95528d3. See inline comments for fixes.}

[github-actions]

React Doctor · react-doctor/no-ref-current-in-render (error)

This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.

Fix → Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.

Docs

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6c055a2e-4c92-4a09-9610-1e20d9df7564

📥 Commits

Reviewing files that changed from the base of the PR and between c278b56 and 95528d3.

📒 Files selected for processing (1)

• package/src/hooks/useStableValue.ts

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

📝 Walkthrough

Summary by CodeRabbit

• Performance

  • Improved map rendering stability by reducing unnecessary overlay and animation updates.
  • Improved event-handler stability for smoother native map interactions.

• Bug Fixes

  • Fixed committed-value handling during interrupted renders.
  • Improved detection of meaningful changes to markers, polylines, polygons, circles, coordinates, and animations.

• Documentation

  • Added guidance on reference-based prop comparisons, event handlers, immutable updates, and creating new objects when changing marker data.

Walkthrough

MapView now preserves stable overlay, animation, reference, and event-handler values across equivalent renders. New equality utilities, React hooks, normalization tests, and README guidance document and validate this behavior.

Changes

MapView stability

Layer / File(s)	Summary
Overlay comparison and normalization package/src/overlays/descriptorEquality.ts, package/src/overlays/normalizeMarkerDescriptors.ts, package/src/overlays/__tests__/*, package/src/utils/__tests__/enteringAnimation.test.ts	Structural comparators cover overlay fields, nested values, lists, and entering animations. Marker normalization always maps descriptors to normalized values. Tests cover equality, ordering, mutation, image resolution, and animation mappings.
Stable callback and value hooks package/src/hooks/useNitroCallback.ts, package/src/hooks/useStableValue.ts	useNitroCallback memoizes Nitro callback envelopes. useStableValue commits updated values in a layout effect and retains the previous committed value during discarded renders.
MapView native prop stabilization package/src/components/MapView.tsx, README.md	MapView stabilizes overlay and animation descriptors and memoizes native handlers and the hybrid ref callback. The README documents reference comparison and immutable update requirements.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 95528

The change prevents unnecessary overlay updates while preserving propagation of genuine changes, with documented immutability behavior and passing validation checks; no actionable merge-blocking risk remains.

Suggested reviewers: piotr-graczyk-dev

Sequence Diagram(s)

sequenceDiagram
  participant MapView
  participant useStableValue
  participant useNitroCallback
  participant NativeMapView

  MapView->>useStableValue: compare overlay and animation descriptors
  useStableValue-->>MapView: return committed stable values
  MapView->>useNitroCallback: memoize native event handlers
  useNitroCallback-->>MapView: return Nitro callback envelopes
  MapView->>NativeMapView: pass stable props and callbacks
``

</details>

<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->

---

<!-- pre_merge_checks_override_start -->
> [!CAUTION]
> ## Pre-merge checks failed
> 
> Please resolve all errors before merging. Addressing warnings is optional.
> 
> - [ ] <!-- {"checkboxId":"override-pre-merge-checks"} --> Ignore
<!-- pre_merge_checks_override_end -->

### ❌ Failed checks (1 error)

|   Check name   | Status  | Explanation                                                                                                                                                                                               | Resolution                                                                                                                                                                                                                                        |
| :------------: | :------ | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| Security Check | ❌ Error | High-confidence supply-chain vulnerability: the new release workflow runs attacker-controlled repository code with release credentials. `workflow_dispatch` has no branch restriction, and `actions/chec… | Restrict rehearsal runs to a trusted ref, such as the protected default branch, and reject all other `workflow_dispatch` refs before checkout or before any repository code executes. Prefer separate jobs: run validation with read-only permis… |

<details>
<summary>Full details: Security Check</summary>

**Explanation**

High-confidence supply-chain vulnerability: the new release workflow runs attacker-controlled repository code with release credentials. `workflow_dispatch` has no branch restriction, and `actions/checkout` uses the selected ref. A user with repository write access can push a malicious branch and dispatch this workflow against it. The workflow then runs repository-controlled install, codegen, build, and test commands. The publish step also exposes `GITHUB_TOKEN` while executing `bunx release-it`, which loads repository-controlled package scripts and release hooks. The job grants `contents: write` and `id-token: write`. The manual-run `--dry-run` flag does not sandbox earlier commands or release hooks. This permits exfiltration or misuse of the GitHub token and OIDC-based publishing credentials. Repository guidance says manual runs use the current branch, and the workflow contains no protected environment or trusted-ref check.

**Resolution**

Restrict rehearsal runs to a trusted ref, such as the protected default branch, and reject all other `workflow_dispatch` refs before checkout or before any repository code executes. Prefer separate jobs: run validation with read-only permissions, then run publishing only from a protected tag or reviewed commit after an approval-gated environment. Do not expose `GITHUB_TOKEN` or OIDC permissions to jobs that execute checked-out repository code. If release hooks must run, use a trusted release configuration and pin the source commit rather than loading configuration from the manually selected branch.

</details>

<!-- pre_merge_checks_walkthrough_end -->
<!-- tips_start -->

---




<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>

<!-- tips_end -->

Loading