refactor!: remove Dictionary and use Map instead
#857
Conversation
The `Dictionary` class had been introduced by mxGraph for browser without `Map` object support. The `Map` class is available in the ECMAScript version supported by maxGraph, so we can remove it. In the examples in this repository, the package size decreases by 0.7kB. BREAKING CHANGES: The `Dictionary` class has been removed. Use the `Map` class instead, which is a standard JavaScript feature.
|
""" WalkthroughThis change removes the custom Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Module
participant Map
User->>Module: Call function using key-value storage
Module->>Map: set(key, value) / get(key) / delete(key)
Map-->>Module: Return result
Module-->>User: Return processed data
Possibly related issues
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/core/src/view/plugins/SelectionHandler.ts (1)
321-330: UseMap.haswhen you just need an existence check
isCellMoving(line 803) currently returnsthis.allCells.get(cell), which relies on the truthiness of the stored value.
With aMap, the semantic “is the key present?” is clearer (and marginally faster) viahas.
No behaviour change, just crisper intent.- isCellMoving(cell: Cell) { - return this.allCells.get(cell); - } + isCellMoving(cell: Cell) { + return this.allCells.has(cell); + }packages/core/src/view/layout/HierarchicalLayout.ts (1)
149-160: Consistency ‑ considerhasfor membership checksThe three caches are now native
Maps – good.
Keep in mind thatMap.has(key)is cheaper thangetwhen you only need to know if an entry exists (e.g. ingetEdgesandgetVisibleTerminal).
Not critical, but worth a tiny micro-cleanup.packages/core/src/view/layout/SwimlaneLayout.ts (1)
492-520: Minor: preferMap.hasfor clarity & micro-perfSame comment as in
HierarchicalLayout– useterminalCache.has(edge)when you only want to skip recomputation if cached.-const term = terminalCache.get(edge); -if (term != null) return term; +if (terminalCache.has(edge)) return terminalCache.get(edge)!;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
CHANGELOG.md(1 hunks)packages/core/src/index.ts(0 hunks)packages/core/src/util/Dictionary.ts(0 hunks)packages/core/src/util/arrayUtils.ts(1 hunks)packages/core/src/util/cellArrayUtils.ts(4 hunks)packages/core/src/util/styleUtils.ts(1 hunks)packages/core/src/util/treeTraversal.ts(2 hunks)packages/core/src/view/GraphView.ts(5 hunks)packages/core/src/view/cell/CellRenderer.ts(4 hunks)packages/core/src/view/cell/CellState.ts(1 hunks)packages/core/src/view/cell/CellStatePreview.ts(4 hunks)packages/core/src/view/cell/TemporaryCellStates.ts(2 hunks)packages/core/src/view/image/ImageExport.ts(1 hunks)packages/core/src/view/layout/CompactTreeLayout.ts(1 hunks)packages/core/src/view/layout/GraphLayout.ts(1 hunks)packages/core/src/view/layout/HierarchicalLayout.ts(5 hunks)packages/core/src/view/layout/SwimlaneLayout.ts(5 hunks)packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts(4 hunks)packages/core/src/view/layout/hierarchical/GraphHierarchyModel.ts(6 hunks)packages/core/src/view/layout/hierarchical/MinimumCycleRemover.ts(1 hunks)packages/core/src/view/layout/hierarchical/SwimlaneModel.ts(5 hunks)packages/core/src/view/layout/types.ts(1 hunks)packages/core/src/view/mixins/CellsMixin.ts(5 hunks)packages/core/src/view/mixins/ConnectionsMixin.ts(1 hunks)packages/core/src/view/mixins/EdgeMixin.ts(1 hunks)packages/core/src/view/mixins/SelectionMixin.ts(1 hunks)packages/core/src/view/mixins/TerminalMixin.ts(2 hunks)packages/core/src/view/mixins/TooltipMixin.ts(1 hunks)packages/core/src/view/plugins/SelectionCellsHandler.ts(12 hunks)packages/core/src/view/plugins/SelectionHandler.ts(6 hunks)
💤 Files with no reviewable changes (2)
- packages/core/src/index.ts
- packages/core/src/util/Dictionary.ts
🧰 Additional context used
🧬 Code Graph Analysis (15)
packages/core/src/view/mixins/TerminalMixin.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/util/styleUtils.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/util/cellArrayUtils.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/mixins/ConnectionsMixin.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/mixins/EdgeMixin.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/layout/CompactTreeLayout.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/mixins/SelectionMixin.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/cell/TemporaryCellStates.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/util/treeTraversal.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/mixins/CellsMixin.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/layout/HierarchicalLayout.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/layout/hierarchical/GraphHierarchyModel.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/layout/GraphLayout.ts (1)
packages/core/src/view/layout/types.ts (1)
GraphLayoutTraverseArgs(23-29)
packages/core/src/view/layout/SwimlaneLayout.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
packages/core/src/view/GraphView.ts (1)
packages/core/src/view/cell/Cell.ts (1)
Cell(66-888)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-2022)
🔇 Additional comments (50)
CHANGELOG.md (1)
17-17: LGTM! Clear documentation of the breaking change.The changelog entry properly documents the removal of the
Dictionaryclass and directs users to the nativeMapalternative. This is appropriately categorized as a breaking change.packages/core/src/view/layout/hierarchical/MinimumCycleRemover.ts (1)
56-56: LGTM! Correct migration from Dictionary to Map.The change from
model.vertexMapper.getValues()toArray.from(model.vertexMapper.values())is semantically correct. TheArray.from()wrapper is necessary sinceMap.values()returns an iterator rather than an array.packages/core/src/view/layout/types.ts (1)
28-28: LGTM! Type change is consistent with Dictionary to Map migration.The type change from
Dictionary<Cell, boolean>toMap<Cell, boolean>maintains the same semantics while using the native JavaScript type. This aligns with the overall refactoring effort.packages/core/src/view/mixins/TerminalMixin.ts (2)
39-39: LGTM! Good migration with improved variable naming.The change from
Dictionary<Cell, boolean>toMap<Cell, boolean>is correct, and renamingdicttocoveredEntriesimproves code readability by making the purpose clearer.
54-55: LGTM! Correct method call migration from Dictionary to Map.The replacement of
dict.put()withcoveredEntries.set()correctly migrates from the custom Dictionary API to the native Map API while preserving the deduplication logic.Also applies to: 63-64
packages/core/src/view/mixins/EdgeMixin.ts (2)
394-394: LGTM! Correct type migration to native Map.The type change from
Dictionary<Cell | null, boolean>toMap<Cell | null, boolean>maintains the same semantics while using the native JavaScript type for cell tracking.
397-397: LGTM! Correct method call migration.The replacement of
dict.put(cells[i], true)withdict.set(cells[i], true)correctly migrates from the custom Dictionary API to the native Map API.packages/core/src/view/cell/TemporaryCellStates.ts (2)
38-38: LGTM: Property type correctly updated to use native Map.The type change from
Dictionary<Cell, CellState>toMap<Cell, CellState>is consistent with the broader refactor to remove the custom Dictionary class.
97-97: LGTM: Constructor correctly updated to use native Map.The initialization change from
new Dictionary()tonew Map()properly aligns with the Map type declaration above.packages/core/src/util/styleUtils.ts (2)
497-497: LGTM: Lookup map correctly updated to use native Map.The type change from
Dictionary<Cell, string[]>toMap<Cell, string[]>maintains the same functionality while using the native JavaScript Map.
504-504: LGTM: Method calls correctly updated from Dictionary API to Map API.The changes from
lookup.put(o1, p1)andlookup.put(o2, p2)tolookup.set(o1, p1)andlookup.set(o2, p2)properly use the native Map API while preserving identical functionality.Also applies to: 511-511
packages/core/src/view/mixins/TooltipMixin.ts (1)
50-50: LGTM: Overlay iteration correctly updated to use Map API.The change from
state.overlays.visit((id, shape) => ...)tostate.overlays.forEach((shape: Shape) => ...)properly uses the native Map.forEach() method. Since the callback only uses the shape value and not the key, the parameter order change doesn't affect functionality.packages/core/src/view/image/ImageExport.ts (1)
155-157: LGTM: Overlay iteration improved with optional chaining and Map API.The change from explicit null check with
visit()to optional chaining withforEach()is both more concise and correctly uses the native Map API. The functionality remains identical while being more readable.packages/core/src/view/cell/CellState.ts (1)
56-56: LGTM: Overlays property correctly updated to use native Map.The change from
Dictionary<CellOverlay, Shape>toMap<CellOverlay, Shape>withnew Map()initialization properly implements the migration to native JavaScript Map while maintaining the same generic type signature for type safety.packages/core/src/view/layout/CompactTreeLayout.ts (1)
415-415: LGTM! Dictionary correctly replaced with native Map.The refactoring successfully replaces the custom
Dictionary<Cell, string[]>with the native JavaScriptMap<Cell, string[]>while preserving the exact same caching functionality for split cell paths in edge sorting.Also applies to: 423-423, 431-431
packages/core/src/view/mixins/CellsMixin.ts (4)
460-460: LGTM! Dictionary correctly replaced with native Map in cloneCells.The refactoring properly replaces
Dictionary<Cell, boolean>withMap<Cell, boolean>for fast cell lookups during the cloning process.Also applies to: 464-464
707-707: LGTM! Dictionary correctly replaced with native Map in removeCells.The refactoring maintains the same fast lookup functionality for tracking cells to be removed, now using the native
Map<Cell, boolean>instead of the custom Dictionary.Also applies to: 710-710, 715-715
738-738: LGTM! Dictionary correctly replaced with native Map in cellsRemoved.The change properly replaces the custom Dictionary with native Map for tracking processed cells during the removal process, preserving all functionality.
Also applies to: 741-741, 808-808
1223-1223: LGTM! Dictionary correctly replaced with native Map in moveCells.The refactoring successfully updates the fast cell lookup mechanism in the move operation to use
Map<Cell, boolean>instead of the custom Dictionary.Also applies to: 1226-1226
packages/core/src/view/mixins/SelectionMixin.ts (1)
283-283: LGTM! Dictionary correctly replaced with native Map and improved naming.The refactoring successfully replaces
Dictionary<Cell, boolean>withMap<Cell, boolean>for tracking covered elements during change processing. The variable rename fromdicttocoveredElementsalso improves code readability.Also applies to: 287-287, 289-289
packages/core/src/util/arrayUtils.ts (1)
113-114: LGTM! Dictionary correctly replaced with native Map, with improved typing and naming.The refactoring successfully replaces the custom Dictionary with native
Mapfor duplicate detection. The explicit function typing (arr: any[]): any[]) and descriptive variable namecoveredEntriesare also good improvements.Also applies to: 118-118, 120-120
packages/core/src/view/mixins/ConnectionsMixin.ts (1)
375-378: LGTM! Clean Dictionary to Map migration.The changes correctly replace the custom
Dictionarywith the native JavaScriptMap:
- Type annotation properly updated to
Map<Cell, boolean>- Method call correctly changed from
put()toset()- Existing
get()calls remain unchanged since both APIs use the same methodpackages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (3)
295-309: LGTM! Correct Map initialization and method usage.The changes properly migrate from
DictionarytoMap:
- Type annotation correctly updated to
Map<GraphAbstractHierarchyCell, WeightedCellSorter>- Method call appropriately changed from
put()toset()
861-861: LGTM! Proper conversion from Dictionary.getValues() to Map.values().The change from
model.edgeMapper.getValues()toArray.from(model.edgeMapper.values())correctly handles the difference between Dictionary'sgetValues()(which returned an array) and Map'svalues()(which returns an iterator).
1034-1034: LGTM! Consistent conversion pattern applied.Both instances correctly replace
getValues()withArray.from(...values()), maintaining the same array output for the iteration logic while using the native Map API.Also applies to: 1054-1054
packages/core/src/view/plugins/SelectionCellsHandler.ts (3)
57-57: LGTM! Property type and documentation correctly updated.The property type is properly changed to
Map<Cell, Handler>and the JSDoc comment is appropriately updated to remove the outdated Dictionary reference.Also applies to: 100-100
134-134: LGTM! Comprehensive and correct API migration.All method calls are properly updated:
visit()→forEach()for iteration- Constructor changed from
new Dictionary()→new Map()The forEach method provides the same functionality as the Dictionary's visit method.
Also applies to: 152-152, 184-184, 245-245, 256-256, 267-267
162-163: LGTM! Correct handler manipulation methods.All Dictionary methods are properly migrated to Map equivalents:
put()→set()for adding entriesremove()→delete()for removing entries (with appropriateget()calls to retrieve handlers first)The pattern of getting the handler before deleting it maintains the original logic correctly.
Also applies to: 178-178, 199-199, 218-219, 231-231
packages/core/src/view/layout/GraphLayout.ts (1)
145-145: LGTM! Complete migration of traverse method to use Map.The changes correctly update the traverse method:
- JSDoc parameter documentation properly updated to reflect
Maptype- Default initialization correctly changed to
new Map()- Method call appropriately changed from
put()toset()This aligns with the
GraphLayoutTraverseArgsinterface wherevisitedis typed asMap<Cell, boolean> | null.Also applies to: 150-150, 153-153
packages/core/src/view/cell/CellRenderer.ts (5)
433-433: LGTM: Correct Map initializationThe initialization of
createdOverlaysusing nativeMapis consistent with the refactoring objective.
436-439: LGTM: Proper Map method usageThe conversion from Dictionary methods to Map methods is correctly implemented:
state.overlays.get(cellOverlay)replaces Dictionary get operationstate.overlays.delete(cellOverlay)replaces Dictionary remove operationcreatedOverlays.set(cellOverlay, shape)replaces Dictionary put operation
454-456: LGTM: Correct forEach usageThe replacement of
Dictionary.visit()withMap.forEach()maintains the same iteration semantics while using the native Map API.
1090-1090: LGTM: Consistent Map iterationThe
forEachmethod correctly replaces the previous Dictionary iteration pattern.
1464-1468: LGTM: Proper cleanup and resetThe destruction and reset of overlays using Map methods maintains the same cleanup semantics as the original Dictionary implementation.
packages/core/src/util/treeTraversal.ts (3)
114-114: LGTM: Updated documentationThe JSDoc comment correctly reflects the change from Dictionary to Map in the parameter type.
123-123: LGTM: Correct parameter type updateThe parameter type change from
Dictionary<Cell, boolean>toMap<Cell, boolean>maintains type safety while using the native Map.
129-132: LGTM: Proper Map usage in traversal logicThe conversion is correctly implemented:
new Map<Cell, boolean>()replaces Dictionary initializationvisited.set(vertex, true)replacesvisited.put(vertex, true)The traversal logic remains unchanged, ensuring behavioral compatibility.
packages/core/src/util/cellArrayUtils.ts (3)
63-67: LGTM: Correct Map initialization and usage in getTopmostCellsThe conversion from Dictionary to Map is properly implemented:
new Map<Cell, boolean>()correctly initializes the coverage trackingcoveredEntries.set(cells[i], true)replaces the Dictionary put operation
76-76: LGTM: Proper Map.get() usageThe
coveredEntries.get(parent)call correctly replaces the Dictionary get operation while maintaining the same lookup semantics.
97-102: LGTM: Consistent Map usage in getParents functionThe Map operations are correctly implemented:
new Map<Cell, boolean>()for parent tracking initializationcoveredParents.get(parent)for duplicate detectioncoveredParents.set(parent, true)for marking visited parentsThe function logic remains unchanged while using native Map.
packages/core/src/view/cell/CellStatePreview.ts (5)
25-29: LGTM: Improved type safety with CellDelta interfaceThe introduction of the
CellDeltainterface enhances type safety by explicitly defining the structure of delta objects instead of using an anonymous type. This is a good improvement alongside the Dictionary to Map conversion.
36-36: LGTM: Correct Map initializationThe constructor properly initializes the deltas property with
new Map()instead of the previous Dictionary.
45-45: LGTM: Enhanced type annotationThe property type
Map<Cell, CellDelta>provides better type safety than the previous Dictionary with anonymous object type.
71-71: LGTM: Correct Map.set() usageThe
deltas.set(state.cell, delta)call properly replaces the Dictionary put operation.
92-98: LGTM: Proper Map iteration with forEachThe conversion from
Dictionary.visit()toMap.forEach()maintains the same iteration semantics for both the translation and revalidation phases of the preview.packages/core/src/view/layout/hierarchical/SwimlaneModel.ts (1)
58-60: LGTM: Clean Dictionary to Map conversionThe refactoring correctly replaces the custom
Dictionaryclass with native JavaScriptMap:
- Property types updated to
Map<Cell, GraphHierarchyNode/Edge>- Initialization uses
new Map()constructor- Method calls properly converted:
.put()→.set(),.getValues()→Array.from(.values())The semantic behavior is preserved while gaining the benefits of native Map performance and standardization.
Also applies to: 128-133, 192-192, 241-241, 305-305
packages/core/src/view/GraphView.ts (4)
178-178: LGTM: Property and method signature updatesThe
statesproperty andsetStatesmethod correctly transition fromDictionary<Cell, CellState>toMap<Cell, CellState>, maintaining type safety while using the native Map interface.Also applies to: 312-312
948-949: LGTM: Proper Map API usage with null handlingThe
getStatemethod correctly uses:
Map.get()with nullish coalescing (?? null) to maintain the existing null return behaviorMap.set()instead of the custom.put()methodThe semantic behavior is preserved.
Also applies to: 954-954
972-972: LGTM: Array conversion for Map valuesThe
getCellStatesmethod correctly usesArray.from(this.states.values())to replace the custom.getValues()method, maintaining the same return type ofCellState[].
992-994: LGTM: Map deletion API usageThe
removeStatemethod properly uses:
Map.get()with nullish coalescing for retrievalMap.delete()instead of the custom.remove()methodThe state cleanup logic remains intact.
| this.allCells.forEach((state: CellState | null) => { | ||
| const realState = state ? this.graph.view.getState(state.cell) : null; | ||
|
|
||
| // Checks if cell was removed or replaced | ||
| if (realState !== state && state) { | ||
| state.destroy(); | ||
|
|
||
| if (realState) { | ||
| this.allCells.put(state.cell, realState); | ||
| this.allCells.set(state.cell, realState); | ||
| } else { | ||
| this.allCells.remove(state.cell); | ||
| this.allCells.delete(state.cell); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mutating Map during forEach risks skipping entries
Inside the live-preview loop you call delete / set on this.allCells while iterating the same map.
The current logic works most of the time, but per the spec keys deleted before they are reached are skipped silently, which can lead to missed clean-up if a cell is re-inserted earlier in the iteration.
Safer pattern:
- this.allCells.forEach((state) => {
- …
- if (realState) this.allCells.set(state.cell, realState);
- else this.allCells.delete(state.cell);
- });
+ const pending: [Cell, CellState | null][] = [];
+ this.allCells.forEach((state) => {
+ …
+ pending.push([state.cell, realState]); // record change
+ });
+ pending.forEach(([c, s]) => {
+ if (s) this.allCells.set(c, s);
+ else this.allCells.delete(c);
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/core/src/view/plugins/SelectionHandler.ts around lines 1081 to 1092,
the code mutates the this.allCells Map by calling set and delete inside a
forEach loop iterating over the same Map, which can cause entries to be skipped.
To fix this, avoid mutating the Map during iteration by first collecting the
keys or entries to update or delete in a separate array, then after the loop
completes, perform the set and delete operations on this.allCells. This ensures
all entries are processed without skipping due to mutation during iteration.
There was a problem hiding this comment.
I understand this remark (and all other about this topic).
The code is coming from mxGraph and the logic hasn't been updated as part of this PR. If we want to improve this, we will do it in a dedicated PR.
There was a problem hiding this comment.
@tbouffard Understood! You're absolutely right to keep this PR focused on the Dictionary → Map migration. The forEach mutation issue is indeed a separate concern that can be addressed in a future PR dedicated to improving the underlying logic.
Thanks for maintaining clear scope boundaries - it makes the refactoring much easier to review and ensures the core objective stays on track.
🐰✨ rabbit thoughtfully notes the importance of focused refactoring
✏️ Learnings added
Learnt from: tbouffard
PR: maxGraph/maxGraph#857
File: packages/core/src/view/plugins/SelectionHandler.ts:1081-1092
Timestamp: 2025-06-26T13:27:18.983Z
Learning: User tbouffard prefers to keep PRs focused on their main objective and defer implementation improvements or logic fixes to future dedicated PRs. For maxGraph, refactoring PRs like Dictionary→Map migration should not include unrelated logic improvements even when valid issues are identified in the migrated code.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/AbstractGraph.ts:492-495
Timestamp: 2025-04-24T12:34:56.726Z
Learning: PR #776 focuses on refactoring to introduce AbstractGraph as a base class and moving code from Graph to AbstractGraph. Type improvements and other refinements should be addressed in future PRs to keep this one focused on the core architectural change.
Learnt from: tbouffard
PR: maxGraph/maxGraph#593
File: .github/workflows/create-github-release.yml:42-42
Timestamp: 2025-01-28T16:22:25.804Z
Learning: In the GitHub release notes template for maxGraph, TODO comments are intentionally included to mark sections that must be reviewed by maintainers before publishing the release. These TODOs should not be removed or converted to validation steps as they are part of the release review process.
Learnt from: tbouffard
PR: maxGraph/maxGraph#740
File: packages/core/src/util/requestUtils.ts:79-106
Timestamp: 2025-03-31T08:32:17.055Z
Learning: PR #740 is focused on reorganizing code into namespaces (guiUtils and requestUtils) without changing any implementations. Implementation improvements should be suggested for future PRs.
Learnt from: tbouffard
PR: maxGraph/maxGraph#849
File: packages/html/stories/DragSource.stories.js:98-101
Timestamp: 2025-06-13T07:48:10.300Z
Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.
Learnt from: tbouffard
PR: maxGraph/maxGraph#715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the `fit` method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.
Docstrings generation was requested by @tbouffard. * #857 (comment) The following files were modified: * `packages/core/src/util/treeTraversal.ts`
|
Note Generated docstrings for this pull request at #858 |
|
tbouffard
changed the title
Dictionary and use ECMAScript Map insteadDictionary and use Map instead
1 participant
The
Dictionaryclass had been introduced by mxGraph for browser withoutMapobject support.The
Mapclass is available in the ECMAScript version supported by maxGraph, so we can remove it.In the examples in this repository, the package size decreases by 0.7kB.
BREAKING CHANGES: The
Dictionaryclass has been removed. The maxGraph API use theMapclass instead, which is a standard JavaScript feature.If your code depends on the
Dictionaryclass, you can use theMapclass instead.Notes
Closes #855
Summary by CodeRabbit
Summary by CodeRabbit
Breaking Changes
Dictionaryclass from the API. Users should now use the standard JavaScriptMapobject for similar functionality.Refactor
Dictionaryclass with the native JavaScriptMapobject across the application, including state management, overlays, cell collections, and layout caches.Map.Mapmethods.Documentation
Dictionaryclass and the associated API changes.