Compare commits

...

11 Commits

Author SHA1 Message Date
Alexander Brown
d8c29f0922 Merge branch 'main' into pysssss/fix-nullgrapherror 2026-06-05 17:27:22 -07:00
Terry Jia
c695aa1ee0 fix(load3d): load Preview3DAdvanced / splat / pointcloud previews from temp/ (#12671)
BE is https://github.com/Comfy-Org/ComfyUI/pull/14294, need FE as well

Pair with backend
[BE-1172](https://github.com/Comfy-Org/ComfyUI/pull/14294). Two changes
bundled because both touch the same Preview3DAdvanced /
PreviewGaussianSplat / PreviewPointCloud

1. Backend now saves the incoming File3D into temp/ instead of output/
2. viewport input renamed from 'image' to 'viewport_state', Backend
renamed IO.Load3D.Input("image") to ("viewport_state") on the three
nodes (the field carries the viewport snapshot).
3. Exsting Load3D / Preview3D keep 'image' for workflow JSON
compatibility.
2026-06-05 20:04:09 -04:00
pythongosssss
f0894e2503 update test to match new behavior 2026-06-04 09:04:18 -07:00
pythongosssss
74f55a213e - dispatch node removed from clear, remove fallback
- add isDetached property
- refactor test to evaluateHandle removing window stores, casts
2026-06-04 08:22:18 -07:00
pythongosssss
deb181ab33 Merge remote-tracking branch 'origin/main' into pysssss/fix-nullgrapherror 2026-06-04 08:07:43 -07:00
pythongosssss
a7e4377c10 Merge remote-tracking branch 'origin/main' into pysssss/fix-nullgrapherror
# Conflicts:
#	src/core/graph/subgraph/promotionUtils.ts
#	src/lib/litegraph/src/infrastructure/LGraphEventMap.ts
#	src/lib/litegraph/src/subgraph/SubgraphNode.ts
2026-05-27 07:16:57 -07:00
pythongosssss
7d5cff4519 Merge branch 'main' into pysssss/fix-nullgrapherror 2026-05-22 19:47:37 +01:00
pythongosssss
1fb24d8927 improve comment 2026-05-22 03:01:31 -07:00
pythongosssss
aeb3b49ff2 Merge remote-tracking branch 'origin/main' into pysssss/fix-nullgrapherror
# Conflicts:
#	src/renderer/core/canvas/canvasStore.test.ts
2026-05-21 07:42:56 -07:00
pythongosssss
48907bdcb1 fix clear not removing data 2026-05-01 12:53:24 -07:00
pythongosssss
c29dd37de4 fix: prevent NullGraphError on subgraph node removal
- add pre-detach event (node:before-removed) so reactive consumers can drop references before node.graph is nulled
- move selection and Vue node-manager teardown to this event to eliminate stale panel/render evaluations against detached nodes
- guard SubgraphNode promoted-widget paths resilient on detached access and add regression coverage
2026-05-01 12:11:58 -07:00
18 changed files with 505 additions and 41 deletions

View File

@@ -1,6 +1,9 @@
import type { ConsoleMessage } from '@playwright/test'
import { expect } from '@playwright/test'
import type { ComfyPage } from '@e2e/fixtures/ComfyPage'
import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage'
import { TestIds } from '@e2e/fixtures/selectors'
import { getPseudoPreviewWidgets } from '@e2e/fixtures/utils/promotedWidgets'
const domPreviewSelector = '.image-preview'
@@ -95,4 +98,147 @@ test.describe('Subgraph Lifecycle', { tag: ['@subgraph'] }, () => {
await expect(comfyPage.page.locator(domPreviewSelector)).toHaveCount(0)
})
})
test.describe('Detach Race Repro', { tag: ['@vue-nodes'] }, () => {
const SUBGRAPH_NODE_TITLE = 'New Subgraph'
// Queues legacy onNodeRemoved/onSelectionChange so unpack completes first,
// widening the race window so a guard regression deterministically surfaces.
async function deferLegacyHandlers(comfyPage: ComfyPage) {
return await comfyPage.page.evaluateHandle(() => {
const graph = window.app!.graph!
const canvas = window.app!.canvas!
const queue: Array<() => void> = []
const originalNodeRemoved = graph.onNodeRemoved
const originalSelectionChange = canvas.onSelectionChange
graph.onNodeRemoved = function (node) {
queue.push(() => originalNodeRemoved?.call(this, node))
}
canvas.onSelectionChange = function (selected) {
queue.push(() => originalSelectionChange?.call(this, selected))
}
return {
drain: () => {
for (const fn of queue.splice(0)) fn()
},
restore: () => {
graph.onNodeRemoved = originalNodeRemoved
canvas.onSelectionChange = originalSelectionChange
}
}
})
}
function isNullGraphErrorText(text: string): boolean {
return text.includes('NullGraphError') || text.endsWith('has no graph')
}
// Vue's default errorHandler routes render throws to console.error,
// not pageerror - listen to both.
function captureNullGraphErrors(comfyPage: ComfyPage) {
const captured: string[] = []
const onPageError = (err: Error) => {
if (
err.name === 'NullGraphError' ||
isNullGraphErrorText(err.message ?? '')
) {
captured.push(`pageerror ${err.name}: ${err.message}`)
}
}
const onConsoleMessage = (msg: ConsoleMessage) => {
if (msg.type() !== 'error') return
const text = msg.text()
if (isNullGraphErrorText(text)) {
captured.push(`console.error: ${text}`)
}
}
comfyPage.page.on('pageerror', onPageError)
comfyPage.page.on('console', onConsoleMessage)
return {
getErrors: () => [...captured],
stop: () => {
comfyPage.page.off('pageerror', onPageError)
comfyPage.page.off('console', onConsoleMessage)
}
}
}
async function unpackViaContextMenu(comfyPage: ComfyPage, title: string) {
const fixture = await comfyPage.vueNodes.getFixtureByTitle(title)
await comfyPage.contextMenu.openForVueNode(fixture.header)
await comfyPage.contextMenu.clickMenuItemExact('Unpack Subgraph')
}
async function unpackAndCaptureErrors(
comfyPage: ComfyPage
): Promise<string[]> {
const subgraphNode =
comfyPage.vueNodes.getNodeByTitle(SUBGRAPH_NODE_TITLE)
const errors = captureNullGraphErrors(comfyPage)
const deferred = await deferLegacyHandlers(comfyPage)
try {
await unpackViaContextMenu(comfyPage, SUBGRAPH_NODE_TITLE)
await expect(subgraphNode).toHaveCount(0)
await deferred.evaluate((handlers) => handlers.drain())
// Let drained-handler reactive flushes settle before stop().
await comfyPage.nextFrame()
return errors.getErrors()
} finally {
await deferred.evaluate((handlers) => handlers.restore())
await deferred.dispose()
errors.stop()
}
}
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.settings.setSetting('Comfy.RightSidePanel.IsOpen', true)
await comfyPage.workflow.loadWorkflow(
'subgraphs/subgraph-with-promoted-text-widget'
)
const subgraphNode =
comfyPage.vueNodes.getNodeByTitle(SUBGRAPH_NODE_TITLE)
await expect(subgraphNode).toBeVisible()
const fixture =
await comfyPage.vueNodes.getFixtureByTitle(SUBGRAPH_NODE_TITLE)
await fixture.header.click()
await expect(
comfyPage.page.getByTestId(TestIds.propertiesPanel.root)
).toBeVisible()
await comfyPage.nextFrame()
})
test('unpack does not surface NullGraphError on the LGraphNode render path', async ({
comfyPage
}) => {
const nullGraphErrors = await unpackAndCaptureErrors(comfyPage)
expect(
nullGraphErrors,
'LGraphNode render path: detach race must not surface NullGraphError'
).toEqual([])
})
test('unpack does not surface NullGraphError from the TabSubgraphInputs panel', async ({
comfyPage
}) => {
const nullGraphErrors = await unpackAndCaptureErrors(comfyPage)
expect(
nullGraphErrors,
'TabSubgraphInputs panel: detach race must not surface NullGraphError'
).toEqual([])
})
test('unpack with subgraph editor open does not surface NullGraphError from the SubgraphEditor panel', async ({
comfyPage
}) => {
await comfyPage.page.getByTestId(TestIds.subgraphEditor.toggle).click()
await comfyPage.nextFrame()
const nullGraphErrors = await unpackAndCaptureErrors(comfyPage)
expect(
nullGraphErrors,
'SubgraphEditor panel: detach race must not surface NullGraphError'
).toEqual([])
})
})
})

View File

@@ -776,3 +776,55 @@ describe('reconcileNodeErrorFlags (via lastNodeErrors watcher)', () => {
expect(subgraphNode.has_errors).toBe(true)
})
})
describe('Pre-remove vueNodeData drain', () => {
beforeEach(() => {
setActivePinia(createTestingPinia({ stubActions: false }))
})
it('drops vueNodeData entry before node.onRemoved fires', () => {
const graph = new LGraph()
const node = new LGraphNode('test')
graph.add(node)
const { vueNodeData } = useGraphNodeManager(graph)
expect(vueNodeData.has(String(node.id))).toBe(true)
let dataPresentInOnRemoved: boolean | undefined
node.onRemoved = () => {
dataPresentInOnRemoved = vueNodeData.has(String(node.id))
}
graph.remove(node)
expect(
dataPresentInOnRemoved,
'vueNodeData entry must be cleared before node.onRemoved fires so reactive consumers cannot observe the detached node'
).toBe(false)
})
it('clears vueNodeData when LGraph.clear() dispatches node:before-removed for each node', () => {
const graph = new LGraph()
const nodeA = new LGraphNode('a')
const nodeB = new LGraphNode('b')
graph.add(nodeA)
graph.add(nodeB)
const { vueNodeData } = useGraphNodeManager(graph)
expect(vueNodeData.size).toBe(2)
const beforeRemovedSpy = vi.fn()
graph.events.addEventListener('node:before-removed', beforeRemovedSpy)
graph.clear()
expect(
beforeRemovedSpy,
'clear() must dispatch node:before-removed so reactive consumers can drop refs before nodes detach'
).toHaveBeenCalledTimes(2)
expect(
vueNodeData.size,
'node:before-removed listener must drain vueNodeData when clear() removes every node'
).toBe(0)
})
})

View File

@@ -631,27 +631,20 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
}
}
/**
* Handles node removal from the graph - cleans up all references
*/
const dropNodeReferences = (node: LGraphNode) => {
const id = String(node.id)
nodeRefs.delete(id)
vueNodeData.delete(id)
}
const handleNodeRemoved = (
node: LGraphNode,
originalCallback?: (node: LGraphNode) => void
) => {
const id = String(node.id)
// Remove node from layout store
setSource(LayoutSource.Canvas)
void deleteNode(id)
// Clean up all tracking references
nodeRefs.delete(id)
vueNodeData.delete(id)
// Call original callback if provided
if (originalCallback) {
originalCallback(node)
}
originalCallback?.(node)
}
/**
@@ -674,9 +667,6 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
}
}
/**
* Sets up event listeners - now simplified with extracted handlers
*/
const setupEventListeners = (): (() => void) => {
// Store original callbacks
const originalOnNodeAdded = graph.onNodeAdded
@@ -692,6 +682,16 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
handleNodeRemoved(node, originalOnNodeRemoved)
}
const beforeNodeRemovedListener = (
e: CustomEvent<{ node: LGraphNode }>
) => {
dropNodeReferences(e.detail.node)
}
graph.events.addEventListener(
'node:before-removed',
beforeNodeRemovedListener
)
const triggerHandlers: {
[K in LGraphTriggerAction]: (event: LGraphTriggerParam<K>) => void
} = {
@@ -840,12 +840,19 @@ export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
// Initialize state
syncWithGraph()
// Return cleanup function
return createCleanupFunction(
const cleanup = createCleanupFunction(
originalOnNodeAdded || undefined,
originalOnNodeRemoved || undefined,
originalOnTrigger || undefined
)
return () => {
graph.events.removeEventListener(
'node:before-removed',
beforeNodeRemovedListener
)
cleanup()
}
}
// Set up event listeners immediately

View File

@@ -137,6 +137,18 @@ describe(usePromotedPreviews, () => {
expect(promotedPreviews.value).toEqual([])
})
it('returns empty array (does not throw) when SubgraphNode is detached', () => {
const setup = createSetup()
const parentGraph = setup.subgraphNode.graph!
parentGraph.add(setup.subgraphNode)
parentGraph.remove(setup.subgraphNode)
expect(setup.subgraphNode.graph).toBeNull()
const { promotedPreviews } = usePromotedPreviews(() => setup.subgraphNode)
expect(() => promotedPreviews.value).not.toThrow()
expect(promotedPreviews.value).toEqual([])
})
it('returns empty array when no $$ promotions exist', () => {
const setup = createSetup()
addInteriorNode(setup, { id: 10 })

View File

@@ -68,6 +68,7 @@ export function usePromotedPreviews(
const promotedPreviews = computed((): PromotedPreview[] => {
const node = toValue(lgraphNode)
if (!(node instanceof SubgraphNode)) return []
if (node.isDetached) return []
const rootGraphId = node.rootGraph.id
const hostLocator = String(node.id)

View File

@@ -478,6 +478,22 @@ describe('hasUnpromotedWidgets', () => {
expect(hasUnpromotedWidgets(subgraphNode)).toBe(false)
})
it('returns false (does not throw) when SubgraphNode is detached', () => {
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
const parentGraph = subgraphNode.graph!
parentGraph.add(subgraphNode)
const interiorNode = new LGraphNode('InnerNode')
subgraph.add(interiorNode)
interiorNode.addWidget('text', 'seed', '123', () => {})
parentGraph.remove(subgraphNode)
expect(subgraphNode.graph).toBeNull()
expect(() => hasUnpromotedWidgets(subgraphNode)).not.toThrow()
expect(hasUnpromotedWidgets(subgraphNode)).toBe(false)
})
})
describe('isLinkedPromotion', () => {

View File

@@ -562,6 +562,7 @@ export function pruneDisconnected(subgraphNode: SubgraphNode) {
}
export function hasUnpromotedWidgets(subgraphNode: SubgraphNode): boolean {
if (subgraphNode.isDetached) return false
const { subgraph } = subgraphNode
return subgraph.nodes.some((interiorNode) =>

View File

@@ -173,7 +173,7 @@ function makePreview3DAdvancedNode(
constructor: { comfyClass: overrides.comfyClass ?? 'Preview3DAdvanced' },
size: [400, 550],
setSize: vi.fn(),
widgets: overrides.widgets ?? [{ name: 'image', value: '' }],
widgets: overrides.widgets ?? [{ name: 'viewport_state', value: '' }],
properties: overrides.properties ?? {}
} as unknown as LGraphNode
}
@@ -783,9 +783,9 @@ describe('Comfy.Preview3DAdvanced.nodeCreated', () => {
expect(load3dInstance.setCameraState).not.toHaveBeenCalled()
})
it('attaches a camera-only serializeValue to the image widget', async () => {
it('attaches a camera-only serializeValue to the viewport_state widget', async () => {
const { preview3DAdvancedExt } = await loadExtensionsFresh()
const widgets: FakeWidget[] = [{ name: 'image', value: '' }]
const widgets: FakeWidget[] = [{ name: 'viewport_state', value: '' }]
const node = makePreview3DAdvancedNode({ widgets })
await preview3DAdvancedExt.nodeCreated(node)
@@ -795,7 +795,7 @@ describe('Comfy.Preview3DAdvanced.nodeCreated', () => {
it('serializeValue returns live camera_info plus empty media fields, omitting model_3d_info when none', async () => {
const { preview3DAdvancedExt } = await loadExtensionsFresh()
const widgets: FakeWidget[] = [{ name: 'image', value: '' }]
const widgets: FakeWidget[] = [{ name: 'viewport_state', value: '' }]
const node = makePreview3DAdvancedNode({ widgets })
const load3d = makeLoad3dMock()
@@ -819,7 +819,7 @@ describe('Comfy.Preview3DAdvanced.nodeCreated', () => {
it('serializeValue wraps a present getModelInfo result in a single-element list', async () => {
const { preview3DAdvancedExt } = await loadExtensionsFresh()
const widgets: FakeWidget[] = [{ name: 'image', value: '' }]
const widgets: FakeWidget[] = [{ name: 'viewport_state', value: '' }]
const node = makePreview3DAdvancedNode({ widgets })
const load3d = makeLoad3dMock()

View File

@@ -270,8 +270,16 @@ useExtensionService().registerExtension({
}
],
getCustomWidgets() {
const VIEWPORT_STATE_NODES = new Set([
'Preview3DAdvanced',
'PreviewGaussianSplat',
'PreviewPointCloud'
])
return {
LOAD_3D(node) {
const inputName = VIEWPORT_STATE_NODES.has(node.constructor.comfyClass)
? 'viewport_state'
: 'image'
const hasModelFileWidget = node.widgets?.some(
(w) => w.name === 'model_file'
)
@@ -316,9 +324,9 @@ useExtensionService().registerExtension({
const widget = new ComponentWidgetImpl({
node: node,
name: 'image',
name: inputName,
component: Load3D,
inputSpec: inputSpecLoad3D,
inputSpec: { ...inputSpecLoad3D, name: inputName },
options: {}
})
@@ -715,7 +723,7 @@ useExtensionService().registerExtension({
})
useLoad3d(node).waitForLoad3d((load3d) => {
const sceneWidget = node.widgets?.find((w) => w.name === 'image')
const sceneWidget = node.widgets?.find((w) => w.name === 'viewport_state')
if (!sceneWidget) return
const resolveLoad3d = () => nodeToLoad3dMap.get(node) ?? load3d

View File

@@ -186,7 +186,7 @@ describe('Comfy.PreviewGaussianSplat.nodeCreated', () => {
expect(node.properties['Last Time Model File']).toBe('scene.ply')
expect(configureForSaveMeshMock).toHaveBeenLastCalledWith(
'output',
'temp',
'scene.ply',
expect.objectContaining({ silentOnNotFound: true })
)
@@ -231,7 +231,7 @@ describe('Comfy.PreviewGaussianSplat.nodeCreated', () => {
const node = makePreviewNode({
widgets: [
{ name: 'model_file', value: '' },
{ name: 'image', value: '' },
{ name: 'viewport_state', value: '' },
widthWidget,
heightWidget
]
@@ -262,7 +262,7 @@ describe('Comfy.PreviewGaussianSplat.nodeCreated', () => {
)
const sceneWidget: FakeWidget & {
serializeValue?: () => Promise<unknown>
} = { name: 'image', value: '' }
} = { name: 'viewport_state', value: '' }
const node = makePreviewNode({
widgets: [{ name: 'model_file', value: '' }, sceneWidget]
})
@@ -318,7 +318,7 @@ describe('Comfy.PreviewPointCloud.nodeCreated', () => {
expect(node.properties['Last Time Model File']).toBe('pointcloud.ply')
expect(configureForSaveMeshMock).toHaveBeenLastCalledWith(
'output',
'temp',
'pointcloud.ply',
expect.objectContaining({ silentOnNotFound: true })
)

View File

@@ -46,7 +46,7 @@ function applyResultToLoad3d(
}
const config = new Load3DConfiguration(load3d, node.properties)
config.configureForSaveMesh('output', normalizedPath, {
config.configureForSaveMesh('temp', normalizedPath, {
silentOnNotFound: true
})
@@ -119,7 +119,7 @@ function createPreview3DExtension(
if (!lastTimeModelFile) return
const config = new Load3DConfiguration(load3d, node.properties)
config.configureForSaveMesh('output', lastTimeModelFile as string, {
config.configureForSaveMesh('temp', lastTimeModelFile as string, {
silentOnNotFound: true
})
@@ -136,7 +136,9 @@ function createPreview3DExtension(
})
waitForLoad3d((load3d) => {
const sceneWidget = node.widgets?.find((w) => w.name === 'image')
const sceneWidget = node.widgets?.find(
(w) => w.name === 'viewport_state'
)
const widthWidget = node.widgets?.find((w) => w.name === 'width')
const heightWidget = node.widgets?.find((w) => w.name === 'height')

View File

@@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { NodeId, Subgraph } from '@/lib/litegraph/src/litegraph'
import {
LGraph,
LGraphGroup,
LGraphNode,
LiteGraph,
LLink,
@@ -325,6 +326,96 @@ describe('Graph Clearing and Callbacks', () => {
})
})
describe('node:before-removed event', () => {
it('fires node:before-removed for a successful node removal', () => {
const graph = new LGraph()
const node = new LGraphNode('test')
graph.add(node)
const events: { node: LGraphNode; graphAtDispatch: unknown }[] = []
graph.events.addEventListener('node:before-removed', (e) => {
events.push({
node: e.detail.node,
graphAtDispatch: e.detail.node.graph
})
})
graph.remove(node)
expect(events).toHaveLength(1)
expect(events[0].node).toBe(node)
expect(events[0].graphAtDispatch).toBe(graph)
expect(node.graph).toBeNull()
})
it('does not fire node:before-removed for a node not in the graph', () => {
const graph = new LGraph()
const node = new LGraphNode('test')
const fired = vi.fn()
graph.events.addEventListener('node:before-removed', fired)
graph.remove(node)
expect(fired).not.toHaveBeenCalled()
})
it('does not fire node:before-removed when removing an LGraphGroup', () => {
const graph = new LGraph()
const group = new LGraphGroup('test-group')
graph.add(group)
const fired = vi.fn()
graph.events.addEventListener('node:before-removed', fired)
graph.remove(group)
expect(fired).not.toHaveBeenCalled()
})
it('does not fire node:before-removed when ignore_remove is set', () => {
const graph = new LGraph()
const node = new LGraphNode('test')
graph.add(node)
node.ignore_remove = true
const fired = vi.fn()
graph.events.addEventListener('node:before-removed', fired)
graph.remove(node)
expect(fired).not.toHaveBeenCalled()
expect(graph.nodes).toContain(node)
})
it('fires node:before-removed before node.onRemoved and detach', () => {
const graph = new LGraph()
const node = new LGraphNode('test')
graph.add(node)
const order: string[] = []
graph.events.addEventListener('node:before-removed', () => {
order.push(
`before-removed(graph=${node.graph === graph ? 'set' : 'null'})`
)
})
node.onRemoved = () => {
order.push(`onRemoved(graph=${node.graph === graph ? 'set' : 'null'})`)
}
graph.onNodeRemoved = (n) => {
order.push(`onNodeRemoved(graph=${n.graph === null ? 'null' : 'set'})`)
}
graph.remove(node)
expect(order).toEqual([
'before-removed(graph=set)',
'onRemoved(graph=set)',
'onNodeRemoved(graph=null)'
])
})
})
describe('Subgraph Definition Garbage Collection', () => {
beforeEach(() => {
setActivePinia(createTestingPinia({ stubActions: false }))
@@ -377,6 +468,53 @@ describe('Subgraph Definition Garbage Collection', () => {
expect(graphRemovedNodeIds.size).toBe(2)
})
it('subgraph-definition GC dispatches node:before-removed on the inner subgraph for each inner node', () => {
const rootGraph = new LGraph()
const { subgraph, innerNodes } = createSubgraphWithNodes(rootGraph, 2)
const dispatched: { node: LGraphNode; graphAtDispatch: unknown }[] = []
subgraph.events.addEventListener('node:before-removed', (e) => {
dispatched.push({
node: e.detail.node,
graphAtDispatch: e.detail.node.graph
})
})
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
rootGraph.add(subgraphNode)
rootGraph.remove(subgraphNode)
expect(dispatched.map((e) => e.node)).toEqual(innerNodes)
for (const entry of dispatched) {
expect(entry.graphAtDispatch).toBe(subgraph)
}
})
it('subgraph-definition GC dispatches node:before-removed before each inner node onRemoved', () => {
const rootGraph = new LGraph()
const { subgraph, innerNodes } = createSubgraphWithNodes(rootGraph, 1)
const innerNode = innerNodes[0]
const order: string[] = []
subgraph.events.addEventListener('node:before-removed', () => {
order.push('before-removed')
})
innerNode.onRemoved = () => {
order.push('onRemoved')
}
subgraph.onNodeRemoved = () => {
order.push('onNodeRemoved')
}
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
rootGraph.add(subgraphNode)
rootGraph.remove(subgraphNode)
expect(order).toEqual(['before-removed', 'onRemoved', 'onNodeRemoved'])
})
it('subgraph definition is removed when SubgraphNode is removed', () => {
const rootGraph = new LGraph()
const { subgraph } = createSubgraphWithNodes(rootGraph, 1)

View File

@@ -175,6 +175,13 @@ export interface BaseLGraph {
readonly rootGraph: LGraph
}
function fireNodeRemovalLifecycle(node: LGraphNode): void {
const graph: LGraph | null = node.graph
graph?.events.dispatch('node:before-removed', { node })
node.onRemoved?.()
graph?.onNodeRemoved?.(node)
}
/**
* LGraph is the class that contain a full graph. We instantiate one and add nodes to it, and then we can run the execution loop.
* supported callbacks:
@@ -406,8 +413,7 @@ export class LGraph
// safe clear
if (this._nodes) {
for (const _node of this._nodes) {
_node.onRemoved?.()
this.onNodeRemoved?.(_node)
fireNodeRemovalLifecycle(_node)
}
}
@@ -1070,6 +1076,8 @@ export class LGraph
// sure? - almost sure is wrong
this.beforeChange()
this.events.dispatch('node:before-removed', { node })
const { inputs, outputs } = node
// disconnect inputs
@@ -1105,10 +1113,7 @@ export class LGraph
)
if (!hasRemainingReferences) {
forEachNode(node.subgraph, (innerNode) => {
innerNode.onRemoved?.()
innerNode.graph?.onNodeRemoved?.(innerNode)
})
forEachNode(node.subgraph, fireNodeRemovalLifecycle)
this.rootGraph.subgraphs.delete(node.subgraph.id)
}
}

View File

@@ -1,5 +1,5 @@
import type { LGraph } from '@/lib/litegraph/src/LGraph'
import type { NodeId } from '@/lib/litegraph/src/LGraphNode'
import type { LGraphNode, NodeId } from '@/lib/litegraph/src/LGraphNode'
import type { LLink, ResolvedConnection } from '@/lib/litegraph/src/LLink'
import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
import type { Subgraph } from '@/lib/litegraph/src/subgraph/Subgraph'
@@ -51,6 +51,13 @@ export interface LGraphEventMap {
closingGraph: LGraph | Subgraph
}
/**
* Fires on the owning graph before per-node teardown begins
*/
'node:before-removed': {
node: LGraphNode
}
'node:property:changed': {
nodeId: NodeId
property: string

View File

@@ -79,6 +79,19 @@ describe('SubgraphNode Construction', () => {
expect(subgraphNode.graph).toBeNull()
})
it('should return empty widgets array (not throw) after removal', () => {
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
const parentGraph = subgraphNode.graph!
parentGraph.add(subgraphNode)
parentGraph.remove(subgraphNode)
expect(subgraphNode.graph).toBeNull()
expect(() => subgraphNode.widgets).not.toThrow()
expect(subgraphNode.widgets).toEqual([])
})
subgraphTest(
'should synchronize slots with subgraph definition',
({ subgraphWithNode }) => {

View File

@@ -77,6 +77,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
return this.graph.rootGraph
}
get isDetached(): boolean {
return !this.graph
}
override get displayType(): string {
return 'Subgraph node'
}
@@ -199,6 +203,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
}
private _getPromotedViews(): PromotedWidgetView[] {
if (this.isDetached) return []
const cachedViews = this._promotedViewsCache
if (cachedViews?.version === this._cacheVersion) return cachedViews.views

View File

@@ -1,7 +1,10 @@
import { createTestingPinia } from '@pinia/testing'
import { setActivePinia } from 'pinia'
import { nextTick } from 'vue'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { LGraphCanvas, Positionable } from '@/lib/litegraph/src/litegraph'
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
import { LGraphGroup } from '@/lib/litegraph/src/LGraphGroup'
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
@@ -85,6 +88,42 @@ describe('useCanvasStore', () => {
expect(originalHandler).toHaveBeenCalledWith(2.0, app.canvas.ds.offset)
})
})
describe('node:before-removed selection cleanup', () => {
it('removes the node from store.selectedItems before its onRemoved fires', async () => {
const graph = new LGraph()
const node = new LGraphNode('test')
graph.add(node)
const selectedItems = new Set<Positionable>([node])
const fakeCanvas = {
canvas: document.createElement('canvas'),
graph,
selectedItems,
deselect: vi.fn((item: Positionable) => {
selectedItems.delete(item)
})
}
store.canvas = fakeCanvas as unknown as LGraphCanvas
await nextTick()
store.updateSelectedItems()
expect(store.selectedItems).toContain(node)
let stillSelectedInOnRemoved: boolean | undefined
node.onRemoved = () => {
stillSelectedInOnRemoved = store.selectedItems.includes(node)
}
graph.remove(node)
expect(
stillSelectedInOnRemoved,
'selectedItems must not contain the node when onRemoved fires'
).toBe(false)
expect(store.selectedItems).toEqual([])
})
})
it('Does not include groups in selected nodeIds', async () => {
store.selectedItems = [new LGraphGroup()]

View File

@@ -131,6 +131,18 @@ export const useCanvasStore = defineStore('canvas', () => {
whenever(
() => canvas.value,
(newCanvas) => {
currentGraph.value = newCanvas.graph
// Scoped to the on-screen graph: selection only holds items from it,
// so removals in other graphs can't affect the live selection.
useEventListener(
() => currentGraph.value?.events,
'node:before-removed',
(e: CustomEvent<{ node: LGraphNode }>) => {
newCanvas.deselect(e.detail.node)
updateSelectedItems()
}
)
useEventListener(
newCanvas.canvas,
'litegraph:set-graph',