refactor(types): simplify imports of the `Graph` types by tbouffard · Pull Request #743 · maxGraph/maxGraph · GitHub
Skip to content

refactor(types): simplify imports of the Graph types#743

Merged
tbouffard merged 3 commits into
mainfrom
refactor/types_remove_extra_cast_to_Graph
Apr 1, 2025
Merged

refactor(types): simplify imports of the Graph types#743
tbouffard merged 3 commits into
mainfrom
refactor/types_remove_extra_cast_to_Graph

Conversation

@tbouffard

@tbouffard tbouffard commented Apr 1, 2025

Copy link
Copy Markdown
Member

Remove extra cast to Graph and handle nullish values when needed.
Use type in imports when possible and remove unused imports.
Improve JSDoc in various classes.

Summary by CodeRabbit

  • Refactor & Chore
    • Streamlined internal code for improved maintainability and consistency by converting standard imports to type-only imports for various entities.
    • Applied optimizations that can reduce bundle size and simplify type handling.
  • Documentation
    • Refined inline comments and documentation for enhanced clarity, including the use of {@link} tags for improved usability in generated documentation.

These internal improvements do not alter any user-facing functionality, ensuring a seamless experience while setting the stage for future enhancements.

Remove extra cast to Graph and handle nullish values when needed.
Use type in imports when possible and remove unused imports.
Improve JSDoc in various classes.
@tbouffard tbouffard added the refactor Code refactoring label Apr 1, 2025
@coderabbitai

coderabbitai Bot commented Apr 1, 2025

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
packages/core/src/view/layout/StackLayout.ts (1)

48-52: Optional: Simplify Nullish Checks with the Nullish Coalescing Operator
In the constructor (lines 48–52), you’re using ternary expressions to handle nullish values (e.g., horizontal != null ? horizontal : true). For improved readability and conciseness, consider using the nullish coalescing operator (??). For example, you could refactor as follows:

-    this.horizontal = horizontal != null ? horizontal : true;
-    this.spacing = spacing != null ? spacing : 0;
-    this.x0 = x0 != null ? x0 : 0;
-    this.y0 = y0 != null ? y0 : 0;
-    this.border = border != null ? border : 0;
+    this.horizontal = horizontal ?? true;
+    this.spacing = spacing ?? 0;
+    this.x0 = x0 ?? 0;
+    this.y0 = y0 ?? 0;
+    this.border = border ?? 0;

This change is optional but could enhance clarity.

packages/core/src/view/other/Outline.ts (1)

30-30: Consider using type-only import for Graph

To be consistent with the PR objectives, consider changing this import to a type-only import.

-import { Graph } from '../Graph';
+import type { Graph } from '../Graph';
packages/core/src/view/animate/Effects.ts (1)

127-130: JSDoc Type Annotation Consistency
Consider updating the parameter description for cell in the JSDoc (currently shown as <Cell>) to a more consistent format such as {@link Cell}. This will improve the clarity of the generated documentation and help maintain consistency across the codebase.

packages/core/src/view/plugins/PanningHandler.ts (1)

432-436: Enhanced JSDoc for panGraph method.
The documentation now uses the {@link graph} tag which improves the quality and navigability of generated docs. Please verify that the link resolves correctly in your documentation system.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 59d8f89 and 0652f94.

📒 Files selected for processing (55)
  • packages/core/src/editor/EditorToolbar.ts (1 hunks)
  • packages/core/src/serialization/codecs/GraphCodec.ts (0 hunks)
  • packages/core/src/util/Clipboard.ts (1 hunks)
  • packages/core/src/util/gestureUtils.ts (2 hunks)
  • packages/core/src/util/printUtils.ts (1 hunks)
  • packages/core/src/util/treeTraversal.ts (2 hunks)
  • packages/core/src/util/xmlUtils.ts (1 hunks)
  • packages/core/src/view/GraphSelectionModel.ts (1 hunks)
  • packages/core/src/view/animate/Effects.ts (1 hunks)
  • packages/core/src/view/animate/Morphing.ts (1 hunks)
  • packages/core/src/view/cell/CellHighlight.ts (1 hunks)
  • packages/core/src/view/cell/CellMarker.ts (1 hunks)
  • packages/core/src/view/cell/CellState.ts (2 hunks)
  • packages/core/src/view/cell/CellStatePreview.ts (1 hunks)
  • packages/core/src/view/cell/CellTracker.ts (1 hunks)
  • packages/core/src/view/cell/TemporaryCellStates.ts (1 hunks)
  • packages/core/src/view/cell/VertexHandle.ts (1 hunks)
  • packages/core/src/view/event/InternalEvent.ts (1 hunks)
  • packages/core/src/view/handler/ConstraintHandler.ts (1 hunks)
  • packages/core/src/view/handler/KeyHandler.ts (6 hunks)
  • packages/core/src/view/handler/VertexHandler.ts (7 hunks)
  • packages/core/src/view/image/ImageExport.ts (0 hunks)
  • packages/core/src/view/layout/CircleLayout.ts (1 hunks)
  • packages/core/src/view/layout/CompactTreeLayout.ts (1 hunks)
  • packages/core/src/view/layout/CompositeLayout.ts (1 hunks)
  • packages/core/src/view/layout/EdgeLabelLayout.ts (1 hunks)
  • packages/core/src/view/layout/FastOrganicLayout.ts (1 hunks)
  • packages/core/src/view/layout/GraphLayout.ts (1 hunks)
  • packages/core/src/view/layout/HierarchicalLayout.ts (1 hunks)
  • packages/core/src/view/layout/LayoutManager.ts (1 hunks)
  • packages/core/src/view/layout/ParallelEdgeLayout.ts (1 hunks)
  • packages/core/src/view/layout/PartitionLayout.ts (1 hunks)
  • packages/core/src/view/layout/RadialTreeLayout.ts (1 hunks)
  • packages/core/src/view/layout/StackLayout.ts (1 hunks)
  • packages/core/src/view/layout/SwimlaneLayout.ts (1 hunks)
  • packages/core/src/view/layout/SwimlaneManager.ts (1 hunks)
  • packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (1 hunks)
  • packages/core/src/view/mixins/PanningMixin.ts (1 hunks)
  • packages/core/src/view/other/AutoSaveManager.ts (2 hunks)
  • packages/core/src/view/other/DragSource.ts (1 hunks)
  • packages/core/src/view/other/Guide.ts (1 hunks)
  • packages/core/src/view/other/Multiplicity.ts (1 hunks)
  • packages/core/src/view/other/Outline.ts (8 hunks)
  • packages/core/src/view/other/PanningManager.ts (1 hunks)
  • packages/core/src/view/other/PrintPreview.ts (1 hunks)
  • packages/core/src/view/plugins/CellEditorHandler.ts (2 hunks)
  • packages/core/src/view/plugins/ConnectionHandler.ts (1 hunks)
  • packages/core/src/view/plugins/FitPlugin.ts (1 hunks)
  • packages/core/src/view/plugins/PanningHandler.ts (1 hunks)
  • packages/core/src/view/plugins/PopupMenuHandler.ts (1 hunks)
  • packages/core/src/view/plugins/RubberBandHandler.ts (1 hunks)
  • packages/core/src/view/plugins/SelectionCellsHandler.ts (1 hunks)
  • packages/core/src/view/plugins/SelectionHandler.ts (3 hunks)
  • packages/core/src/view/plugins/TooltipHandler.ts (1 hunks)
  • packages/core/src/view/undoable_changes/CurrentRootChange.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • packages/core/src/serialization/codecs/GraphCodec.ts
  • packages/core/src/view/image/ImageExport.ts
🔇 Additional comments (98)
packages/core/src/view/layout/FastOrganicLayout.ts (1)

20-21: Refined Type-Only Imports for Improved Clarity and Efficiency

The changes correctly convert the Graph and Cell imports to type-only imports. This aligns with the PR objectives by ensuring that these imports are used solely for type-checking without adding unnecessary runtime dependencies. The modification enhances code clarity and maintainability without affecting execution.

packages/core/src/view/plugins/SelectionCellsHandler.ts (2)

24-24: Optimize Graph Import with Type-Only Syntax
Changing the import of Graph to a type-only import clarifies that it is used solely for type-checking and avoids including any runtime value. This improves the code's clarity and reduces unnecessary code inclusion.


27-27: Refine GraphPlugin Import as Type-Only
Updating the GraphPlugin import to use type-only syntax aligns with the project's effort to simplify type imports, ensuring that it's used only for type-checking and not as a runtime dependency.

packages/core/src/view/layout/CircleLayout.ts (1)

20-21: Excellent conversion to type-only imports.

Converting the imports for Graph and Cell to type-only imports aligns perfectly with the PR objectives. This change clarifies that these imports are solely for type-checking, thereby removing unnecessary runtime dependencies. This approach not only improves code clarity but can also have a positive impact on the final bundle size.

packages/core/src/view/other/PanningManager.ts (1)

25-25: Refined Type-Only Import for Graph

Switching to a type-only import for Graph clearly indicates that it is used exclusively for type-checking purposes. This change improves readability and prevents the unnecessary inclusion of the Graph module in the runtime bundle. Excellent alignment with the project's refactoring objectives.

packages/core/src/view/mixins/PanningMixin.ts (1)

21-21: Simplify Import with import type
The update to import Graph as a type (using import type { Graph } from '../Graph';) cleanly communicates that Graph is used solely for type-checking. This not only reduces potential runtime overhead but also helps avoid circular dependency issues. This change is in line with the PR objectives of simplifying imports and enhancing type usage.

packages/core/src/view/plugins/RubberBandHandler.ts (1)

33-33: Type-only Import for Graph Applied

The change on line 33 converts the standard import of Graph to a type-only import, ensuring that Graph is used exclusively for type-checking without being included as a runtime dependency. This aligns with TypeScript best practices to reduce unnecessary runtime overhead and improve code clarity.

packages/core/src/view/layout/HierarchicalLayout.ts (1)

28-29: Import paths simplified and converted to type-only imports.

The changes correctly convert standard imports to type-only imports for Graph and Cell entities, which is appropriate since they're used exclusively for type annotations in this file. The import paths have also been simplified from nested paths to more direct ones.

packages/core/src/view/other/PrintPreview.ts (1)

27-29: Type-Only Imports for Clarity and Efficiency

Changing these imports to use the import type syntax clearly signals that Graph, CellState, and Cell are used solely for type-checking. This change improves bundle size and prevents accidental runtime inclusion of unused code. The modification aligns well with the project's overall effort to simplify type imports while enhancing code clarity.

packages/core/src/view/undoable_changes/CurrentRootChange.ts (3)

17-17: LGTM: Type-only import for GraphView.

Converting to a type-only import is appropriate since GraphView is only used for type annotations in this file, not as a value. This helps with tree-shaking and clarifies the intent.


20-20: LGTM: Type-only import for Cell.

Converting to a type-only import is appropriate since Cell is only used for type annotations in this file, not as a value. This is consistent with best practices for TypeScript.


60-60: LGTM: Simplified type assertion.

Removing the unnecessary type casting to Graph improves code readability while maintaining the same functionality. The TypeScript compiler can correctly infer the type of this.view.graph without explicit assertion.

packages/core/src/view/cell/CellStatePreview.ts (1)

21-24: Refactor to Use Type-Only Imports
The changes to convert the imports for CellState, Cell, Graph, and GraphView to type-only imports are correctly applied. This aligns with the PR objective to simplify and clarify the use of types in runtime files while reducing unnecessary dependencies. Please ensure that these types are not used for any runtime value access elsewhere in the codebase.

packages/core/src/view/other/AutoSaveManager.ts (2)

21-21: Good use of type-only import.

Converting to a type-only import is appropriate here since Graph is only used for type annotations and not at runtime.


165-165: Improved JSDoc documentation.

The change from <graph> to {@link graph} follows standard JSDoc practices and enhances documentation by providing proper linking to the referenced property.

packages/core/src/view/cell/CellTracker.ts (4)

21-22: Use of Type-Only Imports is Correct
The changes converting the imports for Graph and Cell to type-only imports are appropriate since these entities are used solely for type annotations. This aligns perfectly with the PR objectives of simplifying imports and eliminating unnecessary runtime dependencies.


19-20: Overall Import Consistency and Clarity
The rest of the import section continues to use import type where applicable (e.g., for ColorValue), ensuring consistency across the file. This enhances code clarity and prevents unintentional runtime inclusion of type-only modules.

Also applies to: 23-25


80-87: Runtime Usage Considerations
The constructor accepts a Graph instance and passes it to the superclass as well as using it to call addMouseListener. Since the type-only import is only erased during compilation, runtime behavior remains unaffected provided that the module that defines Graph is imported elsewhere when necessary. Please verify that elsewhere in the project the actual runtime dependency of Graph is correctly resolved.


1-17: File Documentation and Licensing Intact
The header comment and export declaration remain unchanged. The licensing information and documentation are clear, which ensures maintainability and clear compliance with the project norms.

Also applies to: 134-135

packages/core/src/view/layout/StackLayout.ts (1)

21-22: Refined Type Imports
The update to use type-only imports for Graph and Cell (lines 21–22) is correctly implemented. This change clearly indicates that these imports are solely used for type-checking, reducing potential runtime overhead and clarifying intent.

packages/core/src/view/other/Outline.ts (6)

146-148: Improved null safety with optional chaining

The code now uses optional chaining operator (?.) to safely call methods on the outline object, preventing potential null reference errors.


172-173: Good use of null safety pattern

Extracting this.outline to a local variable and using optional chaining improves robustness when handling events.


177-179: Consistent null-safe pattern applied

The use of optional chaining here matches the pattern used elsewhere in the class, ensuring consistent handling of potentially null references.


378-394: Well-implemented null safety in createSizer

The code now properly handles cases where outline might be null using both optional chaining and the logical AND shorthand (&&) for assignment. This is a good pattern for ensuring type safety.


627-643: Good null check before using outline

The condition now explicitly checks that outline exists before using it, which prevents potential runtime errors if the outline is not initialized.


717-720: Comprehensive null safety improvements

The refactored condition with explicit null checking for outline makes the code more robust, preventing potential runtime errors during mouse interaction events.

packages/core/src/view/other/DragSource.ts (1)

43-44: Refactor to use type-only imports for Graph and Cell.

Changing these imports to use the import type syntax makes it clear that Graph and Cell are only used for type checking and not at runtime. This helps reduce unnecessary runtime dependencies and aligns with our objective of simplifying type imports.

packages/core/src/view/layout/RadialTreeLayout.ts (1)

24-25: Refactor to Use Type-Only Imports for Clarity

By updating the imports from runtime imports to type-only imports:

  • import Cell from '../cell/Cell';import type Cell from '../cell/Cell'; (line 24)
  • import { Graph } from '../Graph';import type { Graph } from '../Graph'; (line 25)

you clearly signal that these entities are used exclusively for type annotations. This refactoring improves readability, reduces potential runtime overhead, and aligns with the PR objectives of simplifying type usage. Ensure that any runtime behaviors leveraging these types (e.g., methods invoked on this.graph) have an alternative source if needed, but as it appears, the runtime instances are passed in and managed elsewhere.

packages/core/src/view/layout/SwimlaneManager.ts (2)

22-22: Utilize type-only import for Graph
Changing from a standard import to a type-only import clarifies that Graph is used solely for type checking. This tweak helps prevent unintended runtime inclusion and can contribute to a lighter bundle.


24-24: Utilize type-only import for Cell
The updated import now specifies Cell strictly as a type. This ensures that Cell isn’t bundled at runtime, thus improving clarity and maintainability in the codebase.

packages/core/src/view/plugins/PopupMenuHandler.ts (2)

23-23: Good Use of Type-Only Import for Graph.
Changing the import to import type { Graph } from '../Graph'; clearly signals that Graph is used solely as a type. This helps reduce the runtime footprint by eliminating unused runtime imports. Please confirm that no runtime accesses (such as creating instances or accessing static members) of Graph are needed within this file.


25-25: Correctly Switching to a Type-Only Import for GraphPlugin.
Updating the import to import type { GraphPlugin } from '../../types'; cleanly separates type information from runtime code. Since GraphPlugin is only used for type annotations (e.g., as an interface implemented by the class), this change is appropriate and improves both clarity and potential bundle size.

packages/core/src/view/cell/TemporaryCellStates.ts (1)

21-24: Utilize type-only imports for clarity and efficiency.
The conversion of the standard imports to import type for GraphView, Cell, CellState, and Shape is well executed. This change clearly communicates that these imports are used solely for type checking and are not needed at runtime, aligning perfectly with the PR objectives of simplifying type usage and reducing unnecessary imports.

packages/core/src/view/layout/SwimlaneLayout.ts (1)

29-30: Use of Type-Only Imports for Clarity and Bundle Optimization
Converting the standard imports for Graph and Cell to type-only imports clarifies that these are used solely for type checking. This change helps reduce potential runtime overhead and bundle size.
Please verify that no runtime functionality is required from these modules in this file, as type-only imports will be erased in the transpiled output.

packages/core/src/view/layout/LayoutManager.ts (2)

29-29: Conversion to type-only import for Cell
The conversion of the Cell import to a type-only import clarifies that it is used solely for type-checking, thereby avoiding any unintended runtime inclusion. This aligns well with our objective of simplifying the type imports and reducing bundle size.


32-32: Conversion to type-only import for Graph
Changing the Graph import to a type-only import ensures that it is used only for type annotations. Please double-check that Graph is not utilized as a runtime value anywhere in this file (e.g., for instance creation) to avoid potential runtime issues.

packages/core/src/view/animate/Effects.ts (1)

25-26: Use of Type-only Imports
Switching to import type for both the Graph and Cell imports clarifies that these are used solely for type-checking. This enhances readability and ensures that the runtime bundle is not impacted by these imports.

packages/core/src/view/plugins/FitPlugin.ts (1)

18-18: Type-only import update is correct.
Changing the regular import of Graph into a type-only import clarifies that this dependency is used only for type annotations, which aligns with the PR objectives.

packages/core/src/util/printUtils.ts (1)

23-23: Consistent use of type-only import for Graph.
The conversion to import type { Graph } from '../view/Graph'; ensures that Graph is only used for type-checking, reducing any runtime overhead. This is consistent with similar changes across the codebase.

packages/core/src/view/other/Multiplicity.ts (1)

20-21: Refactored type-only imports for Cell and Graph are appropriate.
The updates using import type Cell from '../cell/Cell'; and import type { Graph } from '../Graph'; clearly signal that these dependencies are solely for type information. This avoids unnecessary runtime imports and improves code clarity.

packages/core/src/view/layout/hierarchical/CoordinateAssignment.ts (1)

31-31: Type-only import for Graph is applied correctly.
Using import type { Graph } from '../../../view/Graph'; in this layout module is in line with the PR’s goal of simplifying type imports, while leaving runtime functionality intact.

packages/core/src/editor/EditorToolbar.ts (1)

27-28: Updated imports to type-only for Cell and Graph are spot on.
Changing the imports to

import type Cell from '../view/cell/Cell';
import type { Graph } from '../view/Graph';

clearly distinguishes between compile-time types and runtime values. This update aids in maintainability and reduces potential runtime bloat.

packages/core/src/view/GraphSelectionModel.ts (1)

20-21: Good use of type-only imports

Converting standard imports to type-only imports is appropriate here since Graph and Cell are only used for type annotations in this file, not as runtime values. This change aligns with best practices by explicitly signaling the import's purpose and potentially reducing bundle size.

packages/core/src/view/cell/CellState.ts (2)

21-22: Good use of type-only imports

Converting standard imports to type-only imports for Cell and GraphView is appropriate since these are only used for type annotations in this file, not as runtime values.


380-380: Good removal of unnecessary type casting

The removal of the explicit type cast to Graph simplifies the code while maintaining type safety. TypeScript can infer the correct type here, making the previous cast redundant.

packages/core/src/view/layout/ParallelEdgeLayout.ts (1)

22-23: Good use of type-only imports

Converting standard imports to type-only imports for Graph and Cell is appropriate as these are only used for type annotations in this file. This change improves code clarity and follows TypeScript best practices.

packages/core/src/view/other/Guide.ts (1)

22-25: Good use of type-only imports

Converting standard imports to type-only imports for CellState and Graph is appropriate as these are only used for type annotations in this file. This pattern is consistent with the changes in other files and aligns with the PR's objective of simplifying type imports.

packages/core/src/view/handler/ConstraintHandler.ts (2)

34-34: Use type-only import for Graph.

Changing the regular import into a type-only import for Graph ensures that it is used solely for type annotations (e.g. in the graph: Graph; property and constructor parameter). This aligns with the PR objective of simplifying type imports without affecting runtime values.


39-39: Use type-only import for Cell.

Updating the import for Cell to a type-only import clarifies that its usage is strictly for type-checking. Verify that no runtime functionality depends on a value import from this module.

packages/core/src/util/Clipboard.ts (1)

19-20: Convert runtime imports to type-only for Cell and Graph.

The changes converting

import Cell from '../view/cell/Cell';
import { Graph } from '../view/Graph';

to type-only imports

import type Cell from '../view/cell/Cell';
import type { Graph } from '../view/Graph';

ensure that Cell and Graph are used solely for type annotations. This refactoring directly supports improved type safety and cleaner runtime bundles.

packages/core/src/view/layout/EdgeLabelLayout.ts (1)

22-23: Adopt type-only imports for Cell and Graph.

The update from standard imports to type-only imports for both Cell and Graph here is consistent with the overall PR refactoring strategy. This change enhances clarity by making it explicit that these imports are only used in type annotations.

packages/core/src/view/layout/GraphLayout.ts (1)

23-24: Refactor to type-only imports for Graph and Cell.

Shifting to type-only imports:

-import { Graph } from '../Graph';
-import Cell from '../cell/Cell';
+import type { Graph } from '../Graph';
+import type Cell from '../cell/Cell';

ensures these entities are used solely for compile-time type checking. This assists in reducing the runtime bundle size and upholds the PR’s objective for clear type handling.

packages/core/src/view/cell/CellHighlight.ts (1)

27-28: Utilize type-only imports for CellState and Graph.

By changing the imports from:

-import CellState from './CellState';
-import { Graph } from '../Graph';
+import type CellState from './CellState';
+import type { Graph } from '../Graph';

you explicitly indicate that CellState and Graph are only needed for type information, matching the broader refactor objectives.

packages/core/src/view/layout/CompactTreeLayout.ts (1)

26-27: Type-only Imports Conversion for Cell and Graph
The conversion of standard imports to type-only imports using import type for both Cell and Graph is clear and consistent. This change ensures that these imports are used exclusively for type-checking, reducing unnecessary runtime overhead while enhancing clarity.

packages/core/src/util/xmlUtils.ts (1)

21-22: Converting Imports to Type-Only in XML Utilities
Switching the imports for Cell and Graph to type-only imports here aligns well with the overall refactor. This update improves type safety by clarifying that these are solely for compile-time checks and do not produce runtime dependencies.

packages/core/src/view/layout/CompositeLayout.ts (1)

19-20: Consistent Application of Type-Only Imports
Updating the Cell and Graph imports to use the import type syntax confirms that these entities are only required for type annotations. This consistent application across the codebase helps streamline dependencies and improves maintainability.

packages/core/src/view/cell/CellMarker.ts (1)

32-36: Enhanced Type Declaration Clarity in CellMarker
The changes converting the imports for Graph, CellState, and Cell into type-only imports are well executed. This refinement reduces the runtime footprint and makes the intended usage of these entities explicit, reinforcing type safety throughout the module.

packages/core/src/view/layout/PartitionLayout.ts (1)

21-22: Streamlining Type-Only Imports for Graph and Cell
Changing the import statements for Graph and Cell to type-only variants is a straightforward improvement. It avoids bringing in unnecessary runtime code and aligns perfectly with the PR objective of simplifying type imports.

packages/core/src/util/gestureUtils.ts (2)

20-21: Improved type imports using import type

Good job updating the imports to use the import type syntax. This change clarifies that these entities are only used for type information and not for runtime values, which can lead to better tree-shaking during build.


78-78: Documentation improvements with proper linking

Nice improvement to the JSDoc by using the correct class name in the {@link} references. This makes the documentation more accurate and helps developers find the right methods when navigating the API documentation.

Also applies to: 85-85

packages/core/src/util/treeTraversal.ts (2)

17-17: Improved type imports using import type

Good job updating the imports to use the import type syntax. This change clarifies that these entities are only used for type information and not for runtime values, which can lead to better tree-shaking during build.

Also applies to: 19-19


30-30: Fixed JSDoc reference from mxCell to Cell

Good catch updating the JSDoc reference from {@link mxCell} to {@link Cell}. This fixes an outdated reference and makes the documentation consistent with the actual code.

packages/core/src/view/animate/Morphing.ts (1)

22-24: Improved type imports using import type

Good job updating the imports to use the import type syntax. This is especially appropriate in this file since CellState, Cell, and Graph are used exclusively for type annotations throughout the code. This change improves build optimization by clearly separating type imports from runtime imports.

packages/core/src/view/cell/VertexHandle.ts (1)

70-76: Simplified assignment of graph property.
By removing the explicit type assertion (casting) from state.view.graph, the code is now cleaner and assumes that state.view.graph is already of type Graph. Please ensure that this assumption holds true throughout the codebase.

packages/core/src/view/plugins/TooltipHandler.ts (1)

24-25: Converted Graph import to a type-only import.
Changing the import to import type { Graph } from '../Graph'; makes it clear that Graph is only used for type-checking purposes. This simplifies the runtime bundle.

packages/core/src/view/plugins/ConnectionHandler.ts (1)

57-61: Standardized type-only imports for type declarations.
The modifications converting imports for Graph (and for CellStyle, ColorValue, GraphPlugin, Listenable) into type-only imports clarify that these are used solely for compile-time type checks. Please ensure that none of these entities are required as runtime values elsewhere in this file.

packages/core/src/view/plugins/SelectionHandler.ts (3)

43-43: Improved import using type-only syntax.

Converting the Graph import to a type-only import properly indicates that this class only uses Graph as a type reference, not importing its actual implementation.


1133-1134: Simplified by removing unnecessary type casting.

Removing the redundant <Graph> type assertion simplifies the code while maintaining type safety. The TypeScript compiler can infer the correct type from the context.


1235-1236: Removed redundant type assertion.

Good cleanup by removing the unnecessary <Graph> type cast, making the code cleaner while maintaining the same functionality.

packages/core/src/view/handler/VertexHandler.ts (8)

29-29: Improved import using type-only syntax for Graph.

Converting the Graph import to a type-only import properly indicates that this component only uses Graph as a type reference.


32-32: Improved import using type-only syntax for Cell.

Similarly to the Graph import, this changes Cell to a type-only import, correctly indicating its usage solely for type checking.


325-325: Removed redundant type cast in escapeHandler.

Simplified code by removing unnecessary <Graph> type assertion while maintaining type safety.


336-336: Simplified graph event listener registration.

Removing the redundant <Graph> type assertion makes the code cleaner and more maintainable.


682-683: Simplified parent state retrieval.

Removed unnecessary type assertion when getting the parent state, keeping the code clean while preserving functionality.


1164-1164: Streamlined cellRenderer access in updateLivePreview.

Removed redundant type casting of this.state.view.graph to Graph when accessing the cellRenderer.


1920-1920: Simplified conditional check in isCustomHandleVisible.

Removed unnecessary type assertion when checking if the graph is in editing mode, making the code more readable.


2036-2036: Cleaned up event listener removal in onDestroy.

Simplified the code by removing redundant type assertion when removing the escape handler listener.

packages/core/src/view/handler/KeyHandler.ts (15)

19-19: Good use of type-only import

Converting to a type-only import for Graph is a positive change that aligns with the PR objectives of simplifying graph type imports. Type-only imports help with tree-shaking during compilation and more clearly indicate intent in the code.


37-39: Documentation improvement

Good improvement to the JSDoc comment using proper {@link} syntax for references to Graph.container and graph. This enhances readability and makes the documentation more useful for developers and documentation generators.


47-50: Example code formatting improvement

The code examples in the JSDoc have been properly formatted with consistent indentation and arrow function style, which improves readability.


65-67: Improved code example readability

The code example in the JSDoc has been properly formatted with consistent indentation and arrow function style, matching the example style used throughout the documentation.


74-80: Enhanced constructor documentation

The constructor JSDoc has been greatly improved with clearer parameter descriptions and formatting. The added details about the optional target parameter provide important information about the default behavior.


228-230: Improved method documentation

The JSDoc for isGraphEvent has been enhanced with a clearer description of when the method returns true, including proper references to the graph's components using {@link} syntax.


248-248: Simplified conditional logic

Good change to replace a potentially complex nested condition with a more straightforward boolean expression that's easier to read and understand.


253-255: Documentation formatting improvement

The JSDoc for keyDown has been updated with proper backticks around method names and improved wording, which enhances readability.


279-286: Documentation clarity improvement

The JSDoc for isEnabledForEvent has been improved with better formatting and more precise wording, making it clearer what conditions must be met for the method to return true.


290-291: Improved null safety with optional chaining

Good use of optional chaining (?.) when accessing this.graph.isEnabled(). This prevents potential runtime errors if this.graph is null, making the code more robust.


299-300: Documentation formatting improvement

The JSDoc for isEventIgnored has been updated with proper reference formatting using {@link}, which enhances documentation quality.


304-304: Use of nullish coalescing operator

Good use of the nullish coalescing operator (??) instead of a ternary expression. This is more concise and clearly communicates the intent to use false as a fallback when this.graph?.isEditing() is null or undefined.


308-310: Documentation clarification

The JSDoc for the escape method has been improved with better reference formatting using {@link}, which enhances documentation quality.


312-312: Parameter documentation improvement

The JSDoc parameter description for the escape method has been enhanced with additional information about the possible keycode, which is helpful for developers.


315-316: Improved null safety with optional chaining

Good use of optional chaining (?.) when accessing this.graph.isEscapeEnabled() and this.graph.escape(evt). This prevents potential runtime errors if this.graph is null, making the code more robust.

packages/core/src/view/event/InternalEvent.ts (3)

22-22: Convert CellState to a type‑only import
Changing the import of CellState to a type‑only import clarifies that it is used purely for type annotations. This change helps ensure that no unnecessary runtime code is imported, improving bundle size and maintainability.


23-29: Utilize type‑only import for utility types
The import block for types such as EventCache, GestureEvent, KeyboardEventListener, Listenable, and MouseEventListener has been converted to use the type‑only import syntax. This standardizes the import style and ensures that these types are not included in the runtime bundle.


30-30: Adopt type‑only import for Graph
Updating the import of Graph to a type‑only import is a great step toward clarifying that the Graph entity is used solely for type checking. This reduces runtime dependencies and aligns with modern TypeScript best practices.

packages/core/src/view/plugins/CellEditorHandler.ts (3)

51-51: Adopt Type-Only Import for Graph
Changing the import to use import type { Graph } from '../Graph'; is an excellent move. It reduces runtime overhead by ensuring that only types are imported, and it aligns well with modern TypeScript practices.


571-573: Simplify Legacy Spacing Check in resize()
The condition now directly accesses state.view.graph.cellRenderer.legacySpacing without casting to <Graph>. This change improves readability and maintainability. Please verify that no edge-case behavior relied on the original type assertion.


933-939: Streamline Legacy Spacing Check in getEditorBounds()
By removing the unnecessary type assertion in the condition

if (!isEdge && state.view.graph.cellRenderer.legacySpacing && state.style.overflow === 'fill') {

the code becomes cleaner and more consistent with the updated type-only imports. Ensure that this change does not inadvertently affect any logic that depended on a specific type structure.

@sonarqubecloud

sonarqubecloud Bot commented Apr 1, 2025

Copy link
Copy Markdown

@tbouffard tbouffard merged commit e0ff569 into main Apr 1, 2025
@tbouffard tbouffard deleted the refactor/types_remove_extra_cast_to_Graph branch April 1, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant