Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-map): memoize ObjectMap's mapConfig so the marker useMemo actually memoizes - #6016
Merged
Merged
Conversation
…actually memoizes getMapConfig(schema) ran unmemoized in the render body, giving mapConfig a fresh object identity every render. The marker transform names it in its dependency array, so that useMemo recomputed on every render while declaring that it does not, cascading through filteredMarkers / clusteredData / markerBounds / initialViewState. Memoized on [schema] — the single value getMapConfig reads — rather than on a JSON.stringify deep-compare key: the schema identity reaching this component is already stable across the renders that matter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
`at` is outside this package's configured `lib` target, so `tsc -p tsconfig.test.json` rejected it (TS2550). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
This was referenced Aug 24, 2026
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 13:33
Uh oh!
There was an error while loading. Please reload this page.
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#5976
What was wrong
getMapConfig(schema)was called bare inObjectMap's render body, somapConfigcarried a fresh object identity on every render. The marker transform namesmapConfigin its dependency array ([data, mapConfig, objectSchema]), so thatuseMemorecomputed on every single render while declaring that it does not — auseMemoin spelling only. Every pass walked all records throughextractCoordinatesand the display-name resolver and re-ranObjectMapConfigSchema.safeParse, and the invalidation cascaded on intofilteredMarkers→clusteredData/markerBounds→initialViewState.Measured, not inferred: this is failure mode (b) — the builder called outside the memo — not a fresh-identity value inside an otherwise-sound dep array. The memo's own dep list is fine; its input was not.
Re-measured on current
main, after #5953 merged#5953 (PR #5975) rewrote this exact region under the card, so the quoted code was re-derived rather than trusted. Delta against the card body:
getRecordDisplayName(objectSchema, record, { titleField, fallback }), notrecord[titleField];|| 'name'fallback is gone from bothgetMapConfigbranches;objectSchemais now in the marker memo's deps —[data, mapConfig, objectSchema], not the card's[data, mapConfig].The defect itself is unchanged.
ObjectMap.tsx:594still readconst mapConfig = getMapConfig(schema);, and the extraobjectSchemadep neither causes nor worsens it.The fix, and why it is not the neighbouring idiom
[schema]is the entire dependency, not a shorthand for one:getMapConfigis a pure function ofschemaand reads nothing else.No
JSON.stringifydeep-compare key was needed, because the identity reaching this component is already stable across the renders that matter. Every re-renderObjectMapcauses itself — data landing, the object definition landing, search typing, zoom, selection, geolocation — leaves theschemaprop untouched by construction, and each caller upstream hands over a memoized node:SchemaRendererschemaForComponent=evaluatedSchemauseMemo,SchemaRenderer.tsx:516ObjectMapRenderer→ElementDataSourceGatebound.schema=mappedReact.useMemo,useElementDataSourceSchemaListViewcase 'map'flattenviewComponentSchemaReact.useMemo,ListView.tsx:2007Two further reasons the serialize would have been the wrong trade here, beyond its per-render cost:
JSON.stringifydropsundefinedvalues, and an absenttitleFieldis load-bearing in this config since #5953 — it must never compare equal to a present one. Identity is sufficient; nothing on this path needs value equality.The sibling
dataConfigat:590keeps itsJSON.stringifykey and is untouched — it is not the same defect (it does memoize) and changing it is a separate question.Verification
New pin:
packages/plugin-map/src/ObjectMap.configMemo.test.tsx(5 tests). It measures identity, at the two module boundaries the component actually crosses —getRecordDisplayNamecall count (a direct read of how many times the marker memo evaluated) and theinitialViewStateobject handed toMapGL(the tail of the cascade, so atoBeon it pins the whole chain).Red before, green after. Direction predicted before running; on unmodified
mainthe three stability pins failed and the two counter-probes passed —Tests 3 failed | 2 passed (5), exit 1. One failure reads:— deep-equal, identity-fresh: the defect's exact signature.
Reverse-verification. Reverting the memo to the bare call reproduced that split exactly —
Tests 3 failed | 2 passed (5), exit 1. Mutation proved on disk by grepping the injected text and separately the removed text; restore ran undertrap … EXIT INT TERM, andgit diff HEAD --statwas empty afterwards. No rebuild was involved: the root vitest config aliases@object-ui/*topackages/*/srcand the mutated file is the test's own relative import, so nothing resolved throughdist/.Counter-probes (without these, "stable identity" is also satisfiable by freezing a stale config forever — a worse bug, invisible to the positive assertion alone). Both assert a new identity with a visible consequence:
titleFieldre-resolves the titles and the new binding reaches the rendered popup (Harbour Depot→HD-01);center/zoomrebuilds the camera and it moves to the newly declared position.Gates
pnpm --filter @object-ui/plugin-map type-check> @object-ui/plugin-map@17.6.0 type-check/> tsc --noEmit && tsc -p tsconfig.test.json, so this is not a zero-match silent passpnpm exec vitest run packages/plugin-map/src(root form)Test Files 15 passed (15),Tests 88 passed (88)pnpm exec eslint --no-inline-configon changed filesnode scripts/check-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)All four run on the final commit
0f754a839.eslint delta against merge-base
c0091b82b:ObjectMap.tsxis unchanged — 0 errors / 15 warnings before and after, with an identical rule breakdown (no-explicit-any12,react-hooks/exhaustive-deps2,react-hooks/use-memo1). The baseline was measured by checking out the merge-base copy of the file under a restoringtrapand linting it. The new test file adds 4 warnings, all@typescript-eslint/no-explicit-anyon thereact-map-glmock props — the established idiom in this directory, and fewer than its siblings carry (ObjectMap.markerTitle.test.tsx7,ObjectMap.schemaDataShorthand.test.tsx9). No new rule class, no errors.Notes
filteredMarkers,clusteredData(which callsclusterMarkers),markerBounds(which callscomputeMarkerBounds),selectedMarkerandinitialViewState. NouseEffectre-fired — neither of the component's two effects namesmapConfigormarkers. So the blast radius is wasted per-render computation cascading through the whole marker pipeline, which is real, but it is narrower than "re-fire effects per render" would suggest. Recorded as the dispatch asked, not as a reason to skip the fix.react-hooks/exhaustive-depsis active in this repo but only at warn, and there is no rule that would catch a builder called outside a memo at all. Nothing mechanical would have caught this, and nothing mechanical would catch its regression — the new identity test is what makes the declared intent stay true.ObjectMap.listViewMapConfigReach.test.tsx:138narrates agetMapConfigliteral in the present tense that no longer exists #5977 is untouched, as instructed. Nothing here falsifies or repairs what it narrates: it is about stale prose above an arm ofObjectMap.listViewMapConfigReach.test.tsx, and that file is not modified.⛔ Draft on purpose — not marked ready, not enqueued, auto-merge not enabled. The PM lands it.
Generated by Claude Code