feat(dia.Graph): add GraphHierarchyIndex for cell embedding #3155
Conversation
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
kumilingus
force-pushed
the
feat/graph-hierarchy-index
branch
from
cfa03ed to
1bfd33d
Compare
zbynekstara
approved these changes
Jan 30, 2026
There was a problem hiding this comment.
Pull request overview
This PR introduces a new GraphHierarchyIndex that maintains an in-memory index of parent-child cell relationships based on the parent attribute. This enables querying embedded cells without reading the embeds attribute directly. Additionally, a new config.storeEmbeds option allows disabling storage of the redundant embeds attribute while maintaining full embedding functionality through the index.
Changes:
- New
GraphHierarchyIndexclass that tracks parent-child relationships using Maps and Sets for O(1) lookups - Updated
Cell.getEmbeddedCells()to use the hierarchy index instead of theembedsattribute - New
config.storeEmbedsconfiguration option (defaults totruefor backwards compatibility)
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/joint-core/src/dia/GraphHierarchyIndex.mjs | New index class that maintains parent-child relationships by listening to cell add/remove and parent change events |
| packages/joint-core/src/dia/Graph.mjs | Instantiates the hierarchyIndex alongside the existing topologyIndex |
| packages/joint-core/src/dia/Cell.mjs | Updates getEmbeddedCells to read from hierarchy index; conditionally sets embeds attribute based on config |
| packages/joint-core/src/config/index.mjs | Adds storeEmbeds configuration option with documentation |
| packages/joint-core/types/joint.d.ts | Adds TypeScript definitions for GraphHierarchyIndex class and hierarchyIndex property |
| packages/joint-core/test/jointjs/graph.js | Comprehensive test coverage for hierarchy index and storeEmbeds configuration |
| packages/joint-react/src/components/paper/tests/snapshots/paper.test.tsx.snap | Updates snapshot test (pattern ID change) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/joint-react/src/components/paper/__tests__/__snapshots__/paper.test.tsx.snap
Show resolved
Hide resolved
Add a new GraphHierarchyIndex that tracks parent-child relationships based on the `parent` attribute. This allows querying embedded cells without reading the redundant `embeds` attribute. Changes: - Add GraphHierarchyIndex class (similar to GraphTopologyIndex) - Update Cell.getEmbeddedCells() to read from index instead of `embeds` - Add config.storeEmbeds option to disable storing `embeds` attribute - Add TypeScript definitions for GraphHierarchyIndex When config.storeEmbeds is false: - The `embeds` attribute is not set on cells - The `change:embeds` events do not fire - Cell.getEmbeddedCells() still works via the hierarchy index Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use separate Maps for link and element children (links first ordering) - Use Sets for O(1) add/remove operations - Correctly distinguish numeric vs string IDs (1 !== "1") - Add test for numeric/string ID handling Co-Authored-By: Claude Opus 4.5 <[email protected]>
The hierarchy index is always in sync with the graph, so filtering out undefined values is unnecessary and could hide bugs. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
kumilingus
force-pushed
the
feat/graph-hierarchy-index
branch
from
1bfd33d to
f6d033c
Compare
…eaks Remove Map entries when their Set becomes empty after child removal. This prevents accumulation of empty Set objects in long-running apps. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zbynekstara
approved these changes
Jan 30, 2026
Sign up for free
to 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.
Summary
Add a new
GraphHierarchyIndexthat tracks parent-child relationships based on theparentattribute. This allows querying embedded cells without reading the redundantembedsattribute.Changes
GraphHierarchyIndexclass (similar toGraphTopologyIndex)Cell.getEmbeddedCells()to read from index instead ofembedsattributeconfig.storeEmbedsoption to disable storingembedsattributeGraphHierarchyIndexBehavior
Default (
config.storeEmbeds: true):embedsattribute is set,change:embedsevents fireWith
config.storeEmbeds = false:embedsattribute is not set on cellschange:embedsevents do not fireCell.getEmbeddedCells()still works via the hierarchy indexBenefits
parentattribute on child cells. The hierarchy index automatically builds the parent-child relationships without requiring theembedsarray on parent cells. This eliminates the risk of invalid states from mismatchedparent/embedsdata.Test plan
graph.hierarchyIndexconfig.storeEmbedsoption🤖 Generated with Claude Code