mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-12 08:30:08 +00:00
refactor: migrate ES private fields to TypeScript private for Vue Proxy compatibility (#8440)
## Summary Migrates ECMAScript private fields (`#`) to TypeScript private (`private`) across LiteGraph to fix Vue Proxy reactivity incompatibility. ## Problem ES private fields (`#field`) are incompatible with Vue's Proxy-based reactivity system - accessing `#field` through a Proxy throws `TypeError: Cannot read private member from an object whose class did not declare it`. ## Solution - Converted all `#field` to `private _field` across 10 phases - Added `toJSON()` methods to `LGraph`, `NodeSlot`, `NodeInputSlot`, and `NodeOutputSlot` to prevent circular reference errors during serialization (TypeScript private fields are visible to `JSON.stringify` unlike true ES private fields) - Made `DragAndScale.element.data` non-enumerable to break canvas circular reference chain ## Testing - All 4027 unit tests pass - Added 9 new serialization tests to catch future circular reference issues - Browser tests (undo/redo, save workflows) verified working ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8440-refactor-migrate-ES-private-fields-to-TypeScript-private-for-Vue-Proxy-compatibility-2f76d73d365081a3bd82d429a3e0fcb7) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -103,7 +103,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
|
||||
override onAfterGraphConfigured() {
|
||||
if (this.outputs[0].links?.length && !this.widgets?.length) {
|
||||
this.#onFirstConnection()
|
||||
this._onFirstConnection()
|
||||
|
||||
// Populate widget values from config data
|
||||
if (this.widgets && this.widgets_values) {
|
||||
@@ -116,7 +116,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
}
|
||||
|
||||
// Merge values if required
|
||||
this.#mergeWidgetConfig()
|
||||
this._mergeWidgetConfig()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -133,11 +133,11 @@ export class PrimitiveNode extends LGraphNode {
|
||||
const links = this.outputs[0].links
|
||||
if (connected) {
|
||||
if (links?.length && !this.widgets?.length) {
|
||||
this.#onFirstConnection()
|
||||
this._onFirstConnection()
|
||||
}
|
||||
} else {
|
||||
// We may have removed a link that caused the constraints to change
|
||||
this.#mergeWidgetConfig()
|
||||
this._mergeWidgetConfig()
|
||||
|
||||
if (!links?.length) {
|
||||
this.onLastDisconnect()
|
||||
@@ -159,7 +159,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
}
|
||||
|
||||
if (this.outputs[slot].links?.length) {
|
||||
const valid = this.#isValidConnection(input)
|
||||
const valid = this._isValidConnection(input)
|
||||
if (valid) {
|
||||
// On connect of additional outputs, copy our value to their widget
|
||||
this.applyToGraph([{ target_id: target_node.id, target_slot } as LLink])
|
||||
@@ -170,7 +170,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
return true
|
||||
}
|
||||
|
||||
#onFirstConnection(recreating?: boolean) {
|
||||
private _onFirstConnection(recreating?: boolean) {
|
||||
// First connection can fire before the graph is ready on initial load so random things can be missing
|
||||
if (!this.outputs[0].links || !this.graph) {
|
||||
this.onLastDisconnect()
|
||||
@@ -204,7 +204,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
this.outputs[0].name = type
|
||||
this.outputs[0].widget = widget
|
||||
|
||||
this.#createWidget(
|
||||
this._createWidget(
|
||||
widget[CONFIG] ?? config,
|
||||
theirNode,
|
||||
widget.name,
|
||||
@@ -213,7 +213,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
)
|
||||
}
|
||||
|
||||
#createWidget(
|
||||
private _createWidget(
|
||||
inputData: InputSpec,
|
||||
node: LGraphNode,
|
||||
widgetName: string,
|
||||
@@ -307,8 +307,8 @@ export class PrimitiveNode extends LGraphNode {
|
||||
|
||||
recreateWidget() {
|
||||
const values = this.widgets?.map((w) => w.value)
|
||||
this.#removeWidgets()
|
||||
this.#onFirstConnection(true)
|
||||
this._removeWidgets()
|
||||
this._onFirstConnection(true)
|
||||
if (values?.length && this.widgets) {
|
||||
for (let i = 0; i < this.widgets.length; i++)
|
||||
this.widgets[i].value = values[i]
|
||||
@@ -316,7 +316,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
return this.widgets?.[0]
|
||||
}
|
||||
|
||||
#mergeWidgetConfig() {
|
||||
private _mergeWidgetConfig() {
|
||||
// Merge widget configs if the node has multiple outputs
|
||||
const output = this.outputs[0]
|
||||
const links = output.links ?? []
|
||||
@@ -348,11 +348,11 @@ export class PrimitiveNode extends LGraphNode {
|
||||
const theirInput = theirNode.inputs[link.target_slot]
|
||||
|
||||
// Call is valid connection so it can merge the configs when validating
|
||||
this.#isValidConnection(theirInput, hasConfig)
|
||||
this._isValidConnection(theirInput, hasConfig)
|
||||
}
|
||||
}
|
||||
|
||||
#isValidConnection(input: INodeInputSlot, forceUpdate?: boolean) {
|
||||
private _isValidConnection(input: INodeInputSlot, forceUpdate?: boolean) {
|
||||
// Only allow connections where the configs match
|
||||
const output = this.outputs?.[0]
|
||||
const config2 = (input.widget?.[GET_CONFIG] as () => InputSpec)?.()
|
||||
@@ -367,7 +367,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
)
|
||||
}
|
||||
|
||||
#removeWidgets() {
|
||||
private _removeWidgets() {
|
||||
if (this.widgets) {
|
||||
// Allow widgets to cleanup
|
||||
for (const w of this.widgets) {
|
||||
@@ -398,7 +398,7 @@ export class PrimitiveNode extends LGraphNode {
|
||||
this.outputs[0].name = 'connect to widget input'
|
||||
delete this.outputs[0].widget
|
||||
|
||||
this.#removeWidgets()
|
||||
this._removeWidgets()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user