Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-04-28 18:22:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Contextmenu extension migration (#5993)

Browse Source 
This pull request refactors how context menu items are contributed by
extensions in the LiteGraph-based canvas. The legacy monkey-patching
approach for adding context menu options is replaced by a new, explicit
API (`getCanvasMenuItems` and `getNodeMenuItems`) for extensions. A
compatibility layer is added to support legacy extensions and warn
developers about deprecated usage. The changes improve maintainability,
extension interoperability, and migration to the new context menu
system.

### Context Menu System Refactor

* Introduced a new API for extensions to contribute context menu items
via `getCanvasMenuItems` and `getNodeMenuItems` methods, replacing
legacy monkey-patching of `LGraphCanvas.prototype.getCanvasMenuOptions`.
Major extension files (`groupNode.ts`, `groupOptions.ts`,
`nodeTemplates.ts`) now use this new API.
[[1]](diffhunk://#diff-b29f141b89433027e7bb7cde57fad84f9e97ffbe5c58040d3e0fdb7905022917L1779-R1771)
[[2]](diffhunk://#diff-91169f3a27ff8974d5c8fc3346bd99c07bdfb5399984484630125fdd647ff02fL232-R239)
[[3]](diffhunk://#diff-04c18583d2dbfc013888e4c02fd432c250acbcecdef82bf7f6d9fd888e632a6eL447-R458)
* Added a compatibility layer (`legacyMenuCompat` in
`contextMenuCompat.ts`) to detect and warn when legacy monkey-patching
is used, and to extract legacy-added menu items for backward
compatibility.
[[1]](diffhunk://#diff-2b724cb107c04e290369fb927e2ae9fad03be9e617a7d4de2487deab89d0d018R2-R45)
[[2]](diffhunk://#diff-d3a8284ec16ae3f9512e33abe44ae653ed1aa45c9926485ef6270cc8d2b94ae6R1-R115)

### Extension Migration

* Refactored core extensions (`groupNode`, `groupOptions`, and
`nodeTemplates`) to implement the new context menu API, moving menu item
logic out of monkey-patched methods and into explicit extension methods.
[[1]](diffhunk://#diff-b29f141b89433027e7bb7cde57fad84f9e97ffbe5c58040d3e0fdb7905022917L1633-L1683)
[[2]](diffhunk://#diff-91169f3a27ff8974d5c8fc3346bd99c07bdfb5399984484630125fdd647ff02fL19-R77)
[[3]](diffhunk://#diff-04c18583d2dbfc013888e4c02fd432c250acbcecdef82bf7f6d9fd888e632a6eL366-R373)

### Type and Import Cleanup

* Updated imports for context menu types (`IContextMenuValue`) across
affected files for consistency with the new API.
[[1]](diffhunk://#diff-b29f141b89433027e7bb7cde57fad84f9e97ffbe5c58040d3e0fdb7905022917R4-L7)
[[2]](diffhunk://#diff-91169f3a27ff8974d5c8fc3346bd99c07bdfb5399984484630125fdd647ff02fL1-R11)
[[3]](diffhunk://#diff-04c18583d2dbfc013888e4c02fd432c250acbcecdef82bf7f6d9fd888e632a6eL2-R6)
[[4]](diffhunk://#diff-bde0dce9fe2403685d27b0e94a938c3d72824d02d01d1fd6167a0dddc6e585ddR10)

### Backward Compatibility and Migration Guidance

* The compatibility layer logs a deprecation warning to the console when
legacy monkey-patching is detected, helping developers migrate to the
new API.

---

These changes collectively modernize the context menu extension
mechanism, improve code clarity, and provide a migration path for legacy
extensions.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5993-Contextmenu-extension-migration-2876d73d3650813fae07c1141679637a)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
Johnpaul Chiwetelu
2025-10-28 04:02:28 +01:00
committed by GitHub
parent 6afdb9529d
commit b3da6cf1b4
15 changed files with 1030 additions and 374 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
tests-ui/tests/extensions/contextMenuExtension.test.ts
View File

@@ -77,9 +77,9 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(ext1)
extensionStore.registerExtension(ext2)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getCanvasMenuItems', mockCanvas)
.flat() as IContextMenuValue[]
.flat()
expect(items).toHaveLength(3)
expect(items[0]).toMatchObject({ content: 'Canvas Item 1' })
@@ -105,9 +105,9 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(extension)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getCanvasMenuItems', mockCanvas)
.flat() as IContextMenuValue[]
.flat()
expect(items).toHaveLength(3)
expect(items[0].content).toBe('Menu with Submenu')
@@ -127,13 +127,44 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(canvasExtension)
extensionStore.registerExtension(extensionWithoutCanvasMenu)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getCanvasMenuItems', mockCanvas)
.flat() as IContextMenuValue[]
.flat()
expect(items).toHaveLength(1)
expect(items[0].content).toBe('Canvas Item 1')
})
it('should not duplicate menu items when collected multiple times', () => {
const extension = createCanvasMenuExtension('Test Extension', [
canvasMenuItem1,
canvasMenuItem2
])
extensionStore.registerExtension(extension)
// Collect items multiple times (simulating repeated menu opens)
const items1: IContextMenuValue[] = extensionService
.invokeExtensions('getCanvasMenuItems', mockCanvas)
.flat()
const items2: IContextMenuValue[] = extensionService
.invokeExtensions('getCanvasMenuItems', mockCanvas)
.flat()
// Both collections should have the same items (no duplication)
expect(items1).toHaveLength(2)
expect(items2).toHaveLength(2)
// Verify items are unique by checking their content
const contents1 = items1.map((item) => item.content)
const uniqueContents1 = new Set(contents1)
expect(uniqueContents1.size).toBe(contents1.length)
const contents2 = items2.map((item) => item.content)
const uniqueContents2 = new Set(contents2)
expect(uniqueContents2.size).toBe(contents2.length)
})
})
describe('collectNodeMenuItems', () => {
@@ -147,9 +178,9 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(ext1)
extensionStore.registerExtension(ext2)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getNodeMenuItems', mockNode)
.flat() as IContextMenuValue[]
.flat()
expect(items).toHaveLength(3)
expect(items[0]).toMatchObject({ content: 'Node Item 1' })
@@ -172,9 +203,9 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(extension)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getNodeMenuItems', mockNode)
.flat() as IContextMenuValue[]
.flat()
expect(items[0].content).toBe('Node Menu with Submenu')
expect(items[0].submenu?.options).toHaveLength(2)
@@ -189,9 +220,9 @@ describe('Context Menu Extension API', () => {
extensionStore.registerExtension(nodeExtension)
extensionStore.registerExtension(extensionWithoutNodeMenu)
const items = extensionService
const items: IContextMenuValue[] = extensionService
.invokeExtensions('getNodeMenuItems', mockNode)
.flat() as IContextMenuValue[]
.flat()
expect(items).toHaveLength(1)
expect(items[0].content).toBe('Node Item 1')

71
tests-ui/tests/extensions/contextMenuExtensionName.test.ts Normal file
View File

@@ -0,0 +1,71 @@
import { describe, expect, it, vi } from 'vitest'
import { legacyMenuCompat } from '@/lib/litegraph/src/contextMenuCompat'
import { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
/**
* Test that demonstrates the extension name appearing in deprecation warnings
*/
describe('Context Menu Extension Name in Warnings', () => {
it('should include extension name in deprecation warning', () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
// Install compatibility layer
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
// Simulate what happens during extension setup
legacyMenuCompat.setCurrentExtension('MyCustomExtension')
// Extension monkey-patches the method
const original = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'My Custom Menu Item', callback: () => {} })
return items
}
// Clear extension (happens after setup completes)
legacyMenuCompat.setCurrentExtension(null)
// Verify the warning includes the extension name
expect(warnSpy).toHaveBeenCalled()
const warningMessage = warnSpy.mock.calls[0][0]
expect(warningMessage).toContain('[DEPRECATED]')
expect(warningMessage).toContain('getCanvasMenuOptions')
expect(warningMessage).toContain('"MyCustomExtension"')
vi.restoreAllMocks()
})
it('should include extension name for node menu patches', () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
// Install compatibility layer
legacyMenuCompat.install(LGraphCanvas.prototype, 'getNodeMenuOptions')
// Simulate what happens during extension setup
legacyMenuCompat.setCurrentExtension('AnotherExtension')
// Extension monkey-patches the method
const original = LGraphCanvas.prototype.getNodeMenuOptions
LGraphCanvas.prototype.getNodeMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'My Node Menu Item', callback: () => {} })
return items
}
// Clear extension (happens after setup completes)
legacyMenuCompat.setCurrentExtension(null)
// Verify the warning includes extension info
expect(warnSpy).toHaveBeenCalled()
const warningMessage = warnSpy.mock.calls[0][0]
expect(warningMessage).toContain('[DEPRECATED]')
expect(warningMessage).toContain('getNodeMenuOptions')
expect(warningMessage).toContain('"AnotherExtension"')
vi.restoreAllMocks()
})
})

346
tests-ui/tests/litegraph/core/contextMenuCompat.test.ts Normal file
View File

@@ -0,0 +1,346 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { legacyMenuCompat } from '@/lib/litegraph/src/contextMenuCompat'
import { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
describe('contextMenuCompat', () => {
let originalGetCanvasMenuOptions: typeof LGraphCanvas.prototype.getCanvasMenuOptions
let mockCanvas: LGraphCanvas
beforeEach(() => {
// Save original method
originalGetCanvasMenuOptions = LGraphCanvas.prototype.getCanvasMenuOptions
// Create mock canvas
mockCanvas = {
constructor: {
prototype: LGraphCanvas.prototype
}
} as unknown as LGraphCanvas
// Clear console warnings
vi.spyOn(console, 'warn').mockImplementation(() => {})
})
afterEach(() => {
// Restore original method
LGraphCanvas.prototype.getCanvasMenuOptions = originalGetCanvasMenuOptions
vi.restoreAllMocks()
})
describe('install', () => {
it('should install compatibility layer on prototype', () => {
const methodName = 'getCanvasMenuOptions'
// Install compatibility layer
legacyMenuCompat.install(LGraphCanvas.prototype, methodName)
// The method should still be callable
expect(typeof LGraphCanvas.prototype.getCanvasMenuOptions).toBe(
'function'
)
})
it('should detect monkey patches and warn', () => {
const methodName = 'getCanvasMenuOptions'
const warnSpy = vi.spyOn(console, 'warn')
// Install compatibility layer
legacyMenuCompat.install(LGraphCanvas.prototype, methodName)
// Set current extension before monkey-patching
legacyMenuCompat.setCurrentExtension('Test Extension')
// Simulate extension monkey-patching
const original = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'Custom Item', callback: () => {} })
return items
}
// Should have logged a warning with extension name
expect(warnSpy).toHaveBeenCalledWith(
expect.stringContaining('[DEPRECATED]'),
expect.any(String),
expect.any(String)
)
expect(warnSpy).toHaveBeenCalledWith(
expect.stringContaining('"Test Extension"'),
expect.any(String),
expect.any(String)
)
// Clear extension
legacyMenuCompat.setCurrentExtension(null)
})
it('should only warn once per unique function', () => {
const methodName = 'getCanvasMenuOptions'
const warnSpy = vi.spyOn(console, 'warn')
legacyMenuCompat.install(LGraphCanvas.prototype, methodName)
legacyMenuCompat.setCurrentExtension('test.extension')
const patchFunction = function (this: LGraphCanvas, ...args: any[]) {
const items = (originalGetCanvasMenuOptions as any).apply(this, args)
items.push({ content: 'Custom', callback: () => {} })
return items
}
// Patch twice with same function
LGraphCanvas.prototype.getCanvasMenuOptions = patchFunction
LGraphCanvas.prototype.getCanvasMenuOptions = patchFunction
// Should only warn once
expect(warnSpy).toHaveBeenCalledTimes(1)
})
})
describe('extractLegacyItems', () => {
beforeEach(() => {
// Setup a mock original method
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [
{ content: 'Item 1', callback: () => {} },
{ content: 'Item 2', callback: () => {} }
]
}
// Install compatibility layer
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
})
it('should extract items added by monkey patches', () => {
// Monkey-patch to add items
const original = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'Custom Item 1', callback: () => {} })
items.push({ content: 'Custom Item 2', callback: () => {} })
return items
}
// Extract legacy items
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(2)
expect(legacyItems[0]).toMatchObject({ content: 'Custom Item 1' })
expect(legacyItems[1]).toMatchObject({ content: 'Custom Item 2' })
})
it('should return empty array when no items added', () => {
// No monkey-patching, so no extra items
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(0)
})
it('should return empty array when patched method returns same count', () => {
// Monkey-patch that replaces items but keeps same count
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [
{ content: 'Replaced 1', callback: () => {} },
{ content: 'Replaced 2', callback: () => {} }
]
}
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(0)
})
it('should handle errors gracefully', () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
// Monkey-patch that throws error
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
throw new Error('Test error')
}
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(0)
expect(errorSpy).toHaveBeenCalledWith(
expect.stringContaining('Failed to extract legacy items'),
expect.any(Error)
)
})
})
describe('integration', () => {
it('should work with multiple extensions patching', () => {
// Setup base method
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [{ content: 'Base Item', callback: () => {} }]
}
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
// First extension patches
const original1 = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original1 as any).apply(this, args)
items.push({ content: 'Extension 1 Item', callback: () => {} })
return items
}
// Second extension patches
const original2 = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original2 as any).apply(this, args)
items.push({ content: 'Extension 2 Item', callback: () => {} })
return items
}
// Extract legacy items
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
// Should extract both items added by extensions
expect(legacyItems).toHaveLength(2)
expect(legacyItems[0]).toMatchObject({ content: 'Extension 1 Item' })
expect(legacyItems[1]).toMatchObject({ content: 'Extension 2 Item' })
})
it('should extract legacy items only once even when called multiple times', () => {
// Setup base method
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [
{ content: 'Base Item 1', callback: () => {} },
{ content: 'Base Item 2', callback: () => {} }
]
}
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
// Simulate legacy extension monkey-patching the prototype
const original = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'Legacy Item 1', callback: () => {} })
items.push({ content: 'Legacy Item 2', callback: () => {} })
return items
}
// Extract legacy items multiple times (simulating repeated menu opens)
const legacyItems1 = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
const legacyItems2 = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
const legacyItems3 = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
// Each extraction should return the same items (no accumulation)
expect(legacyItems1).toHaveLength(2)
expect(legacyItems2).toHaveLength(2)
expect(legacyItems3).toHaveLength(2)
// Verify items are the expected ones
expect(legacyItems1[0]).toMatchObject({ content: 'Legacy Item 1' })
expect(legacyItems1[1]).toMatchObject({ content: 'Legacy Item 2' })
expect(legacyItems2[0]).toMatchObject({ content: 'Legacy Item 1' })
expect(legacyItems2[1]).toMatchObject({ content: 'Legacy Item 2' })
expect(legacyItems3[0]).toMatchObject({ content: 'Legacy Item 1' })
expect(legacyItems3[1]).toMatchObject({ content: 'Legacy Item 2' })
})
it('should not extract items from registered wrapper methods', () => {
// Setup base method
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [{ content: 'Base Item', callback: () => {} }]
}
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
// Create a wrapper that adds new API items (simulating useContextMenuTranslation)
const originalMethod = LGraphCanvas.prototype.getCanvasMenuOptions
const wrapperMethod = function (this: LGraphCanvas) {
const items = (originalMethod as any).apply(this, [])
// Add new API items
items.push({ content: 'New API Item 1', callback: () => {} })
items.push({ content: 'New API Item 2', callback: () => {} })
return items
}
// Set the wrapper as the current method
LGraphCanvas.prototype.getCanvasMenuOptions = wrapperMethod
// Register the wrapper so it's not treated as a legacy patch
legacyMenuCompat.registerWrapper(
'getCanvasMenuOptions',
wrapperMethod,
originalMethod,
LGraphCanvas.prototype // Wrapper is installed
)
// Extract legacy items - should return empty because current method is a registered wrapper
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(0)
})
it('should extract legacy items even when a wrapper is registered but not active', () => {
// Setup base method
LGraphCanvas.prototype.getCanvasMenuOptions = function () {
return [{ content: 'Base Item', callback: () => {} }]
}
legacyMenuCompat.install(LGraphCanvas.prototype, 'getCanvasMenuOptions')
// Register a wrapper (but don't set it as the current method)
const originalMethod = LGraphCanvas.prototype.getCanvasMenuOptions
const wrapperMethod = function () {
return [{ content: 'Wrapper Item', callback: () => {} }]
}
legacyMenuCompat.registerWrapper(
'getCanvasMenuOptions',
wrapperMethod,
originalMethod
// NOT passing prototype, so it won't be marked as installed
)
// Monkey-patch with a different function (legacy extension)
const original = LGraphCanvas.prototype.getCanvasMenuOptions
LGraphCanvas.prototype.getCanvasMenuOptions = function (...args: any[]) {
const items = (original as any).apply(this, args)
items.push({ content: 'Legacy Item', callback: () => {} })
return items
}
// Extract legacy items - should return the legacy item because current method is NOT the wrapper
const legacyItems = legacyMenuCompat.extractLegacyItems(
'getCanvasMenuOptions',
mockCanvas
)
expect(legacyItems).toHaveLength(1)
expect(legacyItems[0]).toMatchObject({ content: 'Legacy Item' })
})
})
})