mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: preserve prototype getter/setter in property instrumentation
LGraphNodeProperties was shadowing LGraphNode's prototype shape getter/setter with a closure-based own accessor. This caused node.shape assignments to store values in a closure variable instead of node._shape, making renderingShape always fall back to the default round shape. Skip instrumentation when a prototype accessor already exists, since the prototype setter already emits change events. Fixes #8532
This commit is contained in:
@@ -68,6 +68,37 @@ describe('LGraphNodeProperties', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('prototype accessor preservation', () => {
|
||||
it('should not shadow prototype getter/setter with closure-based accessor', () => {
|
||||
// Mirrors LGraphNode.shape / _shape pattern
|
||||
class NodeWithShape {
|
||||
_shape: number | undefined = undefined
|
||||
id = 1
|
||||
flags = {}
|
||||
graph = mockGraph
|
||||
title = 'test'
|
||||
get shape(): number | undefined {
|
||||
return this._shape
|
||||
}
|
||||
set shape(v: number) {
|
||||
this._shape = v
|
||||
}
|
||||
}
|
||||
|
||||
const node = new NodeWithShape()
|
||||
new LGraphNodeProperties(node as Partial<LGraphNode> as LGraphNode)
|
||||
|
||||
// The prototype getter/setter should NOT be shadowed
|
||||
expect(Object.prototype.hasOwnProperty.call(node, 'shape')).toBe(false)
|
||||
|
||||
// Before fix: the closure-based accessor would shadow the prototype,
|
||||
// storing the value in a closure and leaving _shape unchanged.
|
||||
node.shape = 42
|
||||
expect(node._shape).toBe(42)
|
||||
expect(node.shape).toBe(42)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isTracked', () => {
|
||||
it('should correctly identify tracked properties', () => {
|
||||
const propManager = new LGraphNodeProperties(mockNode)
|
||||
|
||||
@@ -89,6 +89,16 @@ export class LGraphNodeProperties {
|
||||
const currentValue = targetObject[propertyName]
|
||||
|
||||
if (!hasProperty) {
|
||||
// Check if a prototype in the chain defines a getter/setter for this
|
||||
// property. Defining an own closure-based accessor would shadow the
|
||||
// prototype accessor and break its internal logic (e.g. the `shape`
|
||||
// setter that writes `_shape`). Skip instrumentation in that case –
|
||||
// the prototype setter is expected to emit its own change events.
|
||||
if (this._hasPrototypeAccessor(targetObject, propertyName)) {
|
||||
this._instrumentedPaths.add(path)
|
||||
return
|
||||
}
|
||||
|
||||
let value: unknown = undefined
|
||||
|
||||
Object.defineProperty(targetObject, propertyName, {
|
||||
@@ -128,6 +138,23 @@ export class LGraphNodeProperties {
|
||||
this._instrumentedPaths.add(path)
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether any prototype in the chain defines a getter/setter for
|
||||
* the given property.
|
||||
*/
|
||||
private _hasPrototypeAccessor(
|
||||
obj: Record<string, unknown>,
|
||||
propertyName: string
|
||||
): boolean {
|
||||
let proto = Object.getPrototypeOf(obj)
|
||||
while (proto) {
|
||||
const desc = Object.getOwnPropertyDescriptor(proto, propertyName)
|
||||
if (desc && (desc.get || desc.set)) return true
|
||||
proto = Object.getPrototypeOf(proto)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a property descriptor that emits change events
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user