fix(plugin-tree): re-key ObjectTree's record-fetch effect onto dataConfig primitives (#6700) - #6701
Merged
os-sales merged 1 commit intoAug 28, 2026
Conversation
…nfig primitives (#6700) The record-fetch effect closed over `dataConfig`, a `useMemo` over the `schema` prop, whose identity `useMemo` carries no semantic guarantee for -- React may discard the cache and recompute even when `schema` is unchanged, and `getDataConfig(schema)` builds a fresh `{ provider, object }` / `{ provider, items }` wrapper object on every call. A discard alone was therefore enough to re-run the effect and issue an extra `dataSource.find` call, with nothing about the bound object actually different. Re-keys the effect onto the three primitive fields it actually reads off `dataConfig` (`provider`, and conditionally `object` / `items`) instead of the container object, mirroring the pattern already shipped for `ObjectMap`/`ObjectCalendar`/`ObjectGantt` (objectui#6592). This is the last `dataConfig`-identity dependence in the component: the schema-resolution effect was already re-keyed onto `useSettledSchema`'s primitive `schemaKey` by #6696. New pin (`ObjectTree.discardedConfigMemo.test.tsx`) simulates a discarded memo via a schema reference swap with identical primitive content (verified against this component's own `dataConfig = useMemo(..., [schema])`, which reliably recomputes on that swap) and asserts no extra `dataSource.find` call, alongside a counter-probe that a genuinely different `objectName` still refetches. Reverse-verified RED against the pre-fix source (2 calls observed vs. 1 expected) and GREEN after restoring the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_8ca04858-ea8e-5b85-9182-de59aa49e00c
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
|
os-sales
marked this pull request as ready for review
August 28, 2026 16:55
os-sales
deleted the
claude/issue-6700-objecttree-record-effect-primitives
branch
August 28, 2026 17:21
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #6700
What
packages/plugin-tree/src/ObjectTree.tsx's record-fetch effect closed overdataConfig— auseMemoover theschemaprop — as a dependency, insteadof the primitive fields it actually reads off it. This is the last
dataConfig-identity dependence left in the component: #6696 alreadyre-keyed the schema-resolution half onto
useSettledSchema's primitiveschemaKey, so this PR closes out the component (not just one effect),completing the #6592 census for
ObjectTree.Re-derived read set
Scanning the effect body directly (not the issue's suggested list, which
conflated this effect with an unrelated
headerObjectNameread atObjectTree.tsx:503-504outside this effect) — three primitives:dataConfig?.provider— gates both the'object'and'value'branches.dataConfig.object— read only inside theprovider === 'object'branch.dataConfig.items— read only inside theprovider === 'value'branch.Before / after deps
with the three derived right after the
useSettledSchemacall:Mirrors the pattern already shipped for
ObjectMap/ObjectCalendar/ObjectGantt(objectui#6592) — same naming, same shape.Discard-pin demonstration (
ObjectTree.discardedConfigMemo.test.tsx)Trigger: two schema object literals with identical primitive content but
different references, rerendered in place.
ObjectTree's owndataConfigmemo is
useMemo(() => getDataConfig(schema), [schema])— keyed on thewhole
schemaobject — so a reference swap reliably forces a recompute,which is the same "different identity, same content" observable a genuine
useMemocache discard would produce withschemaheld constant.useSettledSchema's own internal effect is keyed on the derived primitiveschemaKeystring (notschema/dataConfig), so this swap leavesobjectSchema/schemaSettledreferentially stable and can't confound theresult through that channel.
RED-pre-fix proof (mutation leg of the ablation below): reverted
ObjectTree.tsxto the pre-fixorigin/maincontent, kept the new pin,rebuilt, and ran it —
The discard-proxy test failed (an extra
dataSource.findcall fired on theschema-reference swap); the "still refetches for a genuinely different
object" counter-probe passed either way, as expected — that behavior
predates this fix.
Ablation — predicted vs. observed
findcall)expected 2 to be 1git checkout HEAD -- <path>), rebuildTests 2 passed (2)Restoration proven byte-identical:
git diff HEADempty,git hash-objectmatch (
94ba6dce7…both before and after the mutation leg). Mutationlanding on disk proven by anchored
grep -con the pre-fix deps-arraymarker (
dataConfig, dataSource, schema.filter→ 1 hit) and the post-fixprimitive marker (
dataProvider→ 0 hits) before rebuilding.Both legs matched prediction (no direction surprise, unlike #6592's
ObjectCalendartrigger — verified this component's actual[schema]-keyedmemo forces the recompute before trusting the trigger).
Gate table
All commands run from the repo root, exit captured by redirect-then-capture
(never after a pipe), at final commit
ee86dc339:pnpm --filter '@object-ui/plugin-tree^...' buildpnpm --filter '@object-ui/plugin-tree' buildpnpm --filter '@object-ui/plugin-tree' type-checkpnpm exec vitest run packages/plugin-tree/Test Files 6 passed (6)/Tests 22 passed (22)check:control-bytesnode scripts/check-control-bytes.mjs✅ OK (scanned 5554 tracked text file(s)...)--format json)pnpm exec eslint --no-inline-config --format json <2 touched files>errorCount 0both files; warnings are pre-existing (@typescript-eslint/no-explicit-any,react-hooks/exhaustive-deps,react-hooks/set-state-in-effect) — confirmed identical rule/count set against the pre-fix source, config carries noparserOptions.project/projectServiceso type-aware linting is off repo-wide and this diff cannot move any untouched file's verdictcheck:changeset-presencenode scripts/check-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed... declares 1 changeset(s)check:changeset-no-majornode scripts/check-changeset-no-major.mjs✅ No changeset declares a major bump.check:changeset-fixednode scripts/check-changeset-fixed.mjs✅ All workspace packages are in the changeset fixed group.Changeset:
patchon@object-ui/plugin-tree, percheck:changeset-presence's own verdict.Fence
packages/plugin-treeonly. Did not touchpackages/types,app-shell,plugin-designer(#6527),packages/react/src/utils/visibilityDiagnostic.ts(#6504), or
packages/react'suseSettledSchemahook (consumed as-is).Generated by Claude Code