feat!: improve tree-shaking of EdgeStyle
#791
Conversation
Previously, `EdgeStyle` was a class with only static properties referring to all `EdgeStyleFunction` builtin implementations provided by maxGraph. This meant they were all included in the application even if only some values were used (i.e., only the edge styles registered in StyleRegistry). `EdgeStyle` is now a namespace, allowing all bundlers to properly tree-shake and only keep the edge styles actually used by the application. Note that currently this has limited impact on tree-shaking because most builtin implementations are referenced directly in the code, so they are still included in the application. In the future, these references will be removed to further reduce bundle size. The documentation now includes a new page explaining how to use and create `EdgeStyle` implementations. The documentation about the "perimeters" have been updated to have a consistent content with the new page. BREAKING CHANGES: `EdgeStyle` has been changed from a class with static properties to a namespace. This has minimal impact for most applications that only read edge style values. The breaking change only affects applications that modify EdgeStyle properties (add/update/remove values), which is no longer possible. Instead, create your own EdgeStyle implementation and register it.
WalkthroughThis change refactors the export and usage of Changes
Sequence Diagram(s)sequenceDiagram
participant UserApp
participant StyleRegistry
participant EdgeStyle (namespace)
participant CustomEdgeStyle
UserApp->>EdgeStyle (namespace): import { ElbowConnector }
UserApp->>StyleRegistry: StyleRegistry.putValue('customEdge', CustomEdgeStyle)
UserApp->>EdgeStyle (namespace): Use built-in edge styles via namespace
UserApp->>CustomEdgeStyle: Use custom edge style function directly
Assessment against linked issues
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
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:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/core/src/view/style/edge/Loop.ts (1)
26-28: Enhance JSDoc with parameter and behavior detailsThe short description is helpful, but consider adding
@paramtags for each argument (state,source,target,points,result) and a note that the function appends computed loop points toresult. This will boost IDE autocomplete and keep documentation consistent.packages/core/src/view/style/edge/TopToBottom.ts (1)
25-27: Enhance JSDoc with parameter documentationThe new summary is concise, but adding
@paramannotations forstate,source,target,points, andresultwill align this comment block with others in the codebase and improve the developer experience.packages/core/src/view/style/edge/shared.ts (1)
27-28: Consider JSDoc tag redundancy and documentation generator compatibility
You’ve markedscalePointArrayas both@privateand@internal. Depending on your documentation toolchain (e.g., TypeDoc), you may only need one of these tags—@internalis often sufficient to hide it from public docs. Verify that your chosen tag is recognized by your docs generator, and remove the redundant tag to keep the comment concise.packages/core/src/view/style/edge/Elbow.ts (1)
27-30: Add parameter annotations for consistency
The new overview is clear, but other connectors include@paramtags describing each argument. For consistency and IDE-assisted documentation, consider adding@paramannotations forstate,source,target,points, andresult.CHANGELOG.md (1)
15-16: Fix Markdown list formatting
There’s no space after the-on theEdgeStyleentry. For consistency with the other list items, add a space:--`EdgeStyle` has been changed ... +- `EdgeStyle` has been changed ...packages/website/docs/usage/edge-styles.md (2)
1-4: Fix frontmatter description grammar.The
descriptionfield currently reads “How-to use use builtin EdgeStyles…” which has a duplicate “use” and missing hyphen in “built-in”. Consider updating to:-description: How-to use use builtin EdgeStyles and create new EdgeStyles. +description: How to use built-in EdgeStyles and create new EdgeStyles.
15-17: Capitalize “Manhattan” for consistency.In “using manhattan distance as its metric,” capitalize “Manhattan” to match the proper noun.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
CHANGELOG.md(1 hunks)packages/core/src/view/AbstractGraph.ts(1 hunks)packages/core/src/view/handler/EdgeHandler.ts(1 hunks)packages/core/src/view/style/builtin-style-elements.ts(1 hunks)packages/core/src/view/style/edge/Elbow.ts(1 hunks)packages/core/src/view/style/edge/EntityRelation.ts(1 hunks)packages/core/src/view/style/edge/Loop.ts(1 hunks)packages/core/src/view/style/edge/Orthogonal.ts(1 hunks)packages/core/src/view/style/edge/Segment.ts(1 hunks)packages/core/src/view/style/edge/SideToSide.ts(1 hunks)packages/core/src/view/style/edge/TopToBottom.ts(1 hunks)packages/core/src/view/style/edge/index.ts(1 hunks)packages/core/src/view/style/edge/shared.ts(1 hunks)packages/core/src/view/style/register.ts(1 hunks)packages/html/stories/Wires.stories.js(3 hunks)packages/ts-example-selected-features/vite.config.js(1 hunks)packages/ts-example-without-defaults/vite.config.js(1 hunks)packages/ts-example/vite.config.js(1 hunks)packages/website/docs/usage/edge-styles.md(1 hunks)packages/website/docs/usage/perimeters.md(3 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/website/docs/usage/edge-styles.md
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...edgeStyle` key in a style of a specific edge as follows: ```javascript style.edgeSty...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 ESLint
packages/html/stories/Wires.stories.js
[error] 156-156: 'WireConnector' is not defined.
(no-undef)
[error] 611-611: 'WireConnector' is not defined.
(no-undef)
[error] 702-702: 'WireConnector' is not defined.
(no-undef)
🔇 Additional comments (27)
packages/ts-example-without-defaults/vite.config.js (1)
30-30: Updated chunk size warning limit
The warning limit was decreased from 331 to 330 to align with similar example projects. This is a straightforward configuration change and looks correct.packages/ts-example-selected-features/vite.config.js (1)
30-30: Adjusted chunk size warning threshold
Lowering the warning limit from 382 to 381 matches the coordinated updates across example configs. No issues detected.packages/core/src/view/AbstractGraph.ts (1)
42-42: ImportEdgeStylefrom new namespace
The import path was updated to point atstyle/builtin-style-elements, reflecting the refactor ofEdgeStylefrom a class to a namespace. Ensure that all referred members onEdgeStyle(e.g.,.Loop,.ElbowConnector) are present in the aggregated namespace export.packages/core/src/view/style/edge/SideToSide.ts (1)
25-27: Added JSDoc forSideToSide
The new comment “Implements a horizontal elbow edge.” improves clarity and maintains consistency with other edge style modules.packages/core/src/view/handler/EdgeHandler.ts (1)
48-48: UpdatedEdgeStyleimport in handler
Aligned this import with the new namespace-based exports. Confirm thatEdgeHandlerlogic continues to correctly reference all needed style functions onEdgeStyle.packages/core/src/view/style/register.ts (1)
17-17: Consolidated imports improve tree-shaking and maintainabilityCombining
EdgeStyle,EdgeMarker, andPerimeterinto a single import from'./builtin-style-elements'aligns with the new namespace-based exports and simplifies the code. This will help bundlers drop unused symbols more effectively.packages/core/src/view/style/edge/Orthogonal.ts (1)
151-151: JSDoc consolidation maintains clarityThe merged
@param resultdescription accurately conveys that this array holds the computed edge points. Ensure that adjacent parameters (state,sourceScaled,targetScaled,controlHints) remain documented in the surrounding comments for full coverage.packages/core/src/view/style/builtin-style-elements.ts (1)
17-22: Accurate and concise namespace documentation
Great addition of a JSDoc comment and@category EdgeStyleon theEdgeStylenamespace export. This will help users discover and understand the purpose of the namespace when browsing the API docs.packages/website/docs/usage/perimeters.md (4)
3-3: Good description enhancement!The updated description clearly communicates that this documentation covers both builtin and custom perimeters, making it more obvious to readers what they can expect to learn.
17-19: Improved precision in perimeter configuration description.The revised description more accurately describes where perimeters are configured - "within the style properties of the Cell that relates to the Vertex" instead of "within the vertex style", providing better technical precision.
23-26: Better formatting using callout blocks.Reformatting the note into a proper callout block improves readability and draws appropriate attention to this important information about StyleRegistry registration.
125-163: Excellent new "Custom Perimeter" section!This comprehensive new section significantly enhances the documentation by providing:
- Clear instructions for creating custom perimeter functions
- The proper function signature with typed parameters
- Step-by-step guidelines for registering custom perimeters
- Examples for using custom perimeters with both individual vertices and globally
This addition makes the documentation more complete and actionable for developers.
packages/core/src/view/style/edge/EntityRelation.ts (1)
27-43: Great JSDoc comment addition!This comprehensive JSDoc block significantly improves the code's maintainability by:
- Explaining the purpose of the EntityRelation edge style
- Detailing how the function operates on the result array
- Clarifying the role of each parameter
- Describing the expected behavior for terminal points
This documentation enhancement aligns perfectly with the PR's goal of improving the structure and documentation of edge styles.
packages/html/stories/Wires.stories.js (1)
702-702: Registration of WireConnector correctly updated.The registration of the custom edge style has been correctly updated to match the new pattern, using the direct function reference instead of accessing it through the EdgeStyle class.
🧰 Tools
🪛 ESLint
[error] 702-702: 'WireConnector' is not defined.
(no-undef)
packages/core/src/view/style/edge/index.ts (1)
19-26: Excellent restructuring to support tree-shaking!This change perfectly implements the core PR objective by:
- Replacing the static-only class with individual named exports
- Enabling bundlers to tree-shake unused edge styles
- Providing cleaner, more modular access to edge style functions
- Maintaining the aliasing of OrthogonalConnector as OrthConnector for backward compatibility
This structural change will help reduce bundle sizes when only a subset of edge styles is used.
packages/website/docs/usage/edge-styles.md (12)
8-11: Great: TypeScript/JavaScript information.The info block clearly indicates language usage and guides users to adapt examples between TypeScript and JavaScript.
21-24: Note block is clear.The registration note and link to Global Configuration effectively direct users to related documentation.
29-31: Info block on CellStateStyle.edgeStyle is helpful.This guidance directs users to detailed API docs for edgeStyle.
33-34: Introduction of EdgeStyle namespace is clear.The description succinctly explains how the namespace keys map to style properties.
35-39: ElbowEdgeStyle example is correct.The code snippet clearly demonstrates setting the
elbowEdgeStylekey.
41-45: Tip block is informative.The note on
CellStateStyle.edgeStyletype helps users understand type constraints.
66-77: Custom EdgeStyle example is clear.The TypeScript snippet properly demonstrates implementing a custom style function.
79-82: Registration example is correct.The
StyleRegistry.putValuecall clearly shows how to register the custom style.
87-90: Usage example is correct.This snippet demonstrates setting the custom style for an individual edge.
92-94: Verify API reference consistency.The doc mentions both
CellState.edgeStyleandCellStateStyle.edgeStylein different places. Please verify the correct interface/type name and make it consistent throughout.
95-100: Global custom EdgeStyle application snippet is clear.The example correctly shows setting the default edge style for all edges.
102-105: No action needed.This final snippet aligns with the previous usage examples; the static analysis hint about a missing comma appears to be a false positive.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...edgeStyle` key in a style of a specific edge as follows: ```javascript style.edgeSty...(AI_HYDRA_LEO_MISSING_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/website/docs/usage/edge-styles.md (5)
3-4: Fix duplicate wording in the description.The front-matter
descriptioncurrently reads “How-to use use builtin EdgeStyles and create new EdgeStyles.”
Consider removing the extra “use” and hyphenating “built-in”:- description: How-to use use builtin EdgeStyles and create new EdgeStyles. + description: How-to use built-in EdgeStyles and create new EdgeStyles.
8-11: Refine the TypeScript/JavaScript admonition.The info block could be made more concise and fix minor punctuation:
- :::info - The code examples provided on this page are written in `TypeScript`. - Don't forget to adapt them if you use `JavaScript`. - ::: + :::info + The examples use `TypeScript`; adapt them if you write `JavaScript`. + :::
33-39: Clarify connector name vs. registry key.The text says “sets the edge style to
ElbowConnector” but the example uses the'elbowEdgeStyle'key. To avoid confusion, align the terminology:- The following example sets the edge style to `ElbowConnector` which is registered by default under the `elbowEdgeStyle` key: + The following example uses the built-in ElbowConnector (registered under the `elbowEdgeStyle` key):
80-83: Include necessary imports for registration snippet.The example registers
MyStylewithout showing imports. For completeness, add:+ import { StyleRegistry } from '@maxgraph/core'; + import type { EdgeStyleFunction } from '@maxgraph/core';above the registration call.
89-90: Remove duplicate usage example.The custom EdgeStyle usage is shown twice (lines 89–90 and 104–105). Consider consolidating one of these to keep the guide concise.
Also applies to: 104-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/html/stories/Wires.stories.js(3 hunks)packages/website/docs/usage/edge-styles.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/html/stories/Wires.stories.js
🧰 Additional context used
🧠 Learnings (1)
packages/website/docs/usage/edge-styles.md (2)
Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:32:14.559Z
Learning: In the EdgeStyles documentation, the sentence "A perimeter is a function matching the `EdgeStyleFunction` type" contains an incorrect term - "perimeter" should be replaced with "EdgeStyle" or "custom EdgeStyle" as the document is specifically about EdgeStyles.
Learnt from: tbouffard
PR: maxGraph/maxGraph#791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:30:18.658Z
Learning: In the EdgeStyles documentation, tbouffard prefers the existing text "A perimeter is a function matching the `EdgeStyleFunction` type" over suggestions that might create redundancy, even if terminology might be technically misaligned.
🪛 LanguageTool
packages/website/docs/usage/edge-styles.md
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...edgeStyle` key in a style of a specific edge as follows: ```javascript style.edgeSty...
(AI_HYDRA_LEO_MISSING_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (windows-2022)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build
🔇 Additional comments (2)
packages/website/docs/usage/edge-styles.md (2)
66-77: Verify return semantics of the custom EdgeStyle function.The snippet pushes a point into
resultbut does not explicitly return it. Please confirm whether theEdgeStyleFunctioncontract requiresreturn result;or relies on in-place mutation.
102-102: Skip false-positive punctuation warning.The static analysis tool flagged a missing comma here, but the sentence flows correctly without it.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...edgeStyle` key in a style of a specific edge as follows: ```javascript style.edgeSty...(AI_HYDRA_LEO_MISSING_COMMA)
|
Previously,
EdgeStylewas a class with only static properties referring to allEdgeStyleFunctionbuiltin implementations provided by maxGraph.This meant they were all included in the application even if only some values were used (i.e., only the edge styles registered in StyleRegistry).
EdgeStyleis now a namespace, allowing all bundlers to properly tree-shake and only keep the edge styles actually used by the application.Note that currently this has limited impact on tree-shaking because most builtin implementations are referenced directly in the code, so they are still included in the application.
In the future, these references will be removed to further reduce bundle size.
The documentation now includes a new page explaining how to use and create
EdgeStyleimplementations.The documentation about the "perimeters" have been updated to have a consistent content with the new page.
BREAKING CHANGES:
EdgeStylehas been changed from a class with static properties to a namespace.This has minimal impact for most applications that only read edge style values.
The breaking change only affects applications that modify EdgeStyle properties (add/update/remove values), which is no longer possible.
Instead, create your own EdgeStyle implementation and register it.
Notes
Closes #759
Impact on application size
Note
JS examples use Webpack, TS examples use Vite (Vite/Rollup does better tree-shaking!)
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Refactor
Style
Chores