Background
During review of PR #12872 (fix: bind replacement node widgets to reused node id), @AustinMroz approved the change but raised three remaining concerns about the node replacement code in src/platform/nodeReplacement/useNodeReplacement.ts that were explicitly noted as out of scope for that PR.
Concerns
1. Widget configuring logic is flawed
Rather than iterating over inputMapping, the code needs to (carefully) iterate over newNode's widgets and query inputMapping for the replacement value. The current approach may miss widgets not present in inputMapping or incorrectly skip widgets that should be configured.
2. node.onAdded is not called
During the node replacement flow, node.onAdded is never invoked. This lifecycle hook is normally called when a node is added to a graph via LGraph.add(), and skipping it may cause initialization side-effects to be missed for the replacement node.
3. Bypassing graph.add() is brittle
The current implementation writes directly to graph._nodes and graph._nodes_by_id instead of going through the normal LGraph.add() flow. This is fragile and makes future code divergence risky, as any new logic added to LGraph.add() would be silently skipped during node replacement.
References
Requested by
@jaeone94
Background
During review of PR #12872 (fix: bind replacement node widgets to reused node id), @AustinMroz approved the change but raised three remaining concerns about the node replacement code in
src/platform/nodeReplacement/useNodeReplacement.tsthat were explicitly noted as out of scope for that PR.Concerns
1. Widget configuring logic is flawed
Rather than iterating over
inputMapping, the code needs to (carefully) iterate overnewNode's widgets and queryinputMappingfor the replacement value. The current approach may miss widgets not present ininputMappingor incorrectly skip widgets that should be configured.2.
node.onAddedis not calledDuring the node replacement flow,
node.onAddedis never invoked. This lifecycle hook is normally called when a node is added to a graph viaLGraph.add(), and skipping it may cause initialization side-effects to be missed for the replacement node.3. Bypassing
graph.add()is brittleThe current implementation writes directly to
graph._nodesandgraph._nodes_by_idinstead of going through the normalLGraph.add()flow. This is fragile and makes future code divergence risky, as any new logic added toLGraph.add()would be silently skipped during node replacement.References
Requested by
@jaeone94