Comfy-Org / Comfy-Org/ComfyUI_frontend
Follow-up improvements for LayoutStore type safety (from PR #5385)
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
This issue tracks unresolved review comments from PR #5385 that should be addressed as follow-up improvements to the LayoutStore type safety implementation.
## Architecture Improvements
**Bounds derivation optimization**
- The `bounds` property is currently stored separately from `position` and `size`
- Consider deriving bounds dynamically to reduce redundant data and prevent sync issues
- Proposed solution:
```ts
function getNodeBounds(node: NodeLayoutData): Bounds {
const { x, y } = node.position
const { width, height } = node.size
return { x, y, width, height }
}
```
**Pure function optimization**
- Methods not referencing `this` should be moved to module scope for better performance
- Affected methods: `getNodeField`, `getLinkField`, `getRerouteField`, `asRerouteId`
- Refactor these to pure functions outside the class
## Type Safety Enhancements
**Default ID safety**
- Hardcoded default IDs (0) in `REROUTE_DEFAULTS` could cause bugs with multiple entities
- Risk: Multiple reroutes defaulting to parentId/id of 0 could build up in state
- Consider using `Partial` or `Omit` to explicitly require these fields at instantiation
**TypeScript interface enforcement**
- Current type safety has limitations at the function interface level for Y.Map operations
- Explore ways to improve compile-time type checking for Y.Map type assertions
## Future Architecture Considerations
**Index-based slot robustness**
- Current index-based slot system may have issues with variable input nodes
- When removing middle inputs, subsequent links need targetSlot decremented by 1
- Not blocking current implementation but worth considering alternative approaches for future versions
## Action Items
- Implement bounds derivation function and remove bounds from stored data
- Move pure functions to module scope
- Improve default ID safety with better typing patterns
- Research TypeScript interface enforcement improvements
- Document slot index considerations for future development
## Context
Related to PR #5385 LayoutStore type safety improvements and broader renderer architecture enhancements.
┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-5440-Follow-up-improvements-for-LayoutStore-type-safety-from-PR-5385-2686d73d36508151bd78d2ea8c3b9bd4) by [Unito](https://www.unito.io)
Contributor guide
Assessment
This issue has not been assessed yet.