Comfy-Org / Comfy-Org/ComfyUI_frontend

[Enhancement] Replace JSON.parse(JSON.stringify()) with structuredClone() for object cloning

Open
#4,696 0 comments 0 reactions 0 assignees View on GitHub
developer experience
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

# [Enhancement] Replace JSON.parse(JSON.stringify()) with structuredClone() for object cloning

## Summary
Replace the current `LiteGraph.cloneObject()` implementation that uses `JSON.parse(JSON.stringify())` with the modern `structuredClone()` API to fix circular reference bugs and improve type preservation.

## Problem
The current cloning implementation has several issues:
1. **Throws on circular references** - causes crashes when objects have circular refs
2. **Poor type preservation** - Dates become strings, RegExp becomes `{}`, `undefined` becomes `null`
3. **Missing modern API adoption** - `structuredClone()` has been available since 2022

## Proposed Solution
Implement a hybrid approach with `structuredClone()` as primary method and JSON fallback:

```typescript
cloneObject(obj: T): T {
if (obj == null) return obj

if (typeof structuredClone !== 'undefined') {
try {
return structuredClone(obj)
} catch (error) {
console.warn('structuredClone failed, falling back to JSON method:', error)
}
}

return JSON.parse(JSON.stringify(obj))
}
```

## Benefits
- ✅ **Fixes circular reference crashes**
- ✅ **Better type preservation** (Dates, RegExp, undefined)
- ✅ **Improved performance** (native implementation)
- ✅ **Backward compatibility** (fallback for edge cases)

## Impact Assessment
**Risk Level: LOW** ✅

- **Compatibility**: Already targeting ES2022/modern browsers
- **Usage sites**: 6 locations, all handle plain data objects
- **Breaking changes**: None expected (fallback maintains compatibility)
- **Performance**: Neutral to positive

## Affected Files
- `src/LiteGraphGlobal.ts` - Main implementation
- `src/LGraphNode.ts` - 4 usage sites (serialize, configure, clone)
- `src/subgraph/subgraphUtils.ts` - 1 usage site (multiClone)

## Testing Requirements
- [ ] Circular reference handling
- [ ] Date/RegExp preservation
- [ ] undefined value handling
- [ ] Widget value compatibility
- [ ] Fallback behavior
- [ ] Performance regression testing

## Implementation Steps
1. **Phase 1**: Replace `cloneObject()` with hybrid implementation
2. **Phase 2**: Add comprehensive tests
3. **Phase 3**: Monitor for any behavioral changes in production
4. **Phase 4**: Consider removing fallback after confidence period

## References
- [MDN structuredClone documentation](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone)
- Audit report: `STRUCTURED_CLONE_AUDIT.md`
- Browser support: Chrome 98+, Firefox 94+, Safari 15.4+, Node.js 17+

## Priority
**Medium** - This fixes real bugs (circular references) and improves robustness, but doesn't block current functionality.

---
**Acceptance Criteria:**
- [ ] `LiteGraph.cloneObject()` uses `structuredClone()` when available
- [ ] Maintains fallback to JSON method for compatibility
- [ ] All existing tests pass
- [ ] New tests added for circular references and type preservation
- [ ] No regressions in node serialization/deserialization
- [ ] Performance neutral or improved

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-4696-Enhancement-Replace-JSON-parse-JSON-stringify-with-structuredClone-for-object-c-2476d73d3650817682b3d226740224ab) 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.