Skip to content

Conversation

@Myestery
Copy link
Collaborator

@Myestery Myestery commented Oct 9, 2025

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] [2] [3]
  • 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] [2]

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] [2] [3]

Type and Import Cleanup

  • Updated imports for context menu types (IContextMenuValue) across affected files for consistency with the new API. [1] [2] [3] [4]

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 by Unito

Introduces a new extension API that allows extensions to provide context menu items directly, without monkey-patching. This provides a clean, type-safe way for extensions to add menu items.

**New API methods:**
- `getCanvasMenuItems(canvas)`: Add items to canvas right-click menus
- `getNodeMenuItems(node)`: Add items to node right-click menus

**Implementation:**
- Added TypeScript interfaces in `src/types/comfy.ts`
- Added collection methods in `ComfyApp` class
- Comprehensive test coverage for the new API
…extensions

Adds a compatibility layer that detects and supports legacy extensions using the monkey-patching pattern, while warning developers about the deprecated approach.

**Features:**
- Automatic detection of monkey-patched context menu methods
- Console warnings with extension name for deprecated patterns
- Extraction and integration of legacy menu items
- Extension tracking during setup for accurate warnings

**Files:**
- `src/lib/litegraph/src/contextMenuCompat.ts`: Core compatibility logic
- `src/services/extensionService.ts`: Extension name tracking
- `src/composables/useContextMenuTranslation.ts`: Integration layer
- Comprehensive test coverage

Depends on PR #5977 (context menu extension API)
Migrates core extensions from monkey-patching to the new context menu extension API, demonstrating the migration path for extension developers.

**Migrated extensions:**
- `groupOptions.ts`: Group management context menu items
- `nodeTemplates.ts`: Template save/load context menu items

**Legacy compatibility test:**
- `groupNode.ts`: Kept with monkey-patching as compatibility test case
- Includes documentation explaining the old vs new approach

**Benefits:**
- Type-safe menu item registration
- No prototype pollution
- Easier to test and maintain
- Clear separation of concerns

Depends on PR #5977 (context menu extension API)
Completes the migration by converting the groupNode extension from monkey-patching to the new context menu API.

**Changes:**
- Removed `addConvertToGroupOptions()` function and monkey-patching
- Implemented `getCanvasMenuItems()` method
- Implemented `getNodeMenuItems()` method
- Removed `setup()` hook (no longer needed)

All core extensions now use the standardized API with no monkey-patching.
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

🎨 Storybook Build Status

Build completed successfully!

⏰ Completed at: 10/24/2025, 07:10:49 AM UTC

🔗 Links


🎉 Your Storybook is ready for review!

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

🎭 Playwright Test Results

Some tests failed

⏰ Completed at: 10/24/2025, 07:27:12 AM UTC

📈 Summary

  • Total Tests: 496
  • Passed: 460 ✅
  • Failed: 4 ❌
  • Flaky: 1 ⚠️
  • Skipped: 31 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 451 / ❌ 4 / ⚠️ 1 / ⏭️ 31
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 6 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

christian-byrne pushed a commit that referenced this pull request Oct 10, 2025
This pull request introduces a new extension API for context menu
customization, allowing extensions to contribute items to both canvas
and node right-click menus. It adds two collection methods to the
`ComfyApp` class to aggregate these menu items from all registered
extensions, and updates the extension interface accordingly.
Comprehensive unit tests are included to verify the correct aggregation
behavior and error handling.

**Extension API for Context Menus:**

* Added optional `getCanvasMenuItems` and `getNodeMenuItems` methods to
the `ComfyExtension` interface, enabling extensions to provide context
menu items for canvas and node right-click menus (`src/types/comfy.ts`).

* Updated type imports to support the new API, including
`IContextMenuValue`, `LGraphCanvas`, and `LGraphNode`
(`src/types/comfy.ts`, `src/scripts/app.ts`).
[[1]](diffhunk://#diff-c29886a1b0c982c6fff3545af0ca8ec269876c2cf3948f867d08c14032c04d66L1-R5)
[[2]](diffhunk://#diff-bde0dce9fe2403685d27b0e94a938c3d72824d02d01d1fd6167a0dddc6e585ddR10)

**Core Implementation:**

* Implemented `collectCanvasMenuItems` and `collectNodeMenuItems`
methods in the `ComfyApp` class to gather menu items from all
extensions, with robust error handling and logging for extension
failures (`src/scripts/app.ts`).

**Testing:**

* Added a comprehensive test suite for the new context menu extension
API, covering aggregation logic, error handling, and integration
scenarios (`tests-ui/tests/extensions/contextMenuExtension.test.ts`).

This is PR 1 of the 3 PRs in the Contextmenu standardizations.
-#5992
-#5993
arjansingh pushed a commit that referenced this pull request Oct 12, 2025
This pull request introduces a new extension API for context menu
customization, allowing extensions to contribute items to both canvas
and node right-click menus. It adds two collection methods to the
`ComfyApp` class to aggregate these menu items from all registered
extensions, and updates the extension interface accordingly.
Comprehensive unit tests are included to verify the correct aggregation
behavior and error handling.

**Extension API for Context Menus:**

* Added optional `getCanvasMenuItems` and `getNodeMenuItems` methods to
the `ComfyExtension` interface, enabling extensions to provide context
menu items for canvas and node right-click menus (`src/types/comfy.ts`).

* Updated type imports to support the new API, including
`IContextMenuValue`, `LGraphCanvas`, and `LGraphNode`
(`src/types/comfy.ts`, `src/scripts/app.ts`).
[[1]](diffhunk://#diff-c29886a1b0c982c6fff3545af0ca8ec269876c2cf3948f867d08c14032c04d66L1-R5)
[[2]](diffhunk://#diff-bde0dce9fe2403685d27b0e94a938c3d72824d02d01d1fd6167a0dddc6e585ddR10)

**Core Implementation:**

* Implemented `collectCanvasMenuItems` and `collectNodeMenuItems`
methods in the `ComfyApp` class to gather menu items from all
extensions, with robust error handling and logging for extension
failures (`src/scripts/app.ts`).

**Testing:**

* Added a comprehensive test suite for the new context menu extension
API, covering aggregation logic, error handling, and integration
scenarios (`tests-ui/tests/extensions/contextMenuExtension.test.ts`).

This is PR 1 of the 3 PRs in the Contextmenu standardizations.
-#5992
-#5993
arjansingh pushed a commit that referenced this pull request Oct 13, 2025
This pull request introduces a new extension API for context menu
customization, allowing extensions to contribute items to both canvas
and node right-click menus. It adds two collection methods to the
`ComfyApp` class to aggregate these menu items from all registered
extensions, and updates the extension interface accordingly.
Comprehensive unit tests are included to verify the correct aggregation
behavior and error handling.

**Extension API for Context Menus:**

* Added optional `getCanvasMenuItems` and `getNodeMenuItems` methods to
the `ComfyExtension` interface, enabling extensions to provide context
menu items for canvas and node right-click menus (`src/types/comfy.ts`).

* Updated type imports to support the new API, including
`IContextMenuValue`, `LGraphCanvas`, and `LGraphNode`
(`src/types/comfy.ts`, `src/scripts/app.ts`).
[[1]](diffhunk://#diff-c29886a1b0c982c6fff3545af0ca8ec269876c2cf3948f867d08c14032c04d66L1-R5)
[[2]](diffhunk://#diff-bde0dce9fe2403685d27b0e94a938c3d72824d02d01d1fd6167a0dddc6e585ddR10)

**Core Implementation:**

* Implemented `collectCanvasMenuItems` and `collectNodeMenuItems`
methods in the `ComfyApp` class to gather menu items from all
extensions, with robust error handling and logging for extension
failures (`src/scripts/app.ts`).

**Testing:**

* Added a comprehensive test suite for the new context menu extension
API, covering aggregation logic, error handling, and integration
scenarios (`tests-ui/tests/extensions/contextMenuExtension.test.ts`).

This is PR 1 of the 3 PRs in the Contextmenu standardizations.
-#5992
-#5993
@Myestery Myestery marked this pull request as ready for review October 14, 2025 02:08
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Oct 14, 2025
@christian-byrne
Copy link
Contributor

Is this ready for review or still in progress?

@Myestery
Copy link
Collaborator Author

Is this ready for review or still in progress?

its ready

@christian-byrne
Copy link
Contributor

I'm not able to make the code work. Right-clicking the canvas seems to throw an error.

@Myestery
Copy link
Collaborator Author

I'm not able to make the code work. Right-clicking the canvas seems to throw an error.

Okay let me check that

@github-actions
Copy link

github-actions bot commented Oct 20, 2025

Bundle Size Report

Summary

  • Raw size: 12.3 MB baseline 12.3 MB — 🔴 +4.24 kB
  • Gzip: 2.49 MB baseline 2.48 MB — 🔴 +1.2 kB
  • Brotli: 1.96 MB baseline 1.96 MB — 🔴 +1.08 kB
  • Bundles: 56 current • 56 baseline • 12 added / 12 removed

Category Glance
App Entry Points 🔴 +3.17 kB (3.29 MB) · Graph Workspace 🔴 +1.07 kB (714 kB) · Vendor & Third-Party ⚪ 0 B (5.36 MB) · Other ⚪ 0 B (2.58 MB) · Panels & Settings ⚪ 0 B (294 kB) · UI Components ⚪ 0 B (12.3 kB) · + 3 more

Per-category breakdown
App Entry Points — 3.29 MB (baseline 3.29 MB) • 🔴 +3.17 kB _Main entry bundles and manifests_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------- | ------- | ------- | ----------------------- | ---------------------- | ----------------------- | | **assets/index-DePU7lXW.js** _(new)_ | — | 2.68 MB | 🔴 +2.68 MB | 🔴 +557 kB | 🔴 +422 kB | | ~~assets/index-Bmbgidu2.js~~ _(removed)_ | 2.68 MB | — | 🟢 -2.68 MB | 🟢 -556 kB | 🟢 -421 kB | | ~~assets/index-B-aay2HB.js~~ _(removed)_ | 614 kB | — | 🟢 -614 kB | 🟢 -114 kB | 🟢 -90.1 kB | | **assets/index-BavvOtwb.js** _(new)_ | — | 613 kB | 🔴 +613 kB | 🔴 +114 kB | 🔴 +90 kB |

Status: 2 added / 2 removed

Graph Workspace — 714 kB (baseline 713 kB) • 🔴 +1.07 kB _Graph editor runtime, canvas, workflow orchestration_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | -------------------------------------------- | ------ | ------ | ---------------------- | ---------------------- | ---------------------- | | **assets/GraphView-DHr0F4TK.js** _(new)_ | — | 714 kB | 🔴 +714 kB | 🔴 +140 kB | 🔴 +108 kB | | ~~assets/GraphView-BQu7TvL_.js~~ _(removed)_ | 713 kB | — | 🟢 -713 kB | 🟢 -140 kB | 🟢 -108 kB |

Status: 1 added / 1 removed

Views & Navigation — 8.15 kB (baseline 8.15 kB) • ⚪ 0 B _Top-level views, pages, and routed surfaces_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/UserSelectView-D-DTHlC0.js** _(new)_ | — | 8.15 kB | 🔴 +8.15 kB | 🔴 +2.47 kB | 🔴 +2.16 kB | | ~~assets/UserSelectView-DawphGZS.js~~ _(removed)_ | 8.15 kB | — | 🟢 -8.15 kB | 🟢 -2.47 kB | 🟢 -2.15 kB |

Status: 1 added / 1 removed

Panels & Settings — 294 kB (baseline 294 kB) • ⚪ 0 B _Configuration panels, inspectors, and settings screens_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/CreditsPanel-BjVb5pKF.js** _(new)_ | — | 22.1 kB | 🔴 +22.1 kB | 🔴 +5.28 kB | 🔴 +4.61 kB | | ~~assets/CreditsPanel-DzRoZr5x.js~~ _(removed)_ | 22.1 kB | — | 🟢 -22.1 kB | 🟢 -5.28 kB | 🟢 -4.6 kB | | **assets/KeybindingPanel-jAne56fF.js** _(new)_ | — | 15.2 kB | 🔴 +15.2 kB | 🔴 +3.77 kB | 🔴 +3.31 kB | | ~~assets/KeybindingPanel-Otkwl45J.js~~ _(removed)_ | 15.2 kB | — | 🟢 -15.2 kB | 🟢 -3.76 kB | 🟢 -3.31 kB | | ~~assets/ExtensionPanel-BT2N9Yco.js~~ _(removed)_ | 12.1 kB | — | 🟢 -12.1 kB | 🟢 -2.83 kB | 🟢 -2.48 kB | | **assets/ExtensionPanel-CQE-DFl_.js** _(new)_ | — | 12.1 kB | 🔴 +12.1 kB | 🔴 +2.83 kB | 🔴 +2.48 kB | | ~~assets/AboutPanel-BbtR4v-_.js~~ _(removed)_ | 10.3 kB | — | 🟢 -10.3 kB | 🟢 -2.66 kB | 🟢 -2.32 kB | | **assets/AboutPanel-DvClxgar.js** _(new)_ | — | 10.3 kB | 🔴 +10.3 kB | 🔴 +2.66 kB | 🔴 +2.32 kB | | ~~assets/ServerConfigPanel-DC5b9OPz.js~~ _(removed)_ | 8.2 kB | — | 🟢 -8.2 kB | 🟢 -2.17 kB | 🟢 -1.89 kB | | **assets/ServerConfigPanel-nfgozpCg.js** _(new)_ | — | 8.2 kB | 🔴 +8.2 kB | 🔴 +2.17 kB | 🔴 +1.9 kB | | ~~assets/UserPanel-CJxDkvso.js~~ _(removed)_ | 7.91 kB | — | 🟢 -7.91 kB | 🟢 -2.06 kB | 🟢 -1.79 kB | | **assets/UserPanel-t6c-T5MY.js** _(new)_ | — | 7.91 kB | 🔴 +7.91 kB | 🔴 +2.06 kB | 🔴 +1.79 kB | | assets/settings-B-df0dZe.js | 20.7 kB | 20.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CI6OKvJn.js | 22.9 kB | 22.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CXGVj_nD.js | 24.5 kB | 24.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DfQ6dSJj.js | 31.6 kB | 31.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DJ2QgDzm.js | 25.2 kB | 25.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DRNLPMG6.js | 23.7 kB | 23.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DVVycxDc.js | 19.9 kB | 19.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-G6Dybj1b.js | 24.1 kB | 24.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-M6_GZccG.js | 26 kB | 26 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |

Status: 6 added / 6 removed

UI Components — 12.3 kB (baseline 12.3 kB) • ⚪ 0 B _Reusable component library chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ----------------------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | ~~assets/ComfyQueueButton-1e-mP3q_.js~~ _(removed)_ | 11.1 kB | — | 🟢 -11.1 kB | 🟢 -2.75 kB | 🟢 -2.44 kB | | **assets/ComfyQueueButton-qbwbkAiz.js** _(new)_ | — | 11.1 kB | 🔴 +11.1 kB | 🔴 +2.76 kB | 🔴 +2.44 kB | | assets/UserAvatar.vue_vue_type_script_setup_true_lang-C9bSkTC5.js | 1.12 kB | 1.12 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |

Status: 1 added / 1 removed

Data & Services — 10 kB (baseline 10 kB) • ⚪ 0 B _Stores, services, APIs, and repositories_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/keybindingService-fuXA4kuO.js** _(new)_ | — | 7.21 kB | 🔴 +7.21 kB | 🔴 +1.75 kB | 🔴 +1.51 kB | | ~~assets/keybindingService-IUN9y5Z1.js~~ _(removed)_ | 7.21 kB | — | 🟢 -7.21 kB | 🟢 -1.75 kB | 🟢 -1.51 kB | | assets/serverConfigStore-CvX_HMTJ.js | 2.79 kB | 2.79 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |

Status: 1 added / 1 removed

Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 B _Helpers, composables, and utility bundles_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/mathUtil-CTARWQ-l.js | 1.07 kB | 1.07 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |
Vendor & Third-Party — 5.36 MB (baseline 5.36 MB) • ⚪ 0 B _External libraries and shared vendor chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/vendor-other-EuL9bHKm.js | 3.22 MB | 3.22 MB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-primevue-PESgPnbc.js | 517 B | 517 B | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-tiptap-DY0_3CMM.js | 232 kB | 232 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-visualization-BEfdbjRw.js | 1.82 MB | 1.82 MB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-vue-Di9L6lvm.js | 92.4 kB | 92.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |
Other — 2.58 MB (baseline 2.58 MB) • ⚪ 0 B _Bundles that do not match a named category_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/commands-B2KZRBmX.js | 15.1 kB | 15.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Bw-ckyga.js | 13.9 kB | 13.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-C_NmM85I.js | 13.8 kB | 13.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-CuozCW4W.js | 14 kB | 14 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DGfVUJCR.js | 16.2 kB | 16.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-dOJNDogK.js | 14.5 kB | 14.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DwiE551e.js | 14.7 kB | 14.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Fw7mvqSy.js | 13.1 kB | 13.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-FXnO1W4Q.js | 13.2 kB | 13.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bgu6_Hvd.js | 59.5 kB | 59.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bv0L0qvp.js | 93 kB | 93 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C3Doz3n_.js | 67.6 kB | 67.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C7eBl607.js | 70.7 kB | 70.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CHiV9ds2.js | 76.4 kB | 76.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CIc79Nts.js | 68.5 kB | 68.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-DK5LmuBm.js | 58.8 kB | 58.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-J1nit7cj.js | 66.3 kB | 66.3 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-W97XgvAQ.js | 80.4 kB | 80.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BePSqkA4.js | 195 kB | 195 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BfT7dJcF.js | 204 kB | 204 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BiAtoiXc.js | 194 kB | 194 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CDfbduPY.js | 219 kB | 219 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CDurg_KW.js | 197 kB | 197 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CE-vG3RG.js | 182 kB | 182 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-DAwVV156.js | 200 kB | 200 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-DexhCMEi.js | 233 kB | 233 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-kTrYLFPK.js | 184 kB | 184 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |

Copy link
Contributor

@DrJKL DrJKL left a comment

Choose a reason for hiding this comment

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

Which custom nodes did you test this with, so I can try it myself?

Comment on lines +23 to +26
setCurrentExtension(extensionName: string | null) {
this.currentExtension = extensionName
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please find a way to not have to track state mutably like this. Maybe a stack of operations where the mutation is atomic and the extension's identity is an immutable property?

private originalMethods = new Map<string, AnyFunction>()
private hasWarned = new Set<string>()
private currentExtension: string | null = null
private isExtracting = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that isExtracting is clear, descriptive, and unambiguous in context. But also, I think there's a way to do this without needing to have this semaphore.

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

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

Can you make the PR to add https://docs.comfy.org/custom-nodes/js/context-menu-migration (the link used in the console.warn)? We can also just make a new page in https://github.com/Comfy-Org/docs/tree/main/custom-nodes/js (following the patterns/style of the existing pages with a brief explanation of how to use the API then some snippets we have verified work -- I usually verify just in devtools console by pasting my snippet)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants