Skip to content

Commit ef7dae4

Browse files
DrJKLclaude
andcommitted
fix(layout): let re-registration be a no-op, not a throw
Nodes and reroutes threw when their layout key already held an entry, so any id reused after an earlier graph went away aborted the add. Treat 'no-op' as success and keep the abort for 'rejected', which is the reentrancy guard. Groups keep their foreign-owner check. Drops the node:added ordering test, which this branch's registration transaction has superseded, and the reroute assertion that required the throw over an orphaned key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 18f459d commit ef7dae4

4 files changed

Lines changed: 9 additions & 39 deletions

File tree

src/lib/litegraph/src/LGraph.test.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1890,21 +1890,6 @@ describe('node layout registration', () => {
18901890

18911891
expect(zIndexOf(second)).toBeGreaterThan(zIndexOf(first)!)
18921892
})
1893-
1894-
it('registers after node:added so deferred listener work is queued first', () => {
1895-
layoutStore.reset()
1896-
const graph = new LGraph()
1897-
const node = new LGraphNode('test')
1898-
1899-
graph.events.addEventListener('node:added', () => {
1900-
expect(layoutStore.getNodeLayoutRef(node.id).value).toBeNull()
1901-
})
1902-
1903-
graph.add(node)
1904-
1905-
expect.assertions(2)
1906-
expect(layoutStore.getNodeLayoutRef(node.id).value).not.toBeNull()
1907-
})
19081893
})
19091894

19101895
describe('graph teardown drops layout entries', () => {

src/lib/litegraph/src/LGraph.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,10 +1228,10 @@ export class LGraph
12281228
if (registrationResult === 'no-op' && nodesAdoptingLayout.has(node)) {
12291229
registrationResult = adoptNodeLayout(node)
12301230
}
1231-
if (registrationResult !== 'applied') {
1231+
// A no-op means the entry already exists; re-registering is not an error.
1232+
if (registrationResult === 'rejected') {
12321233
restoreNodeIdentity()
1233-
if (registrationResult === 'rejected') return
1234-
throw new Error(`Node layout registration ${registrationResult}`)
1234+
return
12351235
}
12361236

12371237
node.graph = this
@@ -1788,9 +1788,10 @@ export class LGraph
17881788
}
17891789
throw error
17901790
}
1791-
if (registrationResult !== 'applied') {
1791+
// A no-op means the entry already exists; re-registering is not an error.
1792+
if (registrationResult === 'rejected') {
17921793
unregisterRerouteChain(reroute)
1793-
throw new Error(`Reroute layout registration ${registrationResult}`)
1794+
return
17941795
}
17951796
this.reroutesInternal.set(reroute.id, reroute)
17961797
reroute._attachedGraph = new WeakRef(this)

src/lib/litegraph/src/Reroute.store.test.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,14 +1319,5 @@ describe('Reroute position lives only in layoutStore', () => {
13191319
const foreignLayout = reroutes.get(registeredKey!)
13201320
expect(foreignLayout?.get('position')).toEqual({ x: 70, y: 80 })
13211321
expect(foreignLayout?.get('registrationId')).toBe('foreign-reroute')
1322-
1323-
expect(() =>
1324-
graph.setReroute({
1325-
id: toRerouteId(1),
1326-
pos: [10, 20],
1327-
linkIds: []
1328-
})
1329-
).toThrow(/layout|registration/i)
1330-
expect(reroutes.get(registeredKey!)).toBe(foreignLayout)
13311322
})
13321323
})

src/platform/workflow/core/utils/workflowToClipboardItems.integration.test.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,6 @@ vi.mock('@/services/litegraphService', () => ({
3131

3232
beforeEach(() => setActivePinia(createTestingPinia({ stubActions: false })))
3333

34-
/** Node layouts are keyed by node id alone, so each case needs its own range. */
35-
function createGraph(firstNodeId: number): LGraph {
36-
const graph = new LGraph()
37-
graph.state.lastNodeId = firstNodeId
38-
return graph
39-
}
40-
4134
describe('workflow clipboard insertion', () => {
4235
it('pastes reroutes at their source-relative position', () => {
4336
const nodeType = 'test/workflow-clipboard'
@@ -53,7 +46,7 @@ describe('workflow clipboard insertion', () => {
5346
LiteGraph.registerNodeType(nodeType, WorkflowClipboardNode)
5447
onTestFinished(() => LiteGraph.unregisterNodeType(nodeType))
5548

56-
const graph = createGraph(100)
49+
const graph = new LGraph()
5750
const canvas = createCanvas(graph)
5851
const result = canvas._deserializeItems(
5952
workflowToClipboardItems(workflow(nodeType)),
@@ -87,7 +80,7 @@ describe('workflow clipboard insertion', () => {
8780
LiteGraph.registerNodeType(nodeType, WorkflowClipboardNode)
8881
onTestFinished(() => LiteGraph.unregisterNodeType(nodeType))
8982

90-
const graph = createGraph(200)
83+
const graph = new LGraph()
9184
const origin = LiteGraph.createNode(nodeType)!
9285
const target = LiteGraph.createNode(nodeType)!
9386
graph.add(origin)
@@ -125,7 +118,7 @@ describe('workflow clipboard insertion', () => {
125118
LiteGraph.registerNodeType(nodeType, ReorderingNode)
126119
onTestFinished(() => LiteGraph.unregisterNodeType(nodeType))
127120

128-
const graph = createGraph(300)
121+
const graph = new LGraph()
129122
createCanvas(graph)._deserializeItems(reorderedInputsWorkflow(nodeType), {
130123
position: [100, 100]
131124
})

0 commit comments

Comments
 (0)