Comfy-Org / Comfy-Org/ComfyUI_frontend

Follow-up improvements for LayoutStore type safety (from PR #5385)

Open
#5,440 0 comments 0 reactions 0 assignees View on GitHub
area:nodes area:reroutes area:vue-migration
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.