Skip to content

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Apr 28, 2025

  • Replace string parameters with specific types in registry methods. This provides better autocompletion when registering builtin elements
  • Use type-safe name values instead of ARROW, EDGESTYLE and PERIMETER enums that will be removed soon

Notes

Covers #378

Summary by CodeRabbit

  • Refactor

    • Replaced constant references for edge styles, perimeters, and markers with string literals for clearer style identification.
    • Updated type annotations and method signatures to use more specific type aliases for style keys and marker functions.
    • Improved code clarity by centralizing type checks and refining imports.
  • Style

    • Enhanced documentation comments for better clarity regarding perimeter and marker types.
  • Chores

    • Updated example projects to use string literals instead of constants for specifying edge styles.

- Replace string parameters with specific types in registry methods. This provides better autocompletion when registering builtin elements
- Use type-safe name values instead of ARROW, EDGESTYLE and PERIMETER enums that will be removed soon
@tbouffard tbouffard added the refactor Code refactoring label Apr 28, 2025
Copy link

coderabbitai bot commented Apr 28, 2025

Walkthrough

This set of changes refines type usage and constant handling throughout the codebase. TypeScript type annotations are made more specific, particularly for style, shape, edge, perimeter, and marker registration, replacing generic strings and constant references with explicit string literal types and type aliases. Several method signatures are updated to reflect these changes, and some internal logic is refactored to use helper functions for type checking. Additionally, constant references in style assignments are replaced with string literals in both source and example files. No core logic or control flow is altered.

Changes

File(s) Change Summary
packages/core/src/types.ts Updated documentation for perimeter property, changed MarkerFactoryFunction return type to a new MarkerFunction type alias, and added MarkerFunction export.
packages/core/src/view/cell/CellRenderer.ts Changed registerShape method's key parameter type from string to StyleShapeValue; updated imports accordingly.
packages/core/src/view/cell/register-shapes.ts Changed shape registration array type from [string, ShapeConstructor][] to [ShapeValue, ShapeConstructor][]; updated imports.
packages/core/src/view/handler/ElbowEdgeHandler.ts Removed unused constant import; replaced constant comparisons with string literal checks in cursor determination logic.
packages/core/src/view/style/StyleRegistry.ts Updated putValue method to accept `PerimeterValue
packages/core/src/view/style/Stylesheet.ts Removed unused constant import; replaced usage of constant with string literal 'classic' for endArrow property.
packages/core/src/view/style/marker/EdgeMarkerRegistry.ts Added MarkerFunction import; updated addMarker method to use StyleArrowValue and renamed parameter; made createMarker return type explicit.
packages/core/src/view/style/marker/edge-markers.ts Refactored type checks to use new helper functions for arrow type identification; replaced direct constant comparisons.
packages/core/src/view/style/register.ts Replaced constant imports and usages with string literals and explicit type aliases for style, perimeter, and marker registration; updated registration arrays accordingly.
packages/js-example-without-defaults/src/index.js
packages/ts-example-without-defaults/src/main.ts
packages/ts-example/src/main.ts
Replaced edge style assignments using constants with string literals 'orthogonalEdgeStyle'.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CellRenderer
    participant StyleRegistry
    participant EdgeMarkerRegistry

    User->>CellRenderer: registerShape(key: StyleShapeValue, shape)
    CellRenderer->>StyleRegistry: putValue(key, shape)

    User->>EdgeMarkerRegistry: addMarker(type: StyleArrowValue, factory)
    User->>EdgeMarkerRegistry: createMarker(type, ...args)
    EdgeMarkerRegistry->>EdgeMarkerRegistry: lookup factory by type
    EdgeMarkerRegistry-->>User: MarkerFunction | null
Loading

Possibly related PRs

Suggested labels

enhancement

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

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 (1)
packages/core/src/view/style/marker/edge-markers.ts (1)

211-212: Use Math.SQRT1_2 for improved precision

The numeric constant 0.7071 is an approximation of 1/√2. For better precision and clarity, consider using the standard library constant Math.SQRT1_2 instead.

- const swFactor = isDiamond(type) ? 0.7071 : 0.9862;
+ const swFactor = isDiamond(type) ? Math.SQRT1_2 : 0.9862;
🧰 Tools
🪛 Biome (1.9.4)

[error] 211-211: Prefer constants from the standard library.

Unsafe fix: Use Math.SQRT1_2 instead.

(lint/suspicious/noApproximativeNumericConstant)

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 97ec7d0 and 7072a23.

📒 Files selected for processing (12)
  • packages/core/src/types.ts (2 hunks)
  • packages/core/src/view/cell/CellRenderer.ts (2 hunks)
  • packages/core/src/view/cell/register-shapes.ts (2 hunks)
  • packages/core/src/view/handler/ElbowEdgeHandler.ts (2 hunks)
  • packages/core/src/view/style/StyleRegistry.ts (2 hunks)
  • packages/core/src/view/style/Stylesheet.ts (2 hunks)
  • packages/core/src/view/style/marker/EdgeMarkerRegistry.ts (3 hunks)
  • packages/core/src/view/style/marker/edge-markers.ts (5 hunks)
  • packages/core/src/view/style/register.ts (4 hunks)
  • packages/js-example-without-defaults/src/index.js (1 hunks)
  • packages/ts-example-without-defaults/src/main.ts (1 hunks)
  • packages/ts-example/src/main.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/core/src/view/cell/register-shapes.ts (1)
packages/core/src/types.ts (2)
  • ShapeValue (953-969)
  • ShapeConstructor (1388-1388)
packages/core/src/view/cell/CellRenderer.ts (1)
packages/core/src/types.ts (2)
  • StyleShapeValue (975-975)
  • ShapeConstructor (1388-1388)
packages/core/src/view/style/marker/edge-markers.ts (1)
packages/core/src/types.ts (1)
  • StyleArrowValue (941-941)
packages/core/src/view/style/register.ts (2)
packages/core/src/types.ts (6)
  • EdgeStyleValue (1237-1245)
  • EdgeStyleFunction (1224-1230)
  • PerimeterValue (1205-1210)
  • PerimeterFunction (1193-1198)
  • ArrowValue (925-935)
  • MarkerFactoryFunction (1262-1273)
packages/core/src/view/style/edge/index.ts (1)
  • EdgeStyle (84-155)
packages/core/src/view/style/StyleRegistry.ts (1)
packages/core/src/types.ts (2)
  • PerimeterValue (1205-1210)
  • EdgeStyleValue (1237-1245)
packages/core/src/view/style/marker/EdgeMarkerRegistry.ts (1)
packages/core/src/types.ts (3)
  • StyleArrowValue (941-941)
  • MarkerFactoryFunction (1262-1273)
  • MarkerFunction (1279-1279)
🪛 Biome (1.9.4)
packages/core/src/view/style/marker/edge-markers.ts

[error] 211-211: Prefer constants from the standard library.

Unsafe fix: Use Math.SQRT1_2 instead.

(lint/suspicious/noApproximativeNumericConstant)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: build
  • GitHub Check: build (windows-2022)
  • GitHub Check: build (macos-14)
  • GitHub Check: build (ubuntu-22.04)
🔇 Additional comments (24)
packages/core/src/types.ts (3)

561-563: Documentation improvement for perimeter property

The documentation now more clearly specifies that PerimeterFunction types can be builtin functions from the Perimeter namespace, improving developer understanding.


563-564: Better type safety with PerimeterValue reference

Documentation now refers to PerimeterValue instead of PERIMETER, aligning with the move to type-safe values instead of enum constants.


1273-1279: Improved type definition with separate MarkerFunction type

The refactoring separates the marker function type from the factory function type by:

  1. Changing MarkerFactoryFunction to return the new MarkerFunction type
  2. Creating a dedicated MarkerFunction type alias

This provides better type clarity and IDE support when working with markers.

packages/ts-example-without-defaults/src/main.ts (1)

54-54: Replaced enum constant with string literal for edge style

Updated the edge style reference from using constants.EDGESTYLE.ORTHOGONAL to the string literal 'orthogonalEdgeStyle', improving type safety and IDE autocompletion support.

packages/js-example-without-defaults/src/index.js (1)

54-54: Replaced enum constant with string literal for edge style

Updated the edge style reference from using constants.EDGESTYLE.ORTHOGONAL to the string literal 'orthogonalEdgeStyle', providing consistency with the TypeScript examples and aligning with the type-safe approach.

packages/ts-example/src/main.ts (1)

81-81: Replaced enum constant with string literal for edge style

Updated the edge style reference from using constants.EDGESTYLE.ORTHOGONAL to the string literal 'orthogonalEdgeStyle' in the legacy insertEdge method, ensuring consistent implementation of the type-safe approach throughout the codebase.

packages/core/src/view/style/Stylesheet.ts (1)

19-19: String literal usage aligned with TypeScript type safety improvements

Replacing the imported ARROW constant with a string literal for the edge arrow style is a good change. This aligns with the PR objective of replacing enum constants with type-safe name values, which will provide better IDE support through autocompletion.

The change is part of the planned removal of ARROW, EDGESTYLE, and PERIMETER enums mentioned in the PR description.

Also applies to: 85-85

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

49-49: Improved type safety for shape registration

Changing the key parameter type from string to StyleShapeValue provides better type safety and IDE autocompletion. This is a great improvement that aligns with the PR objectives of enhancing developer experience when registering built-in elements.

The StyleShapeValue type restricts the possible values to predefined shape types while still allowing for extensibility through the string union type.

Also applies to: 159-159

packages/core/src/view/cell/register-shapes.ts (1)

18-18: Enhanced type safety for shape registration array

Updating the type of the shapesToRegister array from [string, ShapeConstructor][] to [ShapeValue, ShapeConstructor][] improves type safety. This change ensures that only valid predefined shape values from the ShapeValue type can be registered.

This change is consistent with the updated parameter type in CellRenderer.registerShape() and contributes to better IDE autocompletion support.

Also applies to: 48-48

packages/core/src/view/style/marker/edge-markers.ts (3)

24-27: Good refactoring: Extracted type check predicates

Creating dedicated helper functions for type checking is a good refactoring. These predicate functions make the code more readable and maintainable by centralizing the logic for checking arrow types.

The new functions also work with the StyleArrowValue type, which aligns with the PR's goal of improving type safety.


67-67: Improved readability by using type predicates

Replacing direct string comparisons with the isClassicOrClassicThin function improves code readability and maintainability. If the definition of what constitutes a "classic" arrow changes in the future, you'll only need to update it in one place.

Also applies to: 79-79


226-226: Improved readability with the isDiamond predicate

Using the isDiamond function here improves readability and ensures consistency in how diamond markers are detected throughout the code.

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

20-20: Good removal of the EDGESTYLE import

The EDGESTYLE constant is no longer needed as the code now uses string literals for edge styles, aligning with the PR objective to replace enum constants with type-safe names.


114-116: Good refactoring to use string literals

The change from EDGESTYLE.TOPTOBOTTOM and EDGESTYLE.ELBOW constants to string literals 'topToBottomEdgeStyle' and 'elbowEdgeStyle' improves type safety and IDE support while maintaining the same functionality.

packages/core/src/view/style/StyleRegistry.ts (2)

19-19: Good addition of specific types

Adding explicit imports for EdgeStyleValue and PerimeterValue types enhances type safety and improves IDE support for these specialized string values.


37-37: Excellent type refinement

Updating the parameter type from generic string to the more specific union type PerimeterValue | EdgeStyleValue | string improves IDE guidance through better autocompletion support while maintaining compatibility with existing code.

packages/core/src/view/style/marker/EdgeMarkerRegistry.ts (3)

19-23: Good addition of explicit types

Adding the MarkerFunction type import and organizing imports with proper structure improves code clarity and type safety.


45-46: Great parameter naming and type refinement

Renaming from funct to factory makes the parameter name more descriptive, and changing the type from string to StyleArrowValue enhances type safety and IDE autocomplete support.


63-63: Good explicit return type

Adding the explicit return type MarkerFunction | null improves type safety and makes the method signature more self-documenting.

packages/core/src/view/style/register.ts (5)

18-18: Good refactoring of imports

Updating imports to use named exports from './builtin-style-elements' makes the code more maintainable and aligns with the type-safe approach being implemented.


22-28: Good type imports

Adding explicit imports for type aliases (ArrowValue, EdgeStyleValue, PerimeterValue) improves code consistency and supports the enhanced type safety throughout the file.


41-49: Excellent type-safe edge style registration

Refactoring to use explicit type annotation with EdgeStyleValue and string literals instead of constants provides better IDE guidance and type safety, making it easier for developers to register edge styles correctly.


70-75: Excellent type-safe perimeter registration

Similar to the edge styles, this change replaces constant values with typed string literals for perimeters, providing better IDE autocompletion and type checking.


112-121: Excellent type-safe marker registration

Refactoring to use ArrowValue type and string literals for markers while also using the EdgeMarker class methods improves type safety and IDE support, making the code more maintainable.

@tbouffard tbouffard merged commit 9da8e3a into main Apr 28, 2025
7 checks passed
@tbouffard tbouffard deleted the feat/more_guidance_in_registries_name_param branch April 28, 2025 16:37
@tbouffard tbouffard added enhancement New feature or request and removed refactor Code refactoring labels May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant