refactor: migrate the "Animation" story to TypeScript#635
Conversation
Rationale: - Ease the maintenance - Detect the errors earlier
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/html/stories/Animation.stories.ts (1)
35-35: Consider using a more specific type for the template props.Using
Record<string, string>is too generic and might hide potential type errors. Consider creating a specific interface for the props.interface TemplateProps extends Record<string, string> { label: string; } const Template = ({ label, ...args }: TemplateProps)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/html/stories/Animation.stories.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/html/stories/Animation.stories.ts
[error] 82-82: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
⏰ 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 (4)
packages/html/stories/Animation.stories.ts (4)
18-18: LGTM! Clean type import declaration.The type-only import for
CellStylefollows TypeScript best practices.
Line range hint
42-51: LGTM! Well-typed style object.The
CellStyletype annotation provides good type safety for the vertex styling properties.
77-79: LGTM! Proper boolean values for edge properties.Converting numeric values to proper boolean types improves type safety and readability.
87-91: Consider refactoring repetitive DOM operations.The code could be simplified to reduce repetition and improve null safety.
const path0 = state?.shape?.node.getElementsByTagName('path')[0]; const path1 = state?.shape?.node.getElementsByTagName('path')[1]; if (path0) { path0.removeAttribute('visibility'); path0.setAttribute('stroke-width', '6'); path0.setAttribute('stroke', 'lightGray'); } path1?.setAttribute('class', 'flow');Also, consider adding a comment explaining why the non-null assertion (
!) is safe to use one1.✅ Verification successful
Refactoring suggestion is valid and safe to implement
The suggested refactoring will improve code readability while maintaining the same functionality. The non-null assertion on
e1is safe as the edge is created and configured immediately before use.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find the context around this code rg -B 10 -A 5 "state\?\.shape\?\.node\.getElementsByTagName\('path'\)" packages/html/stories/Animation.stories.ts # Look for similar patterns of DOM operations on SVG paths rg "getElementsByTagName\('path'\)" --type ts # Find usage of graph.view.getState rg "graph\.view\.getState" --type tsLength of output: 5969
| }, | ||
| }); | ||
| e1.geometry.points = [new Point(230, 50)]; | ||
| e1.geometry && (e1.geometry.points = [new Point(230, 50)]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid assignment within expression.
The assignment within the condition makes the code harder to read and maintain.
-e1.geometry && (e1.geometry.points = [new Point(230, 50)]);
+if (e1.geometry) {
+ e1.geometry.points = [new Point(230, 50)];
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🧰 Tools
🪛 Biome (1.9.4)
[error] 82-82: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
1 participant
Rationale:
Summary by CodeRabbit
CellStyle