Skip to content
9 changes: 4 additions & 5 deletions src/extensions/core/customWidgets.clone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'
import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
import type { ComfyNodeDef } from '@/schemas/nodeDefSchema'
import { app } from '@/scripts/app'
import { getRootGraph, setRootGraph } from '@/scripts/__tests__/appTestUtils'
import { useExtensionStore } from '@/stores/extensionStore'
import type { ComfyExtension } from '@/types/comfy'

Expand Down Expand Up @@ -65,10 +66,8 @@ describe('CustomCombo copy/paste', () => {

it('preserves combo options and selected value through clone and paste', () => {
const graph = new LGraph()
type AppWithRootGraph = { rootGraphInternal?: LGraph }
const appWithRootGraph = app as unknown as AppWithRootGraph
const previousRootGraph = appWithRootGraph.rootGraphInternal
appWithRootGraph.rootGraphInternal = graph
const previousRootGraph = getRootGraph(app)
setRootGraph(app, graph)

try {
const original = LiteGraph.createNode(TEST_CUSTOM_COMBO_TYPE)!
Expand Down Expand Up @@ -97,7 +96,7 @@ describe('CustomCombo copy/paste', () => {
'gamma'
])
} finally {
appWithRootGraph.rootGraphInternal = previousRootGraph
setRootGraph(app, previousRootGraph)
}
})
})
17 changes: 7 additions & 10 deletions src/extensions/core/customWidgets.subgraphPromotion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
import type { ComfyNodeDef } from '@/schemas/nodeDefSchema'
import { app } from '@/scripts/app'
import { getRootGraph, setRootGraph } from '@/scripts/__tests__/appTestUtils'
import { useExtensionStore } from '@/stores/extensionStore'
import { useWidgetValueStore } from '@/stores/widgetValueStore'
import type { ComfyExtension } from '@/types/comfy'
Expand Down Expand Up @@ -122,10 +123,8 @@ describe('CustomCombo index widget after subgraph promotion', () => {

it('resolves INDEX from the promoted host choice, not the frozen interior value', async () => {
const rootGraph = new LGraph()
type AppWithRootGraph = { rootGraphInternal?: LGraph }
const appWithRootGraph = app as unknown as AppWithRootGraph
const previousRootGraph = appWithRootGraph.rootGraphInternal
appWithRootGraph.rootGraphInternal = rootGraph
const previousRootGraph = getRootGraph(app)
setRootGraph(app, rootGraph)

try {
const subgraph = createTestSubgraph({ rootGraph })
Expand Down Expand Up @@ -167,16 +166,14 @@ describe('CustomCombo index widget after subgraph promotion', () => {
// "four" is index 3 of ["one", "two", "three", "four"].
expect(promptInputs.index).toBe(3)
} finally {
appWithRootGraph.rootGraphInternal = previousRootGraph
setRootGraph(app, previousRootGraph)
}
})

it('resolves INDEX from the interior widget when choice was never promoted', async () => {
const rootGraph = new LGraph()
type AppWithRootGraph = { rootGraphInternal?: LGraph }
const appWithRootGraph = app as unknown as AppWithRootGraph
const previousRootGraph = appWithRootGraph.rootGraphInternal
appWithRootGraph.rootGraphInternal = rootGraph
const previousRootGraph = getRootGraph(app)
setRootGraph(app, rootGraph)

try {
const comboNode = LiteGraph.createNode(
Expand All @@ -195,7 +192,7 @@ describe('CustomCombo index widget after subgraph promotion', () => {
// "two" is index 1 of ["one", "two", "three"].
expect(promptInputs.index).toBe(1)
} finally {
appWithRootGraph.rootGraphInternal = previousRootGraph
setRootGraph(app, previousRootGraph)
}
})
})
15 changes: 15 additions & 0 deletions src/scripts/__tests__/appTestUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { LGraph } from '@/lib/litegraph/src/litegraph'
import type { ComfyApp } from '@/scripts/app'

/**
* `ComfyApp.setup` is the only production writer of the root graph, and it needs
* a real canvas. Tests that just need a graph in place reach the same storage
* through this seam instead.
*/
export function setRootGraph(app: ComfyApp, graph: LGraph | undefined) {
app['rootGraphRef'].value = graph
}

export function getRootGraph(app: ComfyApp) {
return app['rootGraphRef'].value
}
50 changes: 26 additions & 24 deletions src/scripts/app.test.ts
Comment thread
mattmillerai marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { fromPartial } from '@total-typescript/shoehorn'
import { beforeEach, describe, expect, it, vi } from 'vitest'

import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
Expand All @@ -16,6 +17,7 @@ import { createMockChangeTracker } from '@/utils/__tests__/litegraphTestUtils'
import { useNodeReplacementStore } from '@/platform/nodeReplacement/nodeReplacementStore'
import type { NodeReplacement } from '@/platform/nodeReplacement/types'
import { ComfyApp, app as singletonApp } from './app'
import { setRootGraph } from './__tests__/appTestUtils'
import { createNode } from '@/utils/litegraphUtil'
import {
pasteAudioNode,
Expand Down Expand Up @@ -298,8 +300,8 @@ describe('ComfyApp', () => {
size: 0
})
const graph = new LGraph()
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
mockWorkspaceWorkflow.activeWorkflow = workflow
vi.spyOn(app, 'graphToPrompt').mockResolvedValue({
output: {},
Expand Down Expand Up @@ -336,8 +338,8 @@ describe('ComfyApp', () => {
]
}
}
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
mockWorkspaceWorkflow.activeWorkflow = workflow
vi.spyOn(app, 'graphToPrompt').mockResolvedValue({
output: promptOutput,
Expand Down Expand Up @@ -788,8 +790,8 @@ describe('ComfyApp', () => {
it('clears missing node packs before loading API JSON without missing nodes', async () => {
const graph = new LGraph()
const activeSubgraph = createTestSubgraph({ rootGraph: graph })
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
Reflect.set(mockCanvas, 'graph', activeSubgraph)
Reflect.set(mockCanvas, 'subgraph', activeSubgraph)
vi.mocked(mockCanvas.setGraph).mockImplementation((nextGraph) => {
Expand Down Expand Up @@ -825,8 +827,8 @@ describe('ComfyApp', () => {

it('creates a removable placeholder for an API JSON missing node', async () => {
const graph = new LGraph()
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
const cleanupErrorHooks = installErrorClearingHooks(graph)
const missingNodesStore = useMissingNodesErrorStore()
const missingNodeType = 'Uninstalled<&Node>'
Expand Down Expand Up @@ -891,8 +893,8 @@ describe('ComfyApp', () => {

it('preserves API JSON inputs on a missing node across reload', async () => {
const graph = new LGraph()
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
const sourceNodeType = 'test/ApiJsonSourceNode'
const missingNodeType = 'UninstalledInputNode'
class ApiJsonSourceNode extends LGraphNode {
Expand Down Expand Up @@ -996,8 +998,8 @@ describe('ComfyApp', () => {

it('defers API JSON missing node warnings until they are flushed', async () => {
const graph = new LGraph()
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
const nodeReplacementStore = useNodeReplacementStore()
vi.spyOn(nodeReplacementStore, 'load').mockResolvedValue()
vi.spyOn(nodeReplacementStore, 'getReplacementFor').mockReturnValue(null)
Expand Down Expand Up @@ -1125,7 +1127,7 @@ describe('ComfyApp', () => {
experimental: false
}
}
Reflect.set(app, 'rootGraphInternal', rootGraph)
setRootGraph(app, rootGraph)
vi.spyOn(app, 'getNodeDefs').mockResolvedValue(defs)
vi.spyOn(app, 'registerNodeDef').mockResolvedValue(undefined)

Expand All @@ -1144,15 +1146,15 @@ describe('ComfyApp', () => {

describe('refreshMissingModels', () => {
it('delegates to the app-independent missing model refresh pipeline', async () => {
const graph = {
const graph = fromPartial<LGraph>({
nodes: [],
serialize: vi.fn(() => createWorkflowGraphData())
}
})
const result = {
missingModels: [],
confirmedCandidates: []
}
Reflect.set(app, 'rootGraphInternal', graph)
setRootGraph(app, graph)
vi.spyOn(app, 'reloadNodeDefs').mockResolvedValue()
mockRefreshMissingModelPipeline.mockResolvedValue(result)

Expand All @@ -1172,11 +1174,11 @@ describe('ComfyApp', () => {
})

it('omits the node definition reload when reloadDefs is false', async () => {
const graph = {
const graph = fromPartial<LGraph>({
nodes: [],
serialize: vi.fn(() => createWorkflowGraphData())
}
Reflect.set(app, 'rootGraphInternal', graph)
})
setRootGraph(app, graph)
vi.spyOn(app, 'reloadNodeDefs').mockResolvedValue()
mockRefreshMissingModelPipeline.mockResolvedValue({
missingModels: [],
Expand Down Expand Up @@ -1561,7 +1563,7 @@ describe('ComfyApp', () => {
it('preserves the current graph when A1111 core nodes are unavailable', async () => {
const graph = new LGraph()
const parameters = 'positive\nNegative prompt: negative\nSteps: 20'
Reflect.set(app, 'rootGraphInternal', graph)
setRootGraph(app, graph)
vi.mocked(getWorkflowDataFromFile).mockResolvedValue({ parameters })
mockImportA1111.mockResolvedValue('core-nodes-unavailable')

Expand All @@ -1584,7 +1586,7 @@ describe('ComfyApp', () => {
it('shows one file-load error when parameters are not A1111-shaped', async () => {
const graph = new LGraph()
const parameters = 'positive\nSteps: 20'
Reflect.set(app, 'rootGraphInternal', graph)
setRootGraph(app, graph)
vi.mocked(getWorkflowDataFromFile).mockResolvedValue({ parameters })
mockImportA1111.mockResolvedValue('not-a1111')

Expand All @@ -1601,7 +1603,7 @@ describe('ComfyApp', () => {
it('awaits persistence and orders its clear callback before setGraph', async () => {
const graph = new LGraph()
const parameters = 'positive\nNegative prompt: negative\nSteps: 20'
Reflect.set(app, 'rootGraphInternal', graph)
setRootGraph(app, graph)
vi.mocked(getWorkflowDataFromFile).mockResolvedValue({ parameters })
mockImportA1111.mockImplementation(
async (_graph, _parameters, beforeGraphClear) => {
Expand Down Expand Up @@ -1652,8 +1654,8 @@ describe('ComfyApp', () => {
} as unknown as LGraphCanvas

const graph = new LGraph()
Reflect.set(app, 'rootGraphInternal', graph)
Reflect.set(singletonApp, 'rootGraphInternal', graph)
setRootGraph(app, graph)
setRootGraph(singletonApp, graph)
const outgoingWorkflow = new ComfyWorkflow({
path: 'workflows/outgoing.json',
modified: 0,
Expand Down
16 changes: 8 additions & 8 deletions src/scripts/app.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { useEventListener, useResizeObserver } from '@vueuse/core'
import _ from 'es-toolkit/compat'
import type { ToastMessageOptions } from 'primevue/toast'
import { reactive, unref } from 'vue'
import { shallowRef } from 'vue'
import { reactive, shallowRef, unref } from 'vue'

import { useCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
Expand Down Expand Up @@ -281,24 +280,25 @@
private _nodeOutputs!: Record<string, NodeExecutionOutput>
nodePreviewImages: Record<string, string[]>

private rootGraphInternal: LGraph | undefined
/** Shallow: the graph is observed for readiness, never deep-proxied. */
private readonly rootGraphRef = shallowRef<LGraph | undefined>(undefined)

// TODO: Migrate internal usage to the
/** @deprecated Use {@link rootGraph} instead */
get graph() {
return this.rootGraphInternal!
Comment thread
mattmillerai marked this conversation as resolved.
return this.rootGraphRef.value!
}

get rootGraph(): LGraph {
if (!this.rootGraphInternal) {
if (!this.rootGraphRef.value) {
console.error('ComfyApp graph accessed before initialization')
}
return this.rootGraphInternal!
return this.rootGraphRef.value!
}

/** Whether the root graph has been initialized. Safe to check without triggering error logs. */
get isGraphReady(): boolean {
return !!this.rootGraphInternal
return !!this.rootGraphRef.value
}

canvas!: LGraphCanvas
Expand Down Expand Up @@ -957,7 +957,7 @@

this.addAfterConfigureHandler(graph)

this.rootGraphInternal = graph
this.rootGraphRef.value = graph
installNodeAddedTelemetry(graph)
this.canvas = new LGraphCanvas(canvasEl, graph)
// Make canvas states reactive so we can observe changes on them.
Expand Down Expand Up @@ -2037,7 +2037,7 @@
return
}

this.showErrorOnFileLoad(file)

Check failure on line 2040 in src/scripts/app.ts

View workflow job for this annotation

GitHub Actions / test

src/scripts/app.test.ts > ComfyApp > A1111 import > clears missing node packs, which its graph swap skips clean() for

TypeError: Cannot read properties of undefined (reading 'serialize') ❯ ComfyApp.handleFile src/scripts/app.ts:2040:24 ❯ src/scripts/app.test.ts:1081:7

Check failure on line 2040 in src/scripts/app.ts

View workflow job for this annotation

GitHub Actions / test

src/scripts/app.test.ts > ComfyApp > A1111 import > clears missing node packs, which its graph swap skips clean() for

TypeError: Cannot read properties of undefined (reading 'serialize') ❯ ComfyApp.handleFile src/scripts/app.ts:2040:24 ❯ src/scripts/app.test.ts:1081:7

Check failure on line 2040 in src/scripts/app.ts

View workflow job for this annotation

GitHub Actions / test

src/scripts/app.test.ts > ComfyApp > A1111 import > clears missing node packs, which its graph swap skips clean() for

TypeError: Cannot read properties of undefined (reading 'serialize') ❯ ComfyApp.handleFile src/scripts/app.ts:2040:24 ❯ src/scripts/app.test.ts:1081:7
}

/**
Expand Down
72 changes: 72 additions & 0 deletions src/scripts/rootGraphReadiness.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { useEventListener } from '@vueuse/core'
import { effectScope, nextTick, watchEffect } from 'vue'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import { LGraph } from '@/lib/litegraph/src/litegraph'
import { getRootGraph, setRootGraph } from '@/scripts/__tests__/appTestUtils'
import { app } from '@/scripts/app'

describe('ComfyApp root graph readiness', () => {
let scope: ReturnType<typeof effectScope>
let previousRootGraph: LGraph | undefined

beforeEach(() => {
vi.spyOn(console, 'error').mockImplementation(() => {})
previousRootGraph = getRootGraph(app)
setRootGraph(app, undefined)
scope = effectScope()
})

afterEach(() => {
scope.stop()
setRootGraph(app, previousRootGraph)
vi.restoreAllMocks()
})

it('re-runs an effect reading isGraphReady when the graph is assigned', async () => {
const readiness: boolean[] = []
scope.run(() => {
watchEffect(() => readiness.push(app.isGraphReady))
})

expect(readiness).toEqual([false])

setRootGraph(app, new LGraph())
await nextTick()

expect(readiness).toEqual([false, true])
})

it('binds a rootGraph.events listener registered before the graph exists', async () => {
const onConfigured = vi.fn()
scope.run(() => {
useEventListener(() => app.rootGraph?.events, 'configured', onConfigured)
})

const graph = new LGraph()
setRootGraph(app, graph)
await nextTick()
graph.events.dispatch('configured')

expect(onConfigured).toHaveBeenCalledOnce()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
})

it('rebinds a rootGraph.events listener when the graph is replaced', async () => {
const onConfigured = vi.fn()
const firstGraph = new LGraph()
setRootGraph(app, firstGraph)
scope.run(() => {
useEventListener(() => app.rootGraph?.events, 'configured', onConfigured)
})

const secondGraph = new LGraph()
setRootGraph(app, secondGraph)
await nextTick()

firstGraph.events.dispatch('configured')
expect(onConfigured).not.toHaveBeenCalled()

secondGraph.events.dispatch('configured')
expect(onConfigured).toHaveBeenCalledOnce()
})
})
Loading