ObjectMap's dataConfig pays a JSON.stringify on every render to buy an identity that [schema] already gives for free

Measured while implementing #5976 (PR #6016). Out of that card's scope — filing plainly for triage to grade.

What was measured

packages/plugin-map/src/ObjectMap.tsx:588-592, the line immediately above the one #5976 fixed:

const rawDataConfig = getDataConfig(schema);
// Memoize dataConfig using deep comparison to prevent infinite loops
const dataConfig = useMemo(() => {
  return rawDataConfig;
}, [JSON.stringify(rawDataConfig)]);

getDataConfig(schema) is called bare in the render body — structurally the same shape as the #5976 defect. The difference is where the cost lands: dataConfig does end up with a stable identity, so this is not the same bug. It buys that stability by running JSON.stringify over the config on every render, plus a fresh getDataConfig call to feed it.

The reason that mattered is real: dataConfig is a dependency of the fetch effect, which calls setData — a fresh identity there is a re-render loop, not just waste. That is what the "prevent infinite loops" comment records.

Why the serialize now looks unnecessary rather than merely costly

#5976 established, by tracing all three callers, that the schema identity reaching this component is already stable across the renders that matter:

SchemaRenderer hands over the memoized evaluatedSchema (SchemaRenderer.tsx:516);
ElementDataSourceGate hands over mapped, a React.useMemo in useElementDataSourceSchema;
ListView's case 'map' flatten hands over viewComponentSchema, a React.useMemo (ListView.tsx:2007);
and every re-render ObjectMap causes itself leaves the prop untouched by construction.

getDataConfig is a pure function of schema and reads nothing else, so

const dataConfig = useMemo(() => getDataConfig(schema), [schema]);

appears to give the same stable identity the effect needs, with no serialize and no per-render rebuild. That is the shape #5976's fix landed for mapConfig, and its identity test now pins it.

What has NOT been measured, and would need to be before acting

Whether the loop the comment guards against is still reachable. The comment predates the caller-side memoization above; it should be confirmed the effect stays quiet under the identity-keyed form rather than assumed. ObjectMap's marker useMemo never memoizes — getMapConfig runs unmemoized in the render body, so mapConfig has a fresh identity every render #5976's pin covers the marker pipeline, not the fetch effect.
Whether JSON.stringify is currently masking a false-equal. It is key-order sensitive and drops undefined values, so two semantically different configs can produce the same key (stale — actually wrong, not merely wasteful). I did not find a reachable path: getDataConfig builds its objects with literal key order in every branch, and the one passthrough branch returns the author's own schema.data object, whose key order is fixed per object. Recorded as "not found", not as "cannot happen".

Scope note

Not folded into PR #6016. That PR's claim is narrow — mapConfig's identity — and it deliberately leaves :590 untouched and says so, because this is a different value with a different consumer (an effect, not a derived-value memo) and a genuine loop hazard in its history. Changing it needs its own measurement of that effect.

Unassigned; filed plainly for triage to grade.

Generated by Claude Code

Concentrated triage batch: finding → pm:queue + domain:ui, Task, S — ObjectMap pays JSON.stringify(rawDataConfig) on every render (plus a bare getDataConfig call in the render body) to buy an identity useMemo(() => getDataConfig(schema), [schema]) gives for free. Apply the memo shape, verify the fetch-effect identity contract with the existing tests; if schema identity is itself unstable at some call site (the reason stringify was reached for), measure and report that instead of keeping the stringify.

Generated by Claude Code

Claimed + dispatched — `domain:ui` @ objectui execution seat

PM session session_01CSoz9uGhaaSgiq3hshtN7L, branch claude/issue-6018-objectmap-dataconfig-memo.
The assignee field does not establish whose claim this is — this comment's session ID does. No earlier claim comment exists.

裁决 (binding)

ObjectMap pays JSON.stringify(rawDataConfig) on every render (plus a bare getDataConfig call in the render body) to buy an identity that useMemo(() => getDataConfig(schema), [schema]) gives for free. Apply the memo shape, and verify the fetch-effect identity contract against the existing tests.

⛔ Premise fork — the important half

If schema identity is itself unstable at some call site — which is the reason someone reached for stringify in the first place — ⛔ measure and report that instead of keeping the stringify. Do not "fix" it by leaving the stringify in place, and do not silently swap one identity hack for another. An unstable schema identity is a different and larger card.

So: before applying the memo, find out whether any caller passes a fresh schema object per render. If one does, that is a stop-and-report.

Traps

⚠️ A performance card's pin is the identity contract, not the timing. ⛔ Do not assert on render counts as a proxy for speed. The distinguishing assertion is that the fetch effect does NOT re-fire when the component re-renders with an unchanged schema, and DOES re-fire when schema changes — proven red before the fix on at least one of those directions.

⚠️ useMemo is not a semantic guarantee — React may discard the cache. If any behaviour depends on the memo holding rather than merely being faster, say so; the correct tool there is a ref or a stable key, not useMemo.

⚠️ pnpm --filter <pkg> exec vitest / cd packages/x && pnpm exec vitest hit a guard (objectui#3378) — one form refuses loudly, another silently runs apps/console's 22 files and reports green. Always run vitest from the repo root; sanity-check the file/test count.

⚠️ tsc resolves workspace deps through dist while vitest aliases to src/ — a type-check red can be unbuilt siblings, a real error, or both. Build the closure, then re-read.

Standing rules

Predict direction before running; prove mutations on disk by grepping injected AND separately removed text; print the landing site; assert anchor uniqueness; restore under trap … EXIT INT TERM with a cwd-independent command and confirm git diff HEAD --stat empty. Derive gates by enumerating each CI job's own step list. Changeset: let check-changeset-presence decide and quote its verdict line; ⛔ never major.

Collision bounds

packages/plugin-map is yours alone. ⛔ #5977 (ObjectMap.listViewMapConfigReach.test.tsx stale narration) is open in the same area and is not yours — do not fold it in; report any same-file overlap. Eight sibling agents are live elsewhere — ⛔ do not edit packages/plugin-form/**, packages/components/**, packages/plugin-list/**, packages/plugin-detail/**, packages/types/**, packages/core/**, packages/plugin-kanban/**, packages/app-shell/src/utils/**, examples/schema-catalog/**, apps/site/**.

⛔ Leave the PR draft — the PM lands it.

Generated by Claude Code

perf(plugin-map): key ObjectMap's dataConfig memo on [schema], not a …

chore(changeset): patch @object-ui/plugin-map for the dataConfig memo…

{
  "issue": 6018,
  "status": "done",
  "branch": "claude/issue-6018-objectmap-dataconfig-memo",
  "pr": "https://github.com/objectstack-ai/objectui/pull/6266",
  "premise_still_valid": true,
  "summary": "Replaced ObjectMap's `const rawDataConfig = getDataConfig(schema)` + `useMemo(() => rawDataConfig, [JSON.stringify(rawDataConfig)])` with `useMemo(() => getDataConfig(schema), [schema])` — the shape #5976 landed one line below for `mapConfig`. `getDataConfig` is a pure function of `schema` (reads exactly `data`, `staticData`, `objectName`), so `[schema]` is the whole dependency. Also refreshed `mapConfig`'s docblock, which narrated the deleted `JSON.stringify` dep key as a live neighbour. PREMISE FORK ANSWERED: `schema` identity IS unstable at one call site — `SchemaRenderer.tsx:1076`'s unmemoized `schemaForComponent = { ...evaluatedSchema, className: mergedClassName }` hands a fresh object every parent render to any node declaring `responsiveStyles` (measured: stable=false vs stable=true for a plain node). Reported, not worked around — no identity hack was swapped in. It is bounded and is NOT the loop path: it is fresh per PARENT render, while the loop the old comment guarded needs freshness on ObjectMap's OWN re-renders (setData re-renders ObjectMap only; SchemaRenderer does not re-render), so `[schema]` closes the loop. It is also not new exposure from this PR — `mapConfig` has been keyed `[schema]` since #5976 under the identical instability. It belongs to `packages/react`, outside this card's collision bounds, and is a larger card.",
  "tests": "All from the repo root (canonical invocation; `pnpm --filter … exec vitest` and package-cwd runs hit the #3378 guard). Union re-run on final commit 8311dbb0e. (1) NEW `packages/plugin-map/src/ObjectMap.dataConfigMemo.test.tsx`, 4 tests, pinning the fetch-effect identity contract via `dataSource.find` call count — the module boundary the effect crosses — never render counts as a speed proxy. REVERSE VERIFICATION run twice: once against pristine origin/main before any edit, once as an explicit revert leg on the committed tree. Mutation proven on disk both directions (injected 'JSON.stringify(rawDataConfig)' = 1, removed 'getDataConfig(schema), [schema]' = 0), restored under `trap '<git -C … checkout>' EXIT INT TERM`, `git diff HEAD --stat` empty afterwards. PREDICTED before running: identity directions green on both forms, serializer direction red. OBSERVED, matching: 'Test Files 1 failed (1) | Tests 1 failed | 3 passed (4)' with `AssertionError: expected [Function] to not throw an error but 'TypeError: Converting circular structure to JSON … property 'parent' closes the circle' was thrown`. So 3 of 4 assertions SURVIVE A REVERT (regression pins against the memo ever being re-keyed on something render-fresh — the #5976 defect one line up; also the ruling's 'verify against the existing tests' half) and 1 is genuinely red before the fix. That red is a correctness gain, not a timing proxy: `JSON.stringify` throws on unserializable input and `getDataConfig`'s passthrough branch returns the author's own `schema.data` VERBATIM (inline rows included), so a back-referencing record graph from an `$expand`-ed lookup crashed the whole map subtree from the render body. (2) `pnpm exec vitest run packages/plugin-map/src/ --maxWorkers=2` → 'Test Files 16 passed (16) | Tests 92 passed (92)' — count sanity-checked as plugin-map scope, not apps/console's 22 files. (3) `pnpm --filter @object-ui/plugin-map type-check` → exit 0, script name echoed ('tsc --noEmit && tsc -p tsconfig.test.json') so not a zero-match silent pass; dependency closure built first via `pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-map^...' build` (exit 0) so tsc read rebuilt dist/*.d.ts, not stale siblings. (4) `pnpm --filter @object-ui/plugin-map lint` → 0 errors, 126 warnings (pre-existing house style; lint.yml sets no --max-warnings). DECLARED NARROWING for repo-wide lint, with all three pieces of evidence: population came from eslint's own config resolution (`eslint .` in packages/plugin-map), file count read from `--format json` (23 files, 0 errors, 126 warnings), and `eslint.config.js` declares no `project`/`projectService` so type-aware linting is OFF — every rule is single-file AST, therefore this diff cannot move the verdict on any untouched file. Full farm left to CI. (5) Gates derived by enumerating each CI job's own step list: `node scripts/check-changeset-presence.mjs` → '✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/wild-donkeys-shake.md.'; `node scripts/check-control-bytes.mjs` → '✅ check-control-bytes: OK (scanned 5173 tracked text file(s); skipped 85 binary).'; `node scripts/check-vi-mock-specifiers.mjs` → '✅ check-vi-mock-specifiers: OK'; `node scripts/check-changeset-no-major.mjs` → '✅ No changeset declares a `major` bump.'; `node scripts/check-changeset-fixed.mjs` → '✅ All workspace packages are in the changeset fixed group.'. All exit codes captured before any pipe. Downstream consumers (PREFIX form `pnpm --filter '...@object-ui/plugin-map'`): @object-ui/console, @object-ui/site, examples/schema-catalog — all apps/examples; no exported surface moved, the change is a private render-body memo.",
  "open_questions": [
    {
      "question": "`useMemo` is not a semantic guarantee, and this memo is load-bearing rather than merely faster: React may discard the cache, at which point the fetch effect sees a fresh `dataConfig` and re-fires — and `setData` re-renders, which is exactly what made the original comment say 'prevent infinite loops'. This PR does not change that exposure (old and new form are both a `useMemo`, the new one strictly cheaper and no less safe), but it does not fix it either. Should it be fixed, and as its own card?",
      "options": [
        "A — leave as is: both forms carry the identical exposure, React discarding a memo is rare, and no reported defect traces to it. Zero cost, keeps this card narrow.",
        "B — file a follow-up to key the fetch effect on the PRIMITIVES it actually reads (`dataConfig.provider`, `dataConfig.object`) rather than on an object identity at all, so no cache has to hold for correctness. The `useMemo` then becomes a pure optimisation, which is the only thing it is contractually allowed to be.",
        "C — swap the memo for a `useRef` / stable-key form that React cannot discard. Guarantees the identity, but keeps correctness resting on identity plumbing."
      ],
      "recommendation": "B, as a separate card. It is the only option that removes the dependency on the cache rather than reinforcing it, and it is contract-first: the effect declares the values it genuinely reads instead of a proxy object whose identity it must police. C is the identity hack the ruling explicitly forbade swapping in. A is acceptable in the interim and is what this PR ships, since the exposure is unchanged by it."
    }
  ],
  "out_of_scope_findings": [
    "NOT FILED — the GitHub issue search/list API is rate-limited for this identity right now ('API rate limit already exceeded for user ID 6219465'), and the standing rule is to search before filing, so I did not file blind. Handing it to the PM to file, fully specified: `SchemaRenderer` hands a render-fresh `schema` object to any node declaring `responsiveStyles`, defeating every downstream `[schema]` memo. `packages/react/src/SchemaRenderer.tsx:1076` — `const schemaForComponent = scopeClass ? { ...evaluatedSchema, className: mergedClassName } : evaluatedSchema;` — is not memoized, so for a node with ADR-0065 scoped styles it allocates a new object on every SchemaRenderer render even when the `evaluatedSchema` memo held. MEASURED with a throwaway probe component registered through the real SchemaRenderer path (probe deleted, not committed): plain node → stable=true; node with `responsiveStyles: { large: {...} }` → stable=false. Note `hasResponsiveStyles` requires a `large`/`medium`/`small`/`xsmall` key, so a `{ base: … }` shape does NOT trigger it. Blast radius is every renderer memoizing on `[schema]`, which now includes ObjectMap's `dataConfig` and `mapConfig` plus the whole marker cascade downstream (`markers` → `filteredMarkers` → `clusteredData`/`markerBounds` → `initialViewState`). Bounded, not a loop: fresh per PARENT render only, and ObjectMap's own setState does not re-render SchemaRenderer. Likely fix is memoizing `schemaForComponent` on `[evaluatedSchema, mergedClassName]`. Belongs to `packages/react` — outside this card's collision bounds, and a larger card as the dispatch anticipated. Suggested labels: `finding`, `domain:ui`."
  ]
}

Generated by Claude Code

os-dev-report

(Supersedes the previous comment — the GitHub body sanitizer ate its leading HTML-comment marker on write, which would make it invisible to the PM's scan. Same report, marker as literal text, and the one angle-bracket fragment inside tests rewritten so it survives too.)

{
  "issue": 6018,
  "status": "done",
  "branch": "claude/issue-6018-objectmap-dataconfig-memo",
  "pr": "https://github.com/objectstack-ai/objectui/pull/6266",
  "premise_still_valid": true,
  "summary": "Replaced ObjectMap's `const rawDataConfig = getDataConfig(schema)` + `useMemo(() => rawDataConfig, [JSON.stringify(rawDataConfig)])` with `useMemo(() => getDataConfig(schema), [schema])` — the shape #5976 landed one line below for `mapConfig`. `getDataConfig` is a pure function of `schema` (reads exactly `data`, `staticData`, `objectName`), so `[schema]` is the whole dependency. Also refreshed `mapConfig`'s docblock, which narrated the deleted `JSON.stringify` dep key as a live neighbour. PREMISE FORK ANSWERED: `schema` identity IS unstable at one call site — `SchemaRenderer.tsx:1076`'s unmemoized `schemaForComponent = { ...evaluatedSchema, className: mergedClassName }` hands a fresh object every parent render to any node declaring `responsiveStyles` (measured: stable=false vs stable=true for a plain node). Reported, not worked around — no identity hack was swapped in. It is bounded and is NOT the loop path: it is fresh per PARENT render, while the loop the old comment guarded needs freshness on ObjectMap's OWN re-renders (setData re-renders ObjectMap only; SchemaRenderer does not re-render), so `[schema]` closes the loop. It is also not new exposure from this PR — `mapConfig` has been keyed `[schema]` since #5976 under the identical instability. It belongs to `packages/react`, outside this card's collision bounds, and is a larger card.",
  "tests": "All from the repo root (canonical invocation; `pnpm --filter … exec vitest` and package-cwd runs hit the #3378 guard). Union re-run on final commit 8311dbb0e. (1) NEW `packages/plugin-map/src/ObjectMap.dataConfigMemo.test.tsx`, 4 tests, pinning the fetch-effect identity contract via `dataSource.find` call count — the module boundary the effect crosses — never render counts as a speed proxy. REVERSE VERIFICATION run twice: once against pristine origin/main before any edit, once as an explicit revert leg on the committed tree. Mutation proven on disk both directions (injected 'JSON.stringify(rawDataConfig)' = 1, removed 'getDataConfig(schema), [schema]' = 0), restored under a `trap ... EXIT INT TERM` whose restore command was the cwd-independent `git -C /home/user/objectui-6018 checkout HEAD -- packages/plugin-map/src/ObjectMap.tsx`, with `git diff HEAD --stat` confirmed empty afterwards. PREDICTED before running: identity directions green on both forms, serializer direction red. OBSERVED, matching: 'Test Files 1 failed (1) | Tests 1 failed | 3 passed (4)' with `AssertionError: expected [Function] to not throw an error but 'TypeError: Converting circular structure to JSON … property 'parent' closes the circle' was thrown`. So 3 of 4 assertions SURVIVE A REVERT (regression pins against the memo ever being re-keyed on something render-fresh — the #5976 defect one line up; also the ruling's 'verify against the existing tests' half) and 1 is genuinely red before the fix. That red is a correctness gain, not a timing proxy: `JSON.stringify` throws on unserializable input and `getDataConfig`'s passthrough branch returns the author's own `schema.data` VERBATIM (inline rows included), so a back-referencing record graph from an `$expand`-ed lookup crashed the whole map subtree from the render body. (2) `pnpm exec vitest run packages/plugin-map/src/ --maxWorkers=2` → 'Test Files 16 passed (16) | Tests 92 passed (92)' — count sanity-checked as plugin-map scope, not apps/console's 22 files. (3) `pnpm --filter @object-ui/plugin-map type-check` → exit 0, script name echoed ('tsc --noEmit && tsc -p tsconfig.test.json') so not a zero-match silent pass; dependency closure built first via `pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-map^...' build` (exit 0) so tsc read rebuilt dist/*.d.ts, not stale siblings. (4) `pnpm --filter @object-ui/plugin-map lint` → 0 errors, 126 warnings (pre-existing house style; lint.yml sets no --max-warnings). DECLARED NARROWING for repo-wide lint, with all three pieces of evidence: population came from eslint's own config resolution (`eslint .` in packages/plugin-map), file count read from `--format json` (23 files, 0 errors, 126 warnings), and `eslint.config.js` declares no `project`/`projectService` so type-aware linting is OFF — every rule is single-file AST, therefore this diff cannot move the verdict on any untouched file. Full farm left to CI. (5) Gates derived by enumerating each CI job's own step list: `node scripts/check-changeset-presence.mjs` → '✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/wild-donkeys-shake.md.'; `node scripts/check-control-bytes.mjs` → '✅ check-control-bytes: OK (scanned 5173 tracked text file(s); skipped 85 binary).'; `node scripts/check-vi-mock-specifiers.mjs` → '✅ check-vi-mock-specifiers: OK'; `node scripts/check-changeset-no-major.mjs` → '✅ No changeset declares a `major` bump.'; `node scripts/check-changeset-fixed.mjs` → '✅ All workspace packages are in the changeset fixed group.'. All exit codes captured before any pipe. Downstream consumers (PREFIX form `pnpm --filter '...@object-ui/plugin-map'`): @object-ui/console, @object-ui/site, examples/schema-catalog — all apps/examples; no exported surface moved, the change is a private render-body memo.",
  "open_questions": [
    {
      "question": "`useMemo` is not a semantic guarantee, and this memo is load-bearing rather than merely faster: React may discard the cache, at which point the fetch effect sees a fresh `dataConfig` and re-fires — and `setData` re-renders, which is exactly what made the original comment say 'prevent infinite loops'. This PR does not change that exposure (old and new form are both a `useMemo`, the new one strictly cheaper and no less safe), but it does not fix it either. Should it be fixed, and as its own card?",
      "options": [
        "A — leave as is: both forms carry the identical exposure, React discarding a memo is rare, and no reported defect traces to it. Zero cost, keeps this card narrow.",
        "B — file a follow-up to key the fetch effect on the PRIMITIVES it actually reads (`dataConfig.provider`, `dataConfig.object`) rather than on an object identity at all, so no cache has to hold for correctness. The `useMemo` then becomes a pure optimisation, which is the only thing it is contractually allowed to be.",
        "C — swap the memo for a `useRef` / stable-key form that React cannot discard. Guarantees the identity, but keeps correctness resting on identity plumbing."
      ],
      "recommendation": "B, as a separate card. It is the only option that removes the dependency on the cache rather than reinforcing it, and it is contract-first: the effect declares the values it genuinely reads instead of a proxy object whose identity it must police. C is the identity hack the ruling explicitly forbade swapping in. A is acceptable in the interim and is what this PR ships, since the exposure is unchanged by it."
    }
  ],
  "out_of_scope_findings": [
    "NOT FILED — the GitHub issue search/list API is rate-limited for this identity right now ('API rate limit already exceeded for user ID 6219465'), and the standing rule is to search before filing, so I did not file blind. Handing it to the PM to file, fully specified: `SchemaRenderer` hands a render-fresh `schema` object to any node declaring `responsiveStyles`, defeating every downstream `[schema]` memo. `packages/react/src/SchemaRenderer.tsx:1076` — `const schemaForComponent = scopeClass ? { ...evaluatedSchema, className: mergedClassName } : evaluatedSchema;` — is not memoized, so for a node with ADR-0065 scoped styles it allocates a new object on every SchemaRenderer render even when the `evaluatedSchema` memo held. MEASURED with a throwaway probe component registered through the real SchemaRenderer path (probe deleted, not committed): plain node → stable=true; node with `responsiveStyles: { large: {...} }` → stable=false. Note `hasResponsiveStyles` requires a `large`/`medium`/`small`/`xsmall` key, so a `{ base: … }` shape does NOT trigger it. Blast radius is every renderer memoizing on `[schema]`, which now includes ObjectMap's `dataConfig` and `mapConfig` plus the whole marker cascade downstream (`markers` → `filteredMarkers` → `clusteredData`/`markerBounds` → `initialViewState`). Bounded, not a loop: fresh per PARENT render only, and ObjectMap's own setState does not re-render SchemaRenderer. Likely fix is memoizing `schemaForComponent` on `[evaluatedSchema, mergedClassName]`. Belongs to `packages/react` — outside this card's collision bounds, and a larger card as the dispatch anticipated. Suggested labels: `finding`, `domain:ui`."
  ]
}

Generated by Claude Code

perf(plugin-map): key ObjectMap's dataConfig memo on [schema], not a …

Navigation Menu

Repository navigation

`ObjectMap`'s `dataConfig` pays a `JSON.stringify` on every render to buy an identity that `[schema]` already gives for free #6018

What was measured

Why the serialize now looks unnecessary rather than merely costly

What has NOT been measured, and would need to be before acting

Scope note

Claimed + dispatched — `domain:ui` @ objectui execution seat

裁决 (binding)

⛔ Premise fork — the important half

Traps

Standing rules

Collision bounds

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ObjectMap's dataConfig pays a JSON.stringify on every render to buy an identity that [schema] already gives for free #6018

Description

What was measured

Why the serialize now looks unnecessary rather than merely costly

What has NOT been measured, and would need to be before acting

Scope note

Activity

claude commented on Aug 25, 2026

yinlianghui commented on Aug 25, 2026

Claimed + dispatched — domain:ui @ objectui execution seat

裁决 (binding)

⛔ Premise fork — the important half

Traps

Standing rules

Collision bounds

yinlianghui commented on Aug 25, 2026

yinlianghui commented on Aug 25, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`ObjectMap`'s `dataConfig` pays a `JSON.stringify` on every render to buy an identity that `[schema]` already gives for free #6018

Claimed + dispatched — `domain:ui` @ objectui execution seat