-
Notifications
You must be signed in to change notification settings - Fork 199
feat: improve IDE guidance for style element registration #787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- 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
WalkthroughThis 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
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
Possibly related PRs
Suggested labels
✨ 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:
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.
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 precisionThe numeric constant
0.7071is an approximation of1/√2. For better precision and clarity, consider using the standard library constantMath.SQRT1_2instead.- 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
📒 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 propertyThe documentation now more clearly specifies that
PerimeterFunctiontypes can be builtin functions from thePerimeternamespace, improving developer understanding.
563-564: Better type safety with PerimeterValue referenceDocumentation now refers to
PerimeterValueinstead ofPERIMETER, aligning with the move to type-safe values instead of enum constants.
1273-1279: Improved type definition with separate MarkerFunction typeThe refactoring separates the marker function type from the factory function type by:
- Changing
MarkerFactoryFunctionto return the newMarkerFunctiontype- Creating a dedicated
MarkerFunctiontype aliasThis 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 styleUpdated the edge style reference from using
constants.EDGESTYLE.ORTHOGONALto 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 styleUpdated the edge style reference from using
constants.EDGESTYLE.ORTHOGONALto 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 styleUpdated the edge style reference from using
constants.EDGESTYLE.ORTHOGONALto the string literal'orthogonalEdgeStyle'in the legacyinsertEdgemethod, 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 improvementsReplacing the imported
ARROWconstant 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, andPERIMETERenums 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 registrationChanging the
keyparameter type fromstringtoStyleShapeValueprovides 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
StyleShapeValuetype 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 arrayUpdating the type of the
shapesToRegisterarray from[string, ShapeConstructor][]to[ShapeValue, ShapeConstructor][]improves type safety. This change ensures that only valid predefined shape values from theShapeValuetype 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 predicatesCreating 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
StyleArrowValuetype, which aligns with the PR's goal of improving type safety.
67-67: Improved readability by using type predicatesReplacing direct string comparisons with the
isClassicOrClassicThinfunction 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 predicateUsing the
isDiamondfunction 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 importThe 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 literalsThe change from
EDGESTYLE.TOPTOBOTTOMandEDGESTYLE.ELBOWconstants 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 typesAdding explicit imports for
EdgeStyleValueandPerimeterValuetypes enhances type safety and improves IDE support for these specialized string values.
37-37: Excellent type refinementUpdating the parameter type from generic
stringto the more specific union typePerimeterValue | EdgeStyleValue | stringimproves 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 typesAdding the
MarkerFunctiontype import and organizing imports with proper structure improves code clarity and type safety.
45-46: Great parameter naming and type refinementRenaming from
functtofactorymakes the parameter name more descriptive, and changing the type fromstringtoStyleArrowValueenhances type safety and IDE autocomplete support.
63-63: Good explicit return typeAdding the explicit return type
MarkerFunction | nullimproves type safety and makes the method signature more self-documenting.packages/core/src/view/style/register.ts (5)
18-18: Good refactoring of importsUpdating 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 importsAdding 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 registrationRefactoring to use explicit type annotation with
EdgeStyleValueand 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 registrationSimilar 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 registrationRefactoring to use
ArrowValuetype and string literals for markers while also using the EdgeMarker class methods improves type safety and IDE support, making the code more maintainable.



Notes
Covers #378
Summary by CodeRabbit
Refactor
Style
Chores