Skip to content
Merged
9 changes: 5 additions & 4 deletions src/lib/litegraph/src/subgraph/SubgraphNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import type {
} from '@/lib/litegraph/src/types/serialisation'
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
import type { UUID } from '@/lib/litegraph/src/utils/uuid'
import { BaseWidget } from '@/lib/litegraph/src/widgets/BaseWidget'
import { AssetWidget } from '@/lib/litegraph/src/widgets/AssetWidget'
import { toConcreteWidget } from '@/lib/litegraph/src/widgets/widgetMap'

import { ExecutableNodeDTO } from './ExecutableNodeDTO'
import type { ExecutableLGraphNode, ExecutionId } from './ExecutableNodeDTO'
Expand Down Expand Up @@ -331,9 +331,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
inputWidget: IWidgetLocator | undefined
) {
// Use the first matching widget
const promotedWidget = toConcreteWidget(widget, this).createCopyForNode(
this
)
const promotedWidget =
widget instanceof BaseWidget
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[architecture] high Priority

Issue: Type safety concern - no runtime type checking for BaseWidget instanceof check
Context: The instanceof check relies on runtime type information which could fail if widgets are created through different contexts or loaded modules
Suggestion: Add explicit type checking or use duck typing to verify the widget has createCopyForNode method before calling it

? widget.createCopyForNode(this)
: { ...widget, node: this }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[quality] medium Priority

Issue: Inconsistent widget copying strategies may lead to different behavior
Context: BaseWidget instances use createCopyForNode() which may have different copying semantics than object spread with node assignment
Suggestion: Document the differences between these approaches or consider unifying the widget copying strategy to ensure consistent behavior

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security] low Priority

Issue: Object spread may inadvertently copy unsafe properties from untrusted widgets
Context: The spread operator {...widget} copies all enumerable properties, potentially including functions or references that weren't intended to be cloned
Suggestion: Use a more explicit copying approach that only copies known safe properties, similar to how BaseWidget.constructor already filters out unsafe properties

if (widget instanceof AssetWidget)
promotedWidget.options.nodeType ??= widget.node.type

Expand Down