From 84fc0e9205a6bee7df27b513a3b6a174a61e9d08 Mon Sep 17 00:00:00 2001 From: bymyself Date: Tue, 24 Sep 2024 23:25:08 -0700 Subject: [PATCH] Fix group node naming compatibility (#969) * Convery legacy group node names in workflow * Add Playwright test * Remove hardcoded strings --- browser_tests/assets/legacy_group_node.json | 252 ++++++++++++++++++++ browser_tests/groupNode.spec.ts | 8 + src/extensions/core/groupNode.ts | 38 ++- src/extensions/core/groupNodeManage.ts | 20 +- 4 files changed, 302 insertions(+), 16 deletions(-) create mode 100644 browser_tests/assets/legacy_group_node.json diff --git a/browser_tests/assets/legacy_group_node.json b/browser_tests/assets/legacy_group_node.json new file mode 100644 index 0000000000..e64f2f785e --- /dev/null +++ b/browser_tests/assets/legacy_group_node.json @@ -0,0 +1,252 @@ +{ + "last_node_id": 15, + "last_link_id": 9, + "nodes": [ + { + "id": 15, + "type": "workflow/hello", + "pos": { + "0": 566, + "1": 316 + }, + "size": { + "0": 468.5999755859375, + "1": 582 + }, + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "name": "model", + "type": "MODEL", + "link": null, + "label": "model" + }, + { + "name": "positive", + "type": "CONDITIONING", + "link": null, + "label": "positive" + }, + { + "name": "negative", + "type": "CONDITIONING", + "link": null, + "label": "negative" + }, + { + "name": "latent_image", + "type": "LATENT", + "link": null, + "label": "latent_image" + }, + { + "name": "KSampler model", + "type": "MODEL", + "link": null, + "label": "KSampler model" + }, + { + "name": "KSampler positive", + "type": "CONDITIONING", + "link": null, + "label": "KSampler positive" + }, + { + "name": "KSampler negative", + "type": "CONDITIONING", + "link": null, + "label": "KSampler negative" + }, + { + "name": "KSampler latent_image", + "type": "LATENT", + "link": null, + "label": "KSampler latent_image" + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": null, + "shape": 3, + "label": "LATENT" + }, + { + "name": "KSampler LATENT", + "type": "LATENT", + "links": null, + "shape": 3, + "label": "KSampler LATENT" + } + ], + "properties": { + "Node name for S&R": "workflow/hello" + }, + "widgets_values": [ + "enable", + 0, + "randomize", + 20, + 8, + "euler", + "normal", + 0, + 10000, + "disable", + 0, + "randomize", + 20, + 8, + "euler", + "normal", + 1 + ] + } + ], + "links": [], + "groups": [], + "config": {}, + "extra": { + "groupNodes": { + "hello": { + "nodes": [ + { + "id": -1, + "type": "KSamplerAdvanced", + "pos": { + "0": 351.3332824707031, + "1": 577.3333129882812 + }, + "size": { + "0": 315, + "1": 334 + }, + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { + "name": "model", + "type": "MODEL", + "link": null, + "label": "model" + }, + { + "name": "positive", + "type": "CONDITIONING", + "link": null, + "label": "positive" + }, + { + "name": "negative", + "type": "CONDITIONING", + "link": null, + "label": "negative" + }, + { + "name": "latent_image", + "type": "LATENT", + "link": null, + "label": "latent_image" + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": null, + "shape": 3, + "label": "LATENT" + } + ], + "properties": { + "Node name for S&R": "KSamplerAdvanced" + }, + "widgets_values": [ + "enable", + 0, + "randomize", + 20, + 8, + "euler", + "normal", + 0, + 10000, + "disable" + ], + "index": 0 + }, + { + "id": -1, + "type": "KSampler", + "pos": { + "0": 636, + "1": 427 + }, + "size": { + "0": 315, + "1": 262 + }, + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "name": "model", + "type": "MODEL", + "link": null, + "label": "model" + }, + { + "name": "positive", + "type": "CONDITIONING", + "link": null, + "label": "positive" + }, + { + "name": "negative", + "type": "CONDITIONING", + "link": null, + "label": "negative" + }, + { + "name": "latent_image", + "type": "LATENT", + "link": null, + "label": "latent_image" + } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": null, + "shape": 3, + "label": "LATENT" + } + ], + "properties": { + "Node name for S&R": "KSampler" + }, + "widgets_values": [ + 0, + "randomize", + 20, + 8, + "euler", + "normal", + 1 + ], + "index": 1 + } + ], + "links": [], + "external": [] + } + } + }, + "version": 0.4 +} \ No newline at end of file diff --git a/browser_tests/groupNode.spec.ts b/browser_tests/groupNode.spec.ts index 4a726da275..cf8cb69896 100644 --- a/browser_tests/groupNode.spec.ts +++ b/browser_tests/groupNode.spec.ts @@ -140,4 +140,12 @@ test.describe('Group Node', () => { // Ensure the link is still present expect(await input.getLinkCount()).toBe(1) }) + + test('Loads from a workflow using the legacy path separator ("/")', async ({ + comfyPage + }) => { + await comfyPage.loadWorkflow('legacy_group_node') + expect(await comfyPage.getGraphNodesCount()).toBe(1) + expect(comfyPage.page.locator('.comfy-missing-nodes')).not.toBeVisible() + }) }) diff --git a/src/extensions/core/groupNode.ts b/src/extensions/core/groupNode.ts index 31d662aba7..694081e982 100644 --- a/src/extensions/core/groupNode.ts +++ b/src/extensions/core/groupNode.ts @@ -5,9 +5,15 @@ import { ManageGroupDialog } from './groupNodeManage' import type { LGraphNode } from '@comfyorg/litegraph' import { LGraphCanvas, LiteGraph } from '@comfyorg/litegraph' import { useNodeDefStore } from '@/stores/nodeDefStore' +import { ComfyNode, ComfyWorkflowJSON } from '@/types/comfyWorkflow' const GROUP = Symbol() +// v1 Prefix + Separator: workflow/ +// v2 Prefix + Separator: workflow> (ComfyUI_frontend v1.2.63) +const PREFIX = 'workflow' +const SEPARATOR = '>' + const Workflow = { InUse: { Free: 0, @@ -15,7 +21,7 @@ const Workflow = { InWorkflow: 2 }, isInUseGroupNode(name) { - const id = `workflow>${name}` + const id = `${PREFIX}${SEPARATOR}${name}` // Check if lready registered/in use in this workflow if (app.graph.extra?.groupNodes?.[name]) { if (app.graph.nodes.find((n) => n.type === id)) { @@ -185,15 +191,15 @@ export class GroupNodeConfig { this.outputVisibility = [] } - async registerType(source = 'workflow') { + async registerType(source = PREFIX) { this.nodeDef = { output: [], output_name: [], output_is_list: [], output_is_hidden: [], - name: source + '>' + this.name, + name: source + SEPARATOR + this.name, display_name: this.name, - category: 'group nodes' + ('>' + source), + category: 'group nodes' + (SEPARATOR + source), input: { required: {} }, description: `Group node combining ${this.nodeData.nodes .map((n) => n.type) @@ -216,7 +222,7 @@ export class GroupNodeConfig { p() } this.#convertedToProcess = null - await app.registerNodeDef('workflow>' + this.name, this.nodeDef) + await app.registerNodeDef(`${PREFIX}${SEPARATOR}` + this.name, this.nodeDef) useNodeDefStore().addNodeDef(this.nodeDef) } @@ -647,7 +653,7 @@ export class GroupNodeConfig { app.clean = function () { for (const g in groupNodes) { try { - LiteGraph.unregisterNodeType('workflow/' + g) + LiteGraph.unregisterNodeType(`${PREFIX}${SEPARATOR}` + g) } catch (error) {} } app.clean = clean @@ -662,11 +668,11 @@ export class GroupNodeConfig { if (!(n.type in LiteGraph.registered_node_types)) { missingNodeTypes.push({ type: n.type, - hint: ` (In group node 'workflow/${g}')` + hint: ` (In group node '${PREFIX}${SEPARATOR}${g}')` }) missingNodeTypes.push({ - type: 'workflow/' + g, + type: `${PREFIX}${SEPARATOR}` + g, action: { text: 'Remove from workflow', callback: (e) => { @@ -1380,7 +1386,7 @@ export class GroupNodeHandler { const config = new GroupNodeConfig(name, nodeData) await config.registerType() - const groupNode = LiteGraph.createNode(`workflow>${name}`) + const groupNode = LiteGraph.createNode(`${PREFIX}${SEPARATOR}${name}`) // Reuse the existing nodes for this instance groupNode.setInnerNodes(builder.nodes) groupNode[GROUP].populateWidgets() @@ -1444,6 +1450,14 @@ function addConvertToGroupOptions() { } } +const replaceLegacySeparators = (nodes: ComfyNode[]): void => { + for (const node of nodes) { + if (typeof node.type === 'string' && node.type.startsWith('workflow/')) { + node.type = node.type.replace(/^workflow\//, `${PREFIX}${SEPARATOR}`) + } + } +} + const id = 'Comfy.GroupNode' let globalDefs const ext = { @@ -1451,9 +1465,13 @@ const ext = { setup() { addConvertToGroupOptions() }, - async beforeConfigureGraph(graphData, missingNodeTypes) { + async beforeConfigureGraph( + graphData: ComfyWorkflowJSON, + missingNodeTypes: string[] + ) { const nodes = graphData?.extra?.groupNodes if (nodes) { + replaceLegacySeparators(graphData.nodes) await GroupNodeConfig.registerFromWorkflow(nodes, missingNodeTypes) } }, diff --git a/src/extensions/core/groupNodeManage.ts b/src/extensions/core/groupNodeManage.ts index cf68be0be3..7d6b546028 100644 --- a/src/extensions/core/groupNodeManage.ts +++ b/src/extensions/core/groupNodeManage.ts @@ -10,6 +10,8 @@ import { } from '@comfyorg/litegraph' const ORDER: symbol = Symbol() +const PREFIX = 'workflow' +const SEPARATOR = '>' function merge(target, source) { if (typeof target === 'object' && typeof source === 'object') { @@ -102,7 +104,9 @@ export class ManageGroupDialog extends ComfyDialog { getGroupData() { this.groupNodeType = - LiteGraph.registered_node_types['workflow>' + this.selectedGroup] + LiteGraph.registered_node_types[ + `${PREFIX}${SEPARATOR}` + this.selectedGroup + ] this.groupNodeDef = this.groupNodeType.nodeData this.groupData = GroupNodeHandler.getGroupData(this.groupNodeType) } @@ -367,7 +371,7 @@ export class ManageGroupDialog extends ComfyDialog { groupNodes.map((g) => $el('option', { textContent: g, - selected: 'workflow>' + g === type, + selected: `${PREFIX}${SEPARATOR}` + g === type, value: g }) ) @@ -389,7 +393,7 @@ export class ManageGroupDialog extends ComfyDialog { { onclick: (e) => { const node = app.graph.nodes.find( - (n) => n.type === 'workflow>' + this.selectedGroup + (n) => n.type === `${PREFIX}${SEPARATOR}` + this.selectedGroup ) if (node) { alert( @@ -403,7 +407,9 @@ export class ManageGroupDialog extends ComfyDialog { ) ) { delete app.graph.extra.groupNodes[this.selectedGroup] - LiteGraph.unregisterNodeType('workflow>' + this.selectedGroup) + LiteGraph.unregisterNodeType( + `${PREFIX}${SEPARATOR}` + this.selectedGroup + ) } this.show() } @@ -476,7 +482,7 @@ export class ManageGroupDialog extends ComfyDialog { }, {}) } - const nodes = nodesByType['workflow>' + g] + const nodes = nodesByType[`${PREFIX}${SEPARATOR}` + g] if (nodes) recreateNodes.push(...nodes) } @@ -503,7 +509,9 @@ export class ManageGroupDialog extends ComfyDialog { this.element.replaceChildren(outer) this.changeGroup( - type ? groupNodes.find((g) => 'workflow>' + g === type) : groupNodes[0] + type + ? groupNodes.find((g) => `${PREFIX}${SEPARATOR}` + g === type) + : groupNodes[0] ) this.element.showModal()