Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import {
LGraphNode,
LiteGraph
} from '@/lib/litegraph/src/litegraph'
import { createMockCanvasRenderingContext2D } from '@/utils/__tests__/litegraphTestUtils'
import {
createMockCanvasRenderingContext2D,
reloadSerializedGraph
} from '@/utils/__tests__/litegraphTestUtils'
import { renameWidget } from '@/utils/widgetUtil'

/**
Expand Down Expand Up @@ -38,6 +41,14 @@ LiteGraph.registerNodeType('test/CLIPTextEncodeLike', ClipTextEncodeLikeNode)
* — the channel the label round-trips through. These tests drive the real
* `renameWidget` (never hand-setting `widget.label`) and assert the label
* survives a real serialize -> configure round-trip.
*
* The reload tests must go through {@link reloadSerializedGraph}. A plain
* `new LGraph().configure(graph.serialize())` is vacuous here: `configure`
* re-adopts the payload's graph id, and `widgetValueStore` is keyed
* `graphId:nodeId:name`, so the reloaded node re-adopts the source graph's
* still-live widget state and the assertion passes without reading the JSON.
* Reintroducing this file's own root cause used to leave three of these five
* tests green.
*/
describe('renameWidget label persistence via input lookup (regression #13861)', () => {
beforeEach(() => {
Expand Down Expand Up @@ -90,8 +101,10 @@ describe('renameWidget label persistence via input lookup (regression #13861)',
const node = addClipNode(graph)
renameWidget(node.widgets![0], node, 'Positive Prompt')

const restored = new LGraph()
restored.configure(graph.serialize())
const restored = reloadSerializedGraph(
graph.serialize(),
() => new LGraph()
)

const restoredNode = restored.getNodeById(node.id)!
expect(restoredNode.widgets![0].label).toBe('Positive Prompt')
Expand All @@ -106,8 +119,7 @@ describe('renameWidget label persistence via input lookup (regression #13861)',
graph.remove(node)
expect(graph.getNodeById(node.id)).toBeFalsy()

const restored = new LGraph()
restored.configure(undoSnapshot)
const restored = reloadSerializedGraph(undoSnapshot, () => new LGraph())

expect(restored.getNodeById(node.id)!.widgets![0].label).toBe(
'Positive Prompt'
Expand All @@ -116,16 +128,27 @@ describe('renameWidget label persistence via input lookup (regression #13861)',

test('clearing a rename reverts the label to its default after round-trip', () => {
const graph = new LGraph()
const node = addClipNode(graph)
renameWidget(node.widgets![0], node, 'Positive Prompt')
renameWidget(node.widgets![0], node, '')
const cleared = addClipNode(graph)
renameWidget(cleared.widgets![0], cleared, 'Positive Prompt')
renameWidget(cleared.widgets![0], cleared, '')

// Control arm: an un-cleared rename in the same payload. Without it, a
// build where renaming never persisted at all would also report
// `undefined` here and the test would pass for the wrong reason.
Comment thread
DrJKL marked this conversation as resolved.
Outdated
const kept = addClipNode(graph)
renameWidget(kept.widgets![0], kept, 'Negative Prompt')

expect(node.inputs![0].label).toBeUndefined()
expect(cleared.inputs![0].label).toBeUndefined()

const restored = new LGraph()
restored.configure(graph.serialize())
const restored = reloadSerializedGraph(
graph.serialize(),
() => new LGraph()
)

expect(restored.getNodeById(node.id)!.widgets![0].label).toBeUndefined()
expect(restored.getNodeById(cleared.id)!.widgets![0].label).toBeUndefined()
expect(restored.getNodeById(kept.id)!.widgets![0].label).toBe(
'Negative Prompt'
)
})

test('label survives copy -> paste', () => {
Expand Down
77 changes: 75 additions & 2 deletions src/lib/litegraph/src/subgraph/SubgraphNode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* Tests for SubgraphNode instances including construction,
* IO synchronization, and edge cases.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'
import { fromPartial } from '@total-typescript/shoehorn'

import {
Expand Down Expand Up @@ -398,7 +398,13 @@ describe('SubgraphNode Synchronization', () => {
)
})

it('should preserve renamed label through serialize/configure round-trip', () => {
// Not a round trip. `SubgraphNode.configure` rebuilds `this.inputs` from the
// live `subgraph.inputNode.slots`, and `widgetValueStore` is keyed
// `graphId:nodeId:name` while `configure` re-adopts the payload's graph id —
// so reconfiguring the same node in place reads live state, not JSON. This
// test survives `serialize()` dropping every input label. The reload
// assertion lives in the sibling test below.
Comment thread
DrJKL marked this conversation as resolved.
Outdated
it('keeps the renamed label after an in-place reconfigure', () => {
const subgraph = createTestSubgraph({
inputs: [{ name: 'seed', type: 'INT' }]
})
Expand Down Expand Up @@ -440,6 +446,73 @@ describe('SubgraphNode Synchronization', () => {
'My Seed'
)
})

it('preserves a renamed label across a real definition reload', () => {
const subgraph = createTestSubgraph({
inputs: [{ name: 'seed', type: 'INT' }]
})

// A registered type: `configure` will not re-create an unregistered node,
// so an anonymous interior node makes the reload silently empty.
class InteriorNode extends LGraphNode {
static override title = 'Interior'
constructor() {
super('Interior')
const slot = this.addInput('value', 'INT')
slot.widget = { name: 'value' }
this.addOutput('out', 'INT')
this.addWidget('number', 'value', 0, () => {})
}
}
LiteGraph.registerNodeType('test/interior-label-reload', InteriorNode)
onTestFinished(() => {
delete LiteGraph.registered_node_types['test/interior-label-reload']
})

const interiorNode = LiteGraph.createNode('test/interior-label-reload')!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How do you feel about a const for this to make sure they're all consistently aligned?

subgraph.add(interiorNode)
subgraph.inputNode.slots[0].connect(interiorNode.inputs[0], interiorNode)

const subgraphNode = createTestSubgraphNode(subgraph, { id: 101 })
subgraph.inputs[0].label = 'My Seed'
subgraphNode.inputs[0].label = 'My Seed'
subgraph.events.dispatch('renaming-input', {
input: subgraph.inputs[0],
index: 0,
oldName: 'seed',
newName: 'My Seed'
})

// The label must survive as JSON, not as a live object reference: the
// subgraph *definition* carries it, and the reloaded host reads it back
// through the reloaded definition. `SubgraphNode.configure` rebuilds
// `this.inputs` from the live subgraph slots and never reads
// `info.inputs`, so a same-object reconfigure proves nothing.
Comment thread
DrJKL marked this conversation as resolved.
Outdated
const definition = JSON.parse(
JSON.stringify(subgraph.asSerialisable())
) as ReturnType<typeof subgraph.asSerialisable>
const instance = JSON.parse(
JSON.stringify(subgraphNode.serialize())
) as ExportedSubgraphInstance

// Drop the live widget state so the reloaded host cannot re-adopt it:
// `configure` re-adopts the payload's graph id and `widgetValueStore` is
// keyed `graphId:nodeId:name`.
Comment thread
DrJKL marked this conversation as resolved.
Outdated
useWidgetValueStore().clearGraph(subgraphNode.rootGraph.id)

const reloadedSubgraph = createTestSubgraph({
rootGraph: subgraphNode.rootGraph
})
reloadedSubgraph.configure(definition)

const reloadedHost = createTestSubgraphNode(reloadedSubgraph, { id: 101 })
reloadedHost.configure(instance)

expect(reloadedHost.widgets).toMatchObject([
{ name: 'seed', label: 'My Seed' }
])
expect(reloadedHost.inputs[0].label).toBe('My Seed')
})
})

describe('SubgraphNode widget name collision on rename', () => {
Expand Down
38 changes: 37 additions & 1 deletion src/utils/__tests__/litegraphTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import type { LGraphEventMap } from '@/lib/litegraph/src/infrastructure/LGraphEv
import { Rectangle } from '@/lib/litegraph/src/infrastructure/Rectangle'
import type {
CanvasPointerEvent,
ISerialisedGraph,
LGraph,
LGraphCanvas,
LGraphGroup,
LinkNetwork,
LLink
LLink,
SerialisableGraph
} from '@/lib/litegraph/src/litegraph'
import { LGraphEventMode, LGraphNode } from '@/lib/litegraph/src/litegraph'
import { fromPartial } from '@total-typescript/shoehorn'
Expand All @@ -25,6 +27,8 @@ import { toLinkId } from '@/types/linkId'
import type { NodeId } from '@/types/nodeId'
import { toNodeId } from '@/types/nodeId'
import type { NodeState } from '@/types/nodeState'
import { usePreviewExposureStore } from '@/stores/previewExposureStore'
import { useWidgetValueStore } from '@/stores/widgetValueStore'
import { zeroUuid } from '@/utils/uuid'

/** Creates a node shell state with minimal required fields. */
Expand Down Expand Up @@ -395,3 +399,35 @@ export function createMockLinks(links: LLink[]): LGraph['links'] {
}
return Object.assign(map, record) as LGraph['links']
}

/**
* Reloads a serialized graph the way a fresh page would, and returns the new
* graph.
*
* Save/load assertions written as `configure(...serialize())` are vacuous by
* default. `LGraph.clear()` drops the widget store under the graph's
* *pre-configure* id, but `LGraph._configureBase` then re-adopts the *payload's*
* graph id — and `widgetValueStore` is keyed `graphId:nodeId:name`. The source
* graph's entries are therefore still live under exactly the key the reloaded
* node looks up, so `registerWidget` returns the existing state and the
* assertion passes without the serialized payload ever being read.
*
* This helper removes both escape hatches: the payload is forced through
* `JSON.parse(JSON.stringify(...))` (so live-object aliasing cannot leak) and
* the widget/preview stores are dropped for the payload's graph id before
* `configure()` runs.
*
* Verify with a mutation: a save/load test that still passes when the producer
* stops writing the field under test never read the payload.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

THE COMMENTS...

Comment thread
DrJKL marked this conversation as resolved.
Outdated
export function reloadSerializedGraph(
serialized: ISerialisedGraph | SerialisableGraph,
graphFactory: () => LGraph
): LGraph {
const payload = JSON.parse(JSON.stringify(serialized)) as typeof serialized
useWidgetValueStore().clearGraph(payload.id)
usePreviewExposureStore().clearGraph(payload.id)
const reloaded = graphFactory()
reloaded.configure(payload)
return reloaded
}
Loading