mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-25 07:05:26 +00:00
d2f7be718072f4b1a89e836b98925d813f837708
101 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
pythongosssss
|
4321013798 |
fix: resolve widget input link position drift on reload (#12214)
## Summary The position of a link relative to its slot was able to drift on load, due to widgets inside a node being able to resize without triggering an node-level resize event (min-height node with space at the bottom could have widgets expand into free space, causing misalignment). Recreation: 1. Add KSampler 2. Add Float 3. Connect Float to KSamper.denoise 4. Reload workflow (F5) 5. Observe misalignment ## Changes - **What**: - track widget grid element as signal only that triggers resync - node bound calculations skipped for widget signals - prevent setDirty on non-graph nodes (e.g. LGraphNodePreview) - tests ## Review Focus This is a small focused approach to fix the reported issue - it does not address the underlying issue of the layout not being a SSOT. This fix is a small bandaid and investigation into resolving the layout SOT issue is not impacted by this. ## Screenshots (if applicable) Before: <img width="673" height="374" alt="image" src="https://github.com/user-attachments/assets/2d34b8e3-0731-4fd2-8553-4dd429010ced" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12214-fix-resolve-widget-input-link-position-drift-on-reload-35f6d73d3650814eb31bebb3042ff58b) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
25c2d828c0 |
test: enable vitest/consistent-each-for and migrate .each → .for (#12161)
*PR Created by the Glary-Bot Agent*
---
Enables the oxlint rule `vitest/consistent-each-for` (configured to
prefer `.for` for `test`, `it`, `describe`, and `suite`) and migrates
every `.each` parameterized test in the repo to `.for`. Using `.for`
avoids accidentally splatting tuple elements into separate callback
arguments and exposes `TestContext` as the second callback argument.
The first commit covers the 38 lint-detected files (88 callsites):
renames `.each` → `.for` and updates callback signatures to destructure
when the data is an array of tuples (objects/primitives already work
unchanged with `.for`).
The follow-up commit addresses code review feedback: oxlint's rule does
not recognize `test.each` on extended test bases
(`baseTest.extend(...)`) and skips files in `ignorePatterns`
(`src/extensions/core/*`). These were converted manually so the policy
is uniform across the codebase.
## Verification
- `node_modules/.bin/oxlint src` — 0 errors, 0 `consistent-each-for`
violations
- `pnpm typecheck` — passes
- `pnpm test:unit` — all modified test files pass; pre-existing
environmental flakes (`GraphView.test.ts`, `ColorWidget.test.ts`, etc.,
unchanged here and flaky on `main` in this sandbox) are unrelated
- `pnpm lint` / `pnpm knip` — clean
- Manual verification: 362 tests across 6 representative converted
suites re-run in an interactive shell — all passing
Manual UI verification (Playwright/screenshots) is not applicable:
changes are test-file-only refactors with no production runtime or UI
behavior change.
## Notes on `.for` semantics
- Array-of-tuples (`[[a, b], ...]`) passes the tuple as a single arg, so
callbacks were changed from `(a, b) => …` to `([a, b]) => …`.
- Array-of-objects (`[{a}, …]`) already used destructuring — unchanged.
- Array-of-primitives (`['a', …]`) — callback signature unchanged.
- A handful of complex cases use a small `type Case = [...]` alias plus
`it.for<Case>([...])` to preserve tuple inference where TS narrowed
unions otherwise broke parameter types.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-12161-test-enable-vitest-consistent-each-for-and-migrate-each-for-35e6d73d3650810c9417e07bdd9f27a2)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
|
||
|
jaeone94
|
1ab9752af8 |
fix: keep Reka overlays above PrimeVue dialogs (#12038)
## Summary Temporarily patch FE-569 by keeping the affected portaled Reka dropdowns and menus above their containing PrimeVue dialogs when PrimeVue auto z-index state has been elevated. ## Changes - **What**: Added a small compatibility helper, `usePrimeVueOverlayChildStyle`, that returns an anchor ref plus a computed inline style for child popover content. The helper finds the nearest PrimeVue dialog mask (`.p-dialog-mask` / `.p-overlay-mask`) from the parent surface and, only when found, applies `parent z-index + 1` to the affected Reka overlay content. - **What**: Applied that helper at the exact PrimeVue parent surfaces where the issue was found. This PR does not add a global overlay policy and does not change every Reka select/dropdown in the app. - **What**: Added optional `contentStyle`/`selectContentStyle` plumbing only where needed so the style reaches the actual portaled Reka overlay root. - **What**: Added focused unit coverage for the helper contract: no PrimeVue parent preserves existing stacking, PrimeVue dialog/overlay masks render child content above the parent, low parent z-index values respect the Reka floor, and invalid z-index values do not inject an inline override. - **Approach**: This is intentionally a minimal, parent-scoped band-aid. It avoids a global PrimeVue overlay scanner because global sampling can be polluted by unrelated persistent PrimeVue roots such as Toast and would turn this fix into a broader layering policy. - **Approach**: The patch targets the confirmed failure mode: a Reka child overlay rendering below its owning PrimeVue dialog after PrimeVue autoZIndex has been elevated. It does not attempt to solve PrimeVue z-index globally. - **Lifecycle**: This is temporary migration compatibility. PrimeVue dialogs and controls are being incrementally migrated to Reka UI, so `usePrimeVueOverlayChildStyle` and the optional style props added for FE-569 should be removed once the affected parent surfaces move to Reka. - **Breaking**: None. New props are optional and no public API contract is changed. - **Dependencies**: None. ## Patched Entry Points This PR pinpoints the six affected user-facing surfaces below. Each patch is applied from the PrimeVue dialog parent and passed only to the Reka child overlay content that can render underneath that parent. https://github.com/user-attachments/assets/d0d1522a-ffc7-4934-9e7a-06b83e20f809 1. **Workflow Template Library filters** - **How to enter**: click the Templates button in the left sidebar, or open the Comfy menu and choose **Browse Templates**. - **Affected elements**: the template filter popovers in `WorkflowTemplateSelectorDialog`: **Model**, **Use case**, **Runs on**, and **Sort by**. - **Patch point**: `WorkflowTemplateSelectorDialog.vue` anchors to the template dialog content filter area and passes `selectContentStyle` to the affected `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/3641fa24-da51-4392-a904-9085f8a5a2f4 2. **Manager dialog header controls** - **How to enter**: open Manager from the top/menu Manager entry when the new Manager UI is available. - **Affected elements**: the Manager header controls in `ManagerDialog`: search mode `SingleSelect`, search autocomplete suggestions, and **Sort** `SingleSelect`. - **Patch point**: `ManagerDialog.vue` anchors to the dialog header and passes `selectContentStyle` to those three Reka overlays. https://github.com/user-attachments/assets/cf25cc06-f851-48ef-9d9c-9ec2da8afc06 3. **Asset Browser filter bar** - **How to enter**: open the Asset Browser from an eligible model widget browse action, the Model Library flow, or another `useAssetBrowserDialog` caller. - **Affected elements**: `AssetFilterBar` controls: **File formats**, **Base models**, **Ownership**, and **Sort by**. - **Patch point**: `AssetBrowserModal.vue` anchors to the PrimeVue dialog header and passes the style through `AssetFilterBar` to its `MultiSelect` / `SingleSelect` controls. https://github.com/user-attachments/assets/e27bd805-10c0-4b3b-97f3-9e11faa47021 4. **Asset Browser model info panel** - **How to enter**: open Asset Browser, select an asset, then use the right-side model info panel. - **Affected element**: the **Model type** select in `ModelInfoPanel`. - **Patch point**: `AssetBrowserModal.vue` reuses the same parent-scoped style and passes it to `ModelInfoPanel` as `selectContentStyle`. https://github.com/user-attachments/assets/5e9f7ef0-ebd7-4987-ba1b-2137c034086f 5. **Upload Model confirmation step** - **How to enter**: open Asset Browser, click **Upload**, enter/fetch model metadata, then proceed to the confirmation step. - **Affected element**: the **Model type** `SingleSelect` in `UploadModelConfirmation`. - **Patch point**: `UploadModelConfirmation.vue` anchors within the upload dialog content and passes `selectContentStyle` to the model type selector. https://github.com/user-attachments/assets/ec145f26-8621-455b-915e-bedee47e1cbd 6. **Settings > Keybinding panel controls** - **How to enter**: open Settings from the sidebar/menu, then select the **Keybinding** panel. - **Affected elements**: the keybinding preset select, the preset overflow dropdown menu, and the row context menu inside `KeybindingPanel`. - **Patch point**: `KeybindingPanel.vue` anchors to the settings dialog panel and passes `keybindingOverlayContentStyle` only to those Reka overlay roots. ## Review Focus - Confirm the patch stays narrowly scoped to the six known PrimeVue parent + Reka child overlay surfaces above. - Confirm `contentStyle` reaches the actual portaled Reka overlay content in each patched path. - Confirm the fallback behavior preserves existing stacking when no PrimeVue parent overlay is found; in that case the helper returns an empty style object and leaves existing Tailwind z-index classes alone. - Please avoid expanding this into a larger overlay refactor. The goal is a clean, backport-friendly compatibility patch while the Reka migration continues. Validation performed: - `pnpm exec vitest run src/composables/usePopoverSizing.test.ts` - `pnpm typecheck` - `pnpm lint` (passes with existing unrelated warnings only) - `pnpm format:check` - commit hook lint-staged checks (`oxfmt`, `stylelint`, `oxlint`, `eslint --fix`, `pnpm typecheck`) - pre-push `pnpm knip` Linear: FE-569 ## Bug Screenshots https://github.com/user-attachments/assets/e73761af-9867-4c50-ab0d-4e32e59011e1 https://github.com/user-attachments/assets/145daf4d-3268-428b-9987-1e1afd0b866f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12038-fix-keep-Reka-overlays-above-PrimeVue-dialogs-3596d73d365081e7af49dbc4d3905962) by [Unito](https://www.unito.io) |
||
|
AustinMroz
|
c2abbeda80 |
Fix core node detection for missing nodes (#11809)
Nodes in the 'essentials' category do not have type 'core'. The check has been updated to instead use the dedicated `isCoreNode` prop. No tests currently. The existing tests for this code section all mock out the relevant code path and properly writing a test for this would take far more time than I can allocate right now. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11809-Fix-core-node-detection-for-missing-nodes-3536d73d3650815aabb2deb54c4ecec4) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
11432f7d0e |
refactor: extract missing model refresh pipeline (#11751)
## Summary Extracts the missing-model pipeline orchestration out of `ComfyApp` and into an app-independent platform module, while tightening the workflow-flattening type boundary that refresh needs when rescanning the live LiteGraph graph. This PR is intentionally refactor-heavy. It is the follow-up to the earlier missing-model refresh work: instead of keeping refresh-specific candidate recheck logic beside the UI, this change makes the refresh path reuse the existing missing-model pipeline and removes the direct dependency on private `ComfyApp` pipeline methods. Linear: FE-499 Issues covered by this PR: - Fixes #11678 - Fixes #11680 - Partially addresses #11679 by removing the missing-model refresh path's unsafe `graph.serialize() as unknown as ComfyWorkflowJSON` cast and replacing it with the narrower flattenable workflow contract. Broader workflow serialization/type-boundary cleanup outside this missing-model refresh path remains deferred. ## Changes - **What**: - Added `src/platform/missingModel/missingModelPipeline.ts` as the orchestration module for missing-model detection/verification. - `runMissingModelPipeline(...)` now owns the pipeline previously embedded in `ComfyApp`: - candidate scan and enrichment - active ancestor filtering for muted/bypassed subgraph containers - pending warning cache updates - OSS folder path and file-size follow-up work - cloud asset verification follow-up work - surfaced missing-model errors via the existing execution error store - `refreshMissingModelPipeline(...)` handles the refresh-specific flow: - calls the injected `reloadNodeDefs()` first - serializes the current live graph - preserves model metadata by preferring active workflow `models`, then falling back to current missing-model candidate metadata - delegates back into the same pipeline used during workflow load - Kept `ComfyApp` as the compatibility caller instead of the owner of the pipeline. - `loadGraphData(...)` now calls `runMissingModelPipeline(...)` with `graph`, `graphData`, `missingNodeTypes`, and `silent` options. - `refreshMissingModels(...)` is now a thin wrapper around `refreshMissingModelPipeline(...)` and keeps the existing default `silent: true` refresh behavior. - The new pipeline module does not import `@/scripts/app`; app-owned data/actions are passed in as inputs. - Moved the workflow node-flattening helpers out of `workflowSchema.ts` and into `src/platform/workflow/core/utils/workflowFlattening.ts`. - This includes `flattenWorkflowNodes`, `buildSubgraphExecutionPaths`, and `isSubgraphDefinition`. - The move is intentional: these helpers are not zod schema definitions or workflow validation logic. They are core workflow traversal utilities used to flatten root workflow nodes plus nested subgraph definition nodes into the execution-shaped node list needed by missing-model scanning. - The refresh path receives data from `LGraph.serialize()`, whose return type is serialized LiteGraph data rather than validated `ComfyWorkflowJSON`. Previously this forced unsafe typing like `graph.serialize() as unknown as ComfyWorkflowJSON`. - The new `FlattenableWorkflowGraph` / `FlattenableWorkflowNode` structural contract describes only what flattening actually needs: `nodes`, `definitions.subgraphs`, node `id`, `type`, `mode`, `widgets_values`, and `properties`. - This lets both normal workflow-load data (`ComfyWorkflowJSON`) and refresh-time live graph serialization (`LGraph.serialize()`) flow into the same scan/enrichment path without pretending serialized LiteGraph output is a fully validated workflow schema document. - Updated `missingModelScan.ts` to consume that minimal flattenable workflow shape via `MissingModelWorkflowData`. - `MissingModelWorkflowData` extends the flattenable workflow contract with optional workflow-level `models` metadata. - Removed now-unnecessary casts around execution IDs, flattened nodes, and `widgets_values` object access. - Updated `getSelectedModelsMetadata(...)` to accept readonly widget value arrays so flattened workflow data can stay read-only. - Reduced the exported surface of the new pipeline module after `knip` flagged unused exported internal option/store interfaces. - Kept `workflowSchema.ts` focused on validation schemas. The flattening helpers are not re-exported from the schema module because they are internal workflow core utilities, not public schema API. - **Breaking**: None intended. - Internal imports were updated to the new core utility path. - This repo is not exposing these flattening helpers as a public package API, so the old schema-local helper location is treated as an internal implementation detail. - **Dependencies**: None. ## Review Focus - **Pipeline extraction / dependency direction**: - Please verify that `missingModelPipeline.ts` stays independent from `@/scripts/app`. - `ComfyApp` should remain the caller/adapter, not the owner of missing-model pipeline orchestration. - **Workflow flattening type boundary**: - The main type-cleanup goal is removing the refresh-time `graph.serialize() as unknown as ComfyWorkflowJSON` lie. - `LGraph.serialize()` and validated workflow JSON are not the same contract. The new flattenable workflow contract is deliberately smaller and structural because the missing-model enrichment path only needs enough data to flatten nodes and read embedded model metadata. - This is why the flattening helpers moved from `workflowSchema.ts` to `workflow/core/utils`: the logic is reusable workflow traversal, not validation schema. - **Behavior preservation**: - The PR is intended to preserve existing user-facing missing-model behavior while moving ownership out of `app.ts`. - Existing async follow-up behavior remains intentionally fire-and-forget: - cloud asset verification still surfaces after verification completes - OSS folder paths still update asynchronously before surfacing confirmed missing models - file-size metadata fetching remains asynchronous - More invasive behavior changes, such as adding non-cloud post-fetch `isMissingCandidateActive(...)` re-verification or redesigning the fire-and-forget result contract, are intentionally left for follow-up work because they are not pure extraction. - **Downloadable model metadata**: - `missingModels` returned for download metadata now requires both `url` and `directory`. - Candidates without a directory still remain in `confirmedCandidates`, but they are not exposed as downloadable model metadata. This keeps the returned downloadable list aligned with what the download flow can actually use. - **Test ownership**: - Complex missing-model pipeline behavior tests moved out of `src/scripts/app.test.ts` and into `src/platform/missingModel/missingModelPipeline.test.ts`. - `app.test.ts` now only covers thin delegation for `app.refreshMissingModels(...)`. - Workflow flattening tests moved with the helper from schema tests into `src/platform/workflow/core/utils/workflowFlattening.test.ts`. - **Deferred follow-ups**: - Broader function decomposition for cognitive complexity. - Wider dependency-injection/port cleanup for stores and services beyond the app boundary. - Cloud-specific pipeline unit tests, which need a separate `isCloud` mocking strategy. - Additional E2E coverage expansion beyond the existing OSS refresh path. - More general workflow serialization/type-boundary cleanup outside the missing-model refresh path. ## Validation - `pnpm format` - `pnpm lint` - Passed. Existing lint output included a pre-existing `no-misused-spread` warning and icon-name logs, but the command exited successfully. - `pnpm typecheck` - `pnpm test:unit` - `714 passed`, `9514 passed | 8 skipped` - Pre-push `pnpm knip` - Passed after reducing the exported surface of the new pipeline module. ## Screenshots (if applicable) Not applicable. This PR is a pipeline/type-boundary refactor with no UI changes. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11751-refactor-extract-missing-model-refresh-pipeline-3516d73d3650816d9245d4b1324b71c9) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
9d61b4df06 |
feat: ECS Phase 0b — ID type aliases (FE-166/475/476/477) (#11699)
## Summary
ECS Phase 0b — type-only ID aliases. Builds on FE-165 (centralized
version counter, base of this PR) and adds a tranche of named ID aliases
plus mechanical adoption at known call sites.
This PR now covers four child tickets:
- **FE-166** — adds `GroupId` (in `LGraphGroup.ts`) and `SlotIndex` (in
`interfaces.ts`); re-exported from the litegraph barrel.
- **FE-475** — mechanical adoption of existing aliases (`NodeId`,
`LinkId`, `RerouteId`, `GroupId`, `SlotIndex`, `ExecutionId`) across
litegraph at the audit-listed sites:
`LGraphState.lastGroupId/lastLinkId/lastRerouteId`,
`LGraphExtra.linkExtensions`, `ISerialisedGroup.id`,
`ISerialisedGraph.last_link_id`, `LinkNetwork.removeReroute`,
`INodeOutputSlot.slot_index`, `LGraphNode.{setOutputDataType,
getInputDataType, getOutputPos}` slot params,
`ExecutableNodeDTO.inputs[].linkId` + execution-id locals, and
`RenderLink/MovingLinkBase.fromSlotIndex` (plus subclasses that
redeclare).
- **FE-476** — adds `SubgraphId = UUID` in `LGraph.ts`; adopted at
`_subgraphs` Map, `findUsedSubgraphIds`, `getDirectSubgraphIds`, and
`ExportedSubgraphInstance.type`. Re-exported from the litegraph barrel.
- **FE-477** — adds app-domain entity aliases at their closest
schema/types files: `WorkflowId`, `AssetId`, `PromptId` (propagated as
existing `JobId`), `TaskId`, `UserId`, `WorkspaceId`,
`WorkspaceInviteId`, `NodePackId`. Adopted at primary use sites (entity
id fields, store state, service signatures).
## Entity reference
### ID aliases at a glance
| Alias | Underlying | Defined in | Identifies |
| --- | --- | --- | --- |
| `NodeId` | `number \| string` | `litegraph/LGraphNode.ts` | A node
within a graph |
| `LinkId` | `number` | `litegraph/LLink.ts` | A connection between two
slots |
| `RerouteId` | `number` | `litegraph/Reroute.ts` | A reroute waypoint
on a link |
| `GroupId` *(new)* | `number` | `litegraph/LGraphGroup.ts` | A visual
group of nodes |
| `SlotIndex` *(new)* | `number` | `litegraph/interfaces.ts` | A slot's
position on a node |
| `ExecutionId` | `string` | `litegraph/types/serialisation.ts` | A node
within a subgraph instance |
| `SubgraphId` *(new)* | `UUID` | `litegraph/LGraph.ts` | A subgraph
definition |
| `WorkflowId` *(new)* | `string` |
`platform/workflow/validation/schemas/workflowSchema.ts` | A saved
workflow document |
| `AssetId` *(new)* | `string` |
`platform/assets/schemas/assetSchema.ts` | A binary asset (model, image,
etc.) |
| `JobId` *(reused as `PromptId`)* | `string` | `schemas/apiSchema.ts` |
A queued prompt execution |
| `TaskId` *(new)* | `string` | `platform/tasks/services/taskService.ts`
| A backend background task |
| `UserId` *(new)* | `string` | `types/authTypes.ts` | An authenticated
user |
| `WorkspaceId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A workspace |
| `WorkspaceInviteId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A pending workspace invite |
| `NodePackId` *(new)* | `string` |
`workbench/extensions/manager/types/comfyManagerTypes.ts` | A Comfy
Registry / Manager node pack |
### How the entities relate
```mermaid
flowchart TB
subgraph LG["🎨 Litegraph entities"]
direction TB
Graph["LGraph<br/>id: SubgraphId"]
Subgraph["Subgraph<br/>id: SubgraphId"]
Node["LGraphNode<br/>id: NodeId"]
Link["LLink<br/>id: LinkId"]
Reroute["Reroute<br/>id: RerouteId"]
Group["LGraphGroup<br/>id: GroupId"]
Slot["INodeSlot<br/>slot_index: SlotIndex"]
Exec["ExecutableNodeDTO<br/>id: ExecutionId"]
end
subgraph AP["🌐 App-domain entities"]
direction TB
Workflow["ComfyWorkflow<br/>id: WorkflowId"]
Job["Prompt / Job<br/>id: JobId ≡ PromptId"]
Task["Task<br/>id: TaskId"]
Asset["Asset<br/>id: AssetId"]
Workspace["Workspace<br/>id: WorkspaceId"]
Invite["WorkspaceInvite<br/>id: WorkspaceInviteId"]
User["User<br/>id: UserId"]
Pack["NodePack<br/>id: NodePackId"]
end
Subgraph -. extends .-> Graph
Graph --> Node
Graph --> Link
Graph --> Group
Node --> Slot
Link --> Reroute
Link --> Slot
Node -. instantiates .-> Subgraph
Exec -. wraps .-> Node
Workflow -. serializes .-> Graph
Job --> Workflow
Job --> Task
Task --> Asset
Workspace --> Workflow
User --> Workspace
Workspace --> Invite
Pack -. provides .-> Node
```
Solid arrows are containment / direct references; dashed arrows are *“is
a kind of”* (`extends`) or cross-layer relationships (e.g. a
`ComfyWorkflow` *serializes* an `LGraph`; a `NodePack` *provides* node
definitions).
## Explicit non-goals
- `LGraphState.lastNodeId` is intentionally kept as bare `number`
(auto-increment counter; would widen if aliased to `NodeId = number |
string`).
- No new `SubgraphSlotId` alias — verified subsidiary (subgraph IO slots
are addressed via `SUBGRAPH_INPUT_ID/OUTPUT_ID` sentinel + numeric array
index, not by UUID alone).
- No `WidgetName`, `SlotName`, `WorkspaceMemberId` — verified subsidiary
(only meaningful inside a parent or as a relationship).
- No re-typing of `LGraph.id` / `Subgraph.id` — references adopt
`SubgraphId`, but the inherited UUID typing is left intact (minimal
diff).
## Type-only
All changes are structural-equivalent type aliases. No runtime behavior
changes. No new exports beyond the aliases themselves. No generated code
modified.
## Verification
- `pnpm typecheck` ✅
- `pnpm knip` ✅
- Scoped `npx eslint` on changed files ✅
- Lint-staged hooks (oxfmt, oxlint, eslint, typecheck) passed on every
commit
## Notes for reviewers
This branch was rebased onto `main` after FE-165 (`a441364a5`, PR
#11698) merged independently — the auto-skipped FE-165 commit is no
longer part of this PR. Six commits remain (oldest → newest):
| Commit | Maps to | Summary |
| --- | --- | --- |
| `e8e7ff795` | FE-166 | Add `GroupId` and `SlotIndex` aliases + barrel
re-exports |
| `e0bcb75a0` | FE-476 | Add `SubgraphId = UUID` alias |
| `2c136afb9` | FE-477 + FE-475 bulk | Add app-domain aliases;
mechanical adoption of
`NodeId`/`LinkId`/`RerouteId`/`GroupId`/`SlotIndex`/`ExecutionId`/`SubgraphId`
at audit-listed litegraph sites |
| `06d6e6a8b` | FE-477 | Adopt `TaskId` in asset stores |
| `f943e1c2b` | FE-476 | Adopt `SubgraphId` at remaining UUID reference
sites (`LGraphCanvas` clipboard map + paste, `SubgraphNode.type`) |
| `1739d5241` | review feedback | Tighten alias usage:
`linkExtensions.parentId: RerouteId`, drop redundant `String()` wraps in
`executionStore`, type `assetExportStore` map as `Map<TaskId,
AssetExport>` |
FE-475's mechanical adoption is bundled into `2c136afb9` rather than a
dedicated commit (parallel-agent execution on a shared working tree);
the substitutions themselves are complete — see the diff under
`src/lib/litegraph/src/`. PR will be squash-merged, so commit
granularity is informational.
Fixes FE-166
Fixes FE-475
Fixes FE-476
Fixes FE-477
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Dante
|
3340b77908 |
test: add unit tests for value-control widget family (#11440)
## Summary Adds 42 unit tests across 5 files covering the value-control widget family — first batch of a broader effort to raise widget-test coverage. ## Changes - **What**: - `WidgetInputNumber.test.ts` (9) — variant selection by widget.type (int/float/slider/gradientslider), controlWidget wrapping in WidgetWithControl, modelValue forwarding. - `WidgetInputNumberGradientSlider.test.ts` (11) — initial value, min/max/disabled pass-through, default vs custom gradient stops, precision-derived step, WidgetLayoutField wrapping. - `WidgetWithControl.test.ts` (5) — renders passed component with widget/modelValue, initializes ValueControlButton mode from widget.controlWidget.value, calls controlWidget.update on mode change. - `ValueControlButton.test.ts` (11) — i18n aria-label per mode, text vs icon rendering, pointer and keyboard activation, `type="button"` safety. - `ValueControlPopover.test.ts` (6) — BEFORE/AFTER copy from settingStore, four option render, v-model updates on selection. ## Review Focus - Stack follows the existing widget-test pattern (`@testing-library/vue` + PrimeVue + `createI18n` where needed, no `@vue/test-utils`). - `createMockWidget` from `widgetTestUtils.ts` reused; no new helper extracted (YAGNI — per-file `renderComponent` stays ~10 lines). - `WidgetWithControl` watcher test asserts first-arg of `update` since the vue watch callback passes `(newVal, oldVal, onCleanup)`. - No changes to any widget component source — tests-only PR. This is one of several focused PRs in a widget-test-coverage sequence; subsequent PRs cover form-dropdown internals, utility widgets, media/graph/canvas widgets, and e2e value-type specs. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11440-test-add-unit-tests-for-value-control-widget-family-3486d73d3650813891e1fe8d45eaecaf) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
Dr.Lt.Data
|
bd96bdf4cc |
fix(manager): migrate 4 endpoints GET→POST for CSRF hardening (#11520)
## Summary Align `comfyManagerService` and Manager UI state with CSRF hardening in [Comfy-Org/ComfyUI-Manager#2818](https://github.com/Comfy-Org/ComfyUI-Manager/pull/2818) (4.2.0, Content-Type gate + GET→POST migration) and [Comfy-Org/ComfyUI-Manager#2823](https://github.com/Comfy-Org/ComfyUI-Manager/pull/2823) (4.2.1, `extension.manager.supports_csrf_post` feature flag). ## Changes - **Service layer**: Convert 4 state-mutation endpoints (`START_QUEUE`, `UPDATE_ALL`, `UPDATE_COMFYUI`, `REBOOT`) from GET to POST. `body=null` + axios default `Content-Type: application/json` is allowed by the backend's `reject_simple_form_post` gate (only the three CORS simple-form types are rejected). - **UI/state layer**: Add `ManagerUIState.INCOMPATIBLE` triggered when the backend advertises `supports_manager_v4` but not `supports_csrf_post`. Manager UI is treated as "not installed" — buttons hide via `shouldShowManagerButtons` with zero call-site changes across `TopMenuSection`, `MissingNodeCard`, `MissingPackGroupRow`, `TabErrors`. - **Graceful degraded mode**: One-shot upgrade toast (warn, 15s) dispatched via `watch(immediate:true)` with a module-level guard that survives multiple composable instances. `openManager()` re-emits on explicit user action so stale shortcuts still surface guidance. i18n (en/ko) covering Desktop / standalone pip / Manager UI self-update paths. - **Breaking**: None. Existing policies preserved (`--enable-manager` absent → `DISABLED`; `--enable-manager-legacy-ui` → `LEGACY_UI`; feature flags not yet loaded → `NEW_UI` transient fallback). ## Review Focus - Decision-tree ordering in `useManagerState.ts`: `supports_csrf_post` check evaluates before `NEW_UI`/`LEGACY_UI` branches so stale Manager backends never reach the enabled paths. - Toast guard: module-level `incompatibleToastShown` survives multiple composable instances (tests verify 3× `useManagerState()` = 1 toast call). - `generatedManagerTypes.ts` still declares the 4 endpoints as GET; regeneration follows once Manager 4.2.1 OpenAPI is published. Runtime is unaffected since axios operates on the route string. ## References - [Comfy-Org/ComfyUI-Manager#2818](https://github.com/Comfy-Org/ComfyUI-Manager/pull/2818) — CSRF Content-Type gate + GET→POST migration (4.2.0) - [Comfy-Org/ComfyUI-Manager#2823](https://github.com/Comfy-Org/ComfyUI-Manager/pull/2823) — `supports_csrf_post` feature flag (4.2.1) - [comfyui-manager 4.2.1 on PyPI](https://pypi.org/project/comfyui-manager/4.2.1) — release package |
||
|
Christian Byrne
|
ef59f46495 |
refactor: migrate cn imports from @/utils/tailwindUtil shim to @comfyorg/tailwind-utils directly (#11453)
*PR Created by the Glary-Bot Agent* --- ## Summary - Replace all `cn` / `ClassValue` imports from the `@/utils/tailwindUtil` re-export shim with direct imports from `@comfyorg/tailwind-utils` across 198 source files in `src/` and 3 in `apps/desktop-ui/` - Delete both shim files (`src/utils/tailwindUtil.ts` and `apps/desktop-ui/src/utils/tailwindUtil.ts`) - Add explicit `@comfyorg/tailwind-utils` dependency to `apps/desktop-ui/package.json` - Update documentation references in `AGENTS.md`, `docs/guidance/design-standards.md`, and `docs/guidance/vue-components.md` Fixes #11288 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11453-refactor-migrate-cn-imports-from-utils-tailwindUtil-shim-to-comfyorg-tailwind-utils--3486d73d365081ec92cce91fbf88e6e4) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Christian Byrne
|
a3893a593d |
refactor: move select components from input/ to ui/ component library (#11378)
*PR Created by the Glary-Bot Agent* --- ## Summary Reconciles `src/components/input/` (older select components) into `src/components/ui/` (internal component library), eliminating the separate `input/` directory entirely. ## Changes - **Move MultiSelect** → `src/components/ui/multi-select/MultiSelect.vue` - **Move SingleSelect** → `src/components/ui/single-select/SingleSelect.vue` - **Extract shared resources** → `src/components/ui/select/types.ts` (SelectOption type) and `src/components/ui/select/select.variants.ts` (CVA styling variants) - **Update 7 consuming files** to use new import paths - **Update 1 test file** (AssetFilterBar.test.ts mock paths) - **Move stories and tests** alongside their components - **Delete `src/components/input/`** directory ## Context The `input/` directory contained only MultiSelect and SingleSelect — two well-built components that already used the same stack as `ui/` (Reka UI, CVA, Tailwind 4, Composition API). MultiSelect even imported `ui/button/Button.vue`. Moving them into `ui/` removes the split and consolidates all reusable components in one place. No API changes — all component props, slots, events, and behavior are preserved exactly. ## Verification - `pnpm typecheck` ✅ - `pnpm build` ✅ - `pnpm lint` (stylelint + oxlint + eslint) ✅ - All 15 relevant tests pass (MultiSelect: 5, SingleSelect: 2, AssetFilterBar: 8) ✅ - `pnpm knip` — no dead exports ✅ - No stale `@/components/input/` references remain ✅ - Pre-commit hooks pass ✅ - Git detected all moves as renames (97-100% similarity) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11378-refactor-move-select-components-from-input-to-ui-component-library-3476d73d3650810e99b4c3e0842e67f3) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Alexander Brown
|
f90d6cf607 |
test: migrate 132 test files from @vue/test-utils to @testing-library/vue (#10965)
## Summary Migrate 132 test files from `@vue/test-utils` (VTU) to `@testing-library/vue` (VTL) with `@testing-library/user-event`, adopting user-centric behavioral testing patterns across the codebase. ## Changes - **What**: Systematic migration of component/unit tests from VTU's `mount`/`wrapper` API to VTL's `render`/`screen`/`userEvent` API across 132 files in `src/` - **Breaking**: None — test-only changes, no production code affected ### Migration breakdown | Batch | Files | Description | |-------|-------|-------------| | 1 | 19 | Simple render/assert tests | | 2A | 16 | Interactive tests with user events | | 2B-1 | 14 | Interactive tests (continued) | | 2B-2 | 32 | Interactive tests (continued) | | 3A–3E | 51 | Complex tests (stores, composables, heavy mocking) | | Lint fix | 7 | `await` on `fireEvent` calls for `no-floating-promises` | | Review fixes | 15 | Address CodeRabbit feedback (3 rounds) | ### Review feedback addressed - Removed class-based assertions (`text-ellipsis`, `pr-3`, `.pi-save`, `.skeleton`, `.bg-black\/15`, Tailwind utilities) in favor of behavioral/accessible queries - Added null guards before `querySelector` casts - Added `expect(roots).toHaveLength(N)` guards before indexed NodeList access - Wrapped fake timer tests in `try/finally` for guaranteed cleanup - Split double-render tests into focused single-render tests - Replaced CSS class selectors with `screen.getByText`/`screen.getByRole` queries - Updated stubs to use semantic `role`/`aria-label` instead of CSS classes - Consolidated redundant edge-case tests - Removed manual `document.body.appendChild` in favor of VTL container management - Used distinct mock return values to verify command wiring ### VTU holdouts (2 files) These files intentionally retain `@vue/test-utils` because their components use `<script setup>` without `defineExpose`, making internal computed properties and methods inaccessible via VTL: 1. **`NodeWidgets.test.ts`** — partial VTU for `vm.processedWidgets` 2. **`WidgetSelectDropdown.test.ts`** — full VTU for heavy `wrapper.vm.*` access ## Follow-up Deferred items (`ComponentProps` typing, camelCase listener props) tracked in #10966. ## Review Focus - Test correctness: all migrated tests preserve original behavioral coverage - VTL idioms: proper use of `screen` queries, `userEvent`, and accessibility-based selectors - The 2 VTU holdout files are intentional, not oversights ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10965-test-migrate-132-test-files-from-vue-test-utils-to-testing-library-vue-33c6d73d36508199a6a7e513cf5d8296) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> |
||
|
Alexander Brown
|
661e3d7949 |
test: migrate as unknown as to @total-typescript/shoehorn (#10761)
*PR Created by the Glary-Bot Agent* --- ## Summary - Replace all `as unknown as Type` assertions in 59 unit test files with type-safe `@total-typescript/shoehorn` functions - Use `fromPartial<Type>()` for partial mock objects where deep-partial type-checks (21 files) - Use `fromAny<Type>()` for fundamentally incompatible types: null, undefined, primitives, variables, class expressions, and mocks with test-specific extra properties that `PartialDeepObject` rejects (remaining files) - All explicit type parameters preserved so TypeScript return types are correct - Browser test `.spec.ts` files excluded (shoehorn unavailable in `page.evaluate` browser context) ## Verification - `pnpm typecheck` ✅ - `pnpm lint` ✅ - `pnpm format` ✅ - Pre-commit hooks passed (format + oxlint + eslint + typecheck) - Migrated test files verified passing (ran representative subset) - No test behavior changes — only type assertion syntax changed - No UI changes — screenshots not applicable ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10761-test-migrate-as-unknown-as-to-total-typescript-shoehorn-3336d73d365081f6b8adc44db5dcc380) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Amp <amp@ampcode.com> |
||
|
Alexander Brown
|
5b4ebf4d99 |
test: audit skipped tests — prune stale, re-enable stable, remove dead code (#10312)
## Summary Audit all skipped/fixme tests: delete stale tests whose underlying features were removed, re-enable tests that pass with minimal fixes, and remove orphaned production code that only the deleted tests exercised. Net result: **−2,350 lines** across 50 files. ## Changes - **Pruned stale skipped tests** (entire files deleted): - `LGraph.configure.test.ts`, `LGraph.constructor.test.ts` — tested removed LGraph constructor paths - `LGraphCanvas.ghostAutoPan.test.ts`, `LGraphCanvas.linkDragAutoPan.test.ts`, `useAutoPan.test.ts`, `useSlotLinkInteraction.autoPan.test.ts` — tested removed auto-pan feature - `useNodePointerInteractions.test.ts` — single skipped test for removed callback - `ImageLightbox.test.ts` — component replaced by `MediaLightbox` - `appModeWidgetRename.spec.ts` (E2E) — feature removed; helper `AppModeHelper.ts` also deleted - `domWidget.spec.ts`, `widget.spec.ts` (E2E) — tested removed widget behavior - **Removed orphaned production code** surfaced by test pruning: - `useAutoPan.ts` — composable + 93 lines of auto-pan logic in `LGraphCanvas.ts` - `ImageLightbox.vue` — replaced by `MediaLightbox` - Auto-pan integration in `useSlotLinkInteraction.ts` and `useNodeDrag.ts` - Dead settings (`LinkSnapping.AutoPanSpeed`, `LinkSnapping.AutoPanMargin`) in `coreSettings.ts` and `useLitegraphSettings.ts` - Unused subgraph methods (`SubgraphNode.getExposedInput`, `SubgraphInput.getParentInput`) - Dead i18n key, dead API schema field, dead fixture exports (`dirtyTest`, `basicSerialisableGraph`) - Dead test utility `litegraphTestUtils.ts` - **Re-enabled skipped tests with minimal fixes**: - `useBrowserTabTitle.test.ts` — removed skip, test passes as-is - `eventUtils.test.ts` — replaced MSW dependency with direct `fetch` mock - `SubscriptionPanel.test.ts` — stabilized button selectors, timezone-safe date assertion - `LinkConnector.test.ts` — removed stale describe blocks, kept passing suite - `widgetUtil.test.ts` — removed skipped tests for deleted functionality - `comfyManagerStore.test.ts` — removed skipped `isPackInstalling` / `action buttons` / `loading states` blocks - **Re-enabled then re-skipped 3 flaky E2E tests** (fail in CI for pre-existing reasons): - `browserTabTitle.spec.ts` — canvas click timeout (element not visible) - `groupNode.spec.ts` — screenshot diff (stale golden image) - `nodeSearchBox.spec.ts` — `p-dialog-mask` intercepts pointer events - **Simplified production code** alongside test cleanup: - `useNodeDrag.ts` — removed auto-pan integration, simplified from 170→100 lines - `DropZone.vue` — refactored URL-drop handling, removed unused code path - `ToInputFromIoNodeLink.ts`, `SubgraphInputEventMap.ts` — removed dead subgraph wiring - **Dependencies**: none - **Breaking**: none (all removed code was internal/unused) ## Review Focus - Confirm deleted production code (`useAutoPan`, `ImageLightbox`, subgraph methods) has no remaining callers - Validate that simplified `useNodeDrag.ts` preserves drag behavior without auto-pan - Check that re-skipped E2E tests have clear skip reasons for future triage ## Screenshots (if applicable) N/A --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Dante
|
82242f1b00 |
refactor: add Badge component and fix twMerge font-size detection (#10580)
## Summary - Rename `text-xxxs`/`text-xxs` to `text-3xs`/`text-2xs` in design system CSS — fixes `tailwind-merge` incorrectly classifying custom font-size utilities as color classes, which clobbered text color - Add `Badge` component with updated severity colors matching Figma design (white text on colored backgrounds) - Add Badge stories under `Components/Badges/Badge` - Add unit tests including twMerge regression coverage Split from #10438 per review feedback — this PR contains the foundational Badge component; migration of consumers follows in a separate PR. ## Test plan - [x] Unit tests pass (`Badge.test.ts` — 12 tests) - [x] Typecheck passes - [x] Lint passes - [ ] Verify Badge stories render correctly in Storybook - [ ] Verify existing components using `text-2xs`/`text-3xs` render unchanged Fixes #10438 (partial) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10580-refactor-add-Badge-component-and-fix-twMerge-font-size-detection-32f6d73d3650810dae7cd0d4af67fd1c) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
db1257fdb3 |
test: migrate 8 hard-case component tests from VTU to VTL (Phase 3) (#10493)
## Summary Phase 3 of the VTL migration: migrate 8 hard-case component tests from @vue/test-utils to @testing-library/vue (68 tests). Stacked on #10490. ## Changes - **What**: Migrate SignInForm, CurrentUserButton, NodeSearchBoxPopover, BaseThumbnail, JobAssetsList, SelectionToolbox, QueueOverlayExpanded, PackVersionSelectorPopover from VTU to VTL - **`wrapper.vm` elimination**: 13 instances across 4 files (5 in SignInForm, 3 in CurrentUserButton, 3 in PackVersionSelectorPopover, 2 in BaseThumbnail) replaced with user interactions or removed - **`vm.$emit()` on stubs**: Interactive stubs with `setup(_, { emit })` expose buttons or closure-based emit functions (QueueOverlayExpanded, NodeSearchBoxPopover, JobAssetsList) - **Removed**: 6 change-detector/redundant tests, 3 `@ts-expect-error` annotations, `PackVersionSelectorVM` interface, `getVM` helper - **BaseThumbnail**: Removed `useEventListener` mock — real event handler attaches, `fireEvent.error(img)` triggers error state ## Review Focus - Interactive stub patterns: `JobAssetsListStub` and `NodeSearchBoxStub` use closure-based emit functions to trigger parent event handlers without `vm.$emit` - SignInForm form submission test fills PrimeVue Form fields via `userEvent.type` and submits via button click (replaces `vm.onSubmit()` direct call) - CurrentUserButton Popover stub tracks open/close state reactively - JobAssetsList: file-level `eslint-disable` for `no-container`/`no-node-access`/`prefer-user-event` since stubs lack ARIA roles and hover tests need `fireEvent` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10493-test-migrate-8-hard-case-component-tests-from-VTU-to-VTL-Phase-3-32e6d73d365081f88097df634606d7e3) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
jaeone94
|
66daa6d645 |
refactor: error system cleanup — store separation, DDD fix, test improvements (#10302)
## Summary Refactors the error system to improve separation of concerns, fix DDD layer violations, and address code quality issues. - Extract `missingNodesErrorStore` from `executionErrorStore`, removing the delegation pattern that coupled missing-node logic into the execution error store - Extract `useNodeErrorFlagSync` composable for node error flag reconciliation (previously inlined) - Extract `useErrorClearingHooks` composable with explicit callback cleanup on node removal - Extract `useErrorActions` composable to deduplicate telemetry+command patterns across error card components - Move `getCnrIdFromNode`/`getCnrIdFromProperties` to `platform/nodeReplacement` layer (DDD fix) - Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD alignment) - Add unmount cancellation guard to `useErrorReport` async `onMounted` - Return watch stop handle from `useNodeErrorFlagSync` - Add `asyncResolvedIds` eviction on `missingNodesError` reset - Add `console.warn` to silent catch blocks and empty array guard - Hoist `useCommandStore` to setup scope, fix floating promises - Add `data-testid` to error groups, image/video error spans, copy button - Update E2E tests to use scoped locators and testids - Add unit tests for `onNodeRemoved` restoration and double-install guard Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085 ## Test plan - [x] Existing unit tests pass with updated imports and mocks - [x] New unit tests for `useErrorClearingHooks` (callback restoration, double-install guard) - [x] E2E tests updated to use scoped locators and `data-testid` - [ ] Manual: verify error tab shows runtime errors and missing nodes correctly - [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons work in error cards ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Jin Yi
|
7c6ab19484 |
fix: manager progress toast and install button UX issues (#10423)
## Changes - Reset `isRestartCompleted` in `closeToast()` so the "Apply Changes" button appears correctly on subsequent installs instead of skipping to the success message - Add `@click.stop` on `PackInstallButton` to prevent click from bubbling up to card selection, which was unintentionally opening the right info panel ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10423-fix-manager-progress-toast-and-install-button-UX-issues-32d6d73d365081eba2c6d5b9b35ab26e) by [Unito](https://www.unito.io) |
||
|
Dante
|
cdf74c36f7 |
tool: add layer architecture boundary lint rule (#10109)
## Summary - Add `import-x/no-restricted-paths` ESLint rule enforcing the `base → platform → workbench → renderer` layer hierarchy - Set to `error` with eslint-disable comments on existing violations (~11 suppressions) - Consolidate zone definitions using array syntax for `from`/`target` - Add `layer-audit` Claude skill for auditing violations - Fix knip false positive for `zod` dependency in ingest-types ## Context Ref: https://github.com/Comfy-Org/ComfyUI_frontend/pull/10021#discussion_r2939392141 The codebase is migrating toward a layered architecture. This adds static enforcement so new violations are caught in PR CI. ### Layer rules | Layer | Can import from | |---|---| | `base/` | nothing | | `platform/` | `base/` | | `workbench/` | `platform/`, `base/` | | `renderer/` | `workbench/`, `platform/`, `base/` | ### Current violations (pre-existing, suppressed with eslint-disable) | Direction | Count | |---|---| | base → platform | 2 | | platform → workbench | 3 | | platform → renderer | 5 | | workbench → renderer | 1 | ## Test plan - [x] `pnpm lint` passes (0 errors, 0 warnings) - [x] `pnpm typecheck` passes - [x] `pnpm knip` passes - [ ] CI green --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> |
||
|
Jin Yi
|
b61029b9da |
fix: show correct empty state on Missing tab instead of misleading registry error (#9640)
## Summary Show tab-specific empty state messages instead of a misleading registry connection error on tabs that don't depend on the Manager API. ## Changes - **What**: Scoped `comfyManagerStore.error` to only affect tabs that actually use the Manager API (AllInstalled, UpdateAvailable, Conflicting). Missing and Workflow tabs use the registry directly, so the Manager API error is irrelevant to them. Previously, any prior Manager API failure would cause `"Error connecting to the Comfy Node Registry"` to appear on the Missing tab even when there are simply no missing nodes. ## Review Focus The `isManagerErrorRelevant` computed only shows the error for Manager API-dependent tabs. Verify that AllInstalled/UpdateAvailable/Conflicting still correctly show the error when Manager API fails. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9640-fix-show-correct-empty-state-on-Missing-tab-instead-of-misleading-registry-error-31e6d73d365081868da1d226a9bb5fdc) by [Unito](https://www.unito.io) |
||
|
Jin Yi
|
a786825093 |
feat: replace PrimeVue AutoComplete with SearchAutocomplete in ManagerDialog (#9645)
## Summary Replace legacy PrimeVue `AutoCompletePlus` with a new `SearchAutocomplete` component built on Reka UI, matching the `SearchInput` design system. ## Changes - **What**: Add `SearchAutocomplete` component extending `SearchInput` with dropdown suggestions, IME composition handling, and generic typed `optionLabel` support. Replace `AutoCompletePlus` usage in `ManagerDialog`. - **Dependencies**: None (uses existing Reka UI Combobox primitives) ## Review Focus - `SearchAutocomplete` feature parity with the replaced `AutoCompletePlus` (suggestions, option selection, IME handling) - Dropdown styling and positioning via Reka UI `ComboboxContent` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9645-feat-replace-PrimeVue-AutoComplete-with-SearchAutocomplete-in-ManagerDialog-31e6d73d36508117ba0bef3d30dd0863) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Jin Yi
|
f97c38e6ee | fix: detect missing nodes when registry API fails to resolve packs (#9697) | ||
|
Jin Yi
|
245f840e7c |
fix: prevent HoneyToast from collapsing to minimum width in collapsed state (#9701)
## Summary Fix HoneyToast footer content (text + buttons) being squished when collapsed due to `sm:w-min` on the outer container. ## Changes - **What**: Replace `sm:w-min sm:min-w-0` with state-aware sizing (`sm:w-fit` when collapsed, `sm:w-[max(400px,40vw)]` when expanded) on the HoneyToast container. Add `gap-4` between footer text and buttons in ManagerProgressToast, `min-w-0` on text area, and `shrink-0` on button area to prevent overlap. ## Review Focus - HoneyToast is used in 3 places: ManagerProgressToast, ModelImportProgressDialog, AssetExportProgressDialog. The change moves width control from the inner content div to the outer container, which should have no negative impact on the other two consumers. ## As-is <img width="481" height="146" alt="스크린샷 2026-03-10 오전 10 40 52" src="https://github.com/user-attachments/assets/b5e12e20-23ea-4f11-9778-ad4e6c10a425" /> ## To-be <img width="506" height="62" alt="스크린샷 2026-03-10 오후 1 46 18" src="https://github.com/user-attachments/assets/f2b7963d-eedb-4885-bc57-f8b377962e92" /> <img width="630" height="83" alt="스크린샷 2026-03-10 오후 1 46 10" src="https://github.com/user-attachments/assets/e9c35f1c-3441-4fb2-8fa4-f66b7d53b3e5" /> <img width="683" height="103" alt="스크린샷 2026-03-10 오후 1 46 02" src="https://github.com/user-attachments/assets/afb94a16-cfba-4da9-8676-35f4d3133b57" /> |
||
|
Christian Byrne
|
725a0a2b89 |
fix: remove timeouts from error toasts so they persist until dismissed (#9543)
## Summary
Remove `life` (timeout) property from all error-severity toast calls so
they persist until manually dismissed, preventing users from missing
important error messages.
## Changes
- **What**: Removed `life` property from 86 error toast calls across 46
files. Error toasts now use PrimeVue's default behavior (no
auto-dismiss). Non-error toasts (success, warn, info) are unchanged.
- Also fixed a pre-existing lint issue in `TaskListPanel.vue` (`import {
t } from '@/i18n'` → `useI18n()`)
## Review Focus
- One conditional toast in `useMediaAssetActions.ts` intentionally keeps
`life` because its severity alternates between `warn` and `error`
Fixes
https://www.notion.so/comfy-org/Implement-Remove-timeouts-for-all-error-toasts-31b6d73d365081cead54fddc77ae7c3d
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9543-fix-remove-timeouts-from-error-toasts-so-they-persist-until-dismissed-31c6d73d365081fa8d30f6366e9bfe38)
by [Unito](https://www.unito.io)
|
||
|
Benjamin Lu
|
1ada6dbfc6 |
fix: align run controls with queue modal design (#9134)
## Summary - move queue batch controls to the left of the run button - align run control styling to the Figma queue modal spec using PrimeVue PT/Tailwind (secondary background on batch + dropdown, primary run button) - normalize control heights to match actionbar buttons and tighten dropdown hit area - update run typography/spacing and replace all three chevrons (dropdown + batch up/down) with the requested SVG Design: https://www.figma.com/design/LVilZgHGk5RwWOkVN6yCEK/Queue-Progress-Modal?node-id=3845-23904&m=dev <img width="303" height="122" alt="image" src="https://github.com/user-attachments/assets/4ed80ee7-3ceb-4512-96ce-f55ec6da835e" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9134-fix-align-run-controls-with-queue-modal-design-3106d73d36508160afcedbcfe4b98291) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
ef4e4a69d5 |
fix: enable enforce-consistent-class-order tailwind lint rule (#9428)
## Summary Enable `better-tailwindcss/enforce-consistent-class-order` lint rule and auto-fix all 1027 violations across 263 files. Stacked on #9427. ## Changes - **What**: Sort Tailwind classes into consistent order via `eslint --fix` - Enable `enforce-consistent-class-order` as `'error'` in eslint config - Purely cosmetic reordering — no behavioral or visual changes ## Review Focus Mechanical auto-fix PR — all changes are class reordering only. This is the largest diff but lowest risk since it changes no class names, only their order. **Stack:** #9417 → #9427 → **this PR** Fixes #9300 (partial — 3 of 3 rules) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9428-fix-enable-enforce-consistent-class-order-tailwind-lint-rule-31a6d73d3650811c9065f5178ba3e724) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
1221756e05 |
fix: enable enforce-canonical-classes tailwind lint rule (#9427)
## Summary Enable `better-tailwindcss/enforce-canonical-classes` lint rule and auto-fix all 611 violations across 173 files. Stacked on #9417. ## Changes - **What**: Simplify Tailwind classes to canonical forms via `eslint --fix`: - `h-X w-X` → `size-X` - `overflow-x-hidden overflow-y-hidden` → `overflow-hidden` - and other canonical simplifications - Enable `enforce-canonical-classes` as `'error'` in eslint config ## Review Focus Mechanical auto-fix PR — all changes produced by `eslint --fix`. No visual or behavioral changes; canonical forms are functionally identical. **Stack:** #9417 → **this PR** → PR 3 (class order) Fixes #9300 (partial — 2 of 3 rules) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9427-fix-enable-enforce-canonical-classes-tailwind-lint-rule-31a6d73d365081a49340d7d4640ede45) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
493b1e42aa |
fix: enable no-deprecated-classes tailwind lint rule (#9417)
## Summary Enable `better-tailwindcss/no-deprecated-classes` lint rule and auto-fix all 103 violations across 65 files. First PR in a stacked series for #9300. ## Changes - **What**: Replace deprecated Tailwind v3 classes with v4 equivalents: - `rounded` → `rounded-sm` (85) - `flex-shrink-0` → `shrink-0` (16) - `flex-grow` → `grow` (2) - Enable `no-deprecated-classes` as `'error'` in eslint config - Update one test asserting on `'rounded'` class string ## Review Focus Mechanical auto-fix PR — all changes produced by `eslint --fix`. No visual or behavioral changes (Tailwind v4 aliases these classes identically). Fixes #9300 (partial — 1 of 3 rules) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9417-fix-enable-no-deprecated-classes-tailwind-lint-rule-31a6d73d3650819eaef4cf8ad84fb186) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
jaeone94
|
1c3984a178 |
feat: add node replacement UI to Errors Tab (#9253)
## Summary Adds a node replacement UI to the Errors Tab so users can swap missing nodes with compatible alternatives directly from the error panel, without opening a separate dialog. ## Changes - **What**: New `SwapNodesCard` and `SwapNodeGroupRow` components render swap groups in the Errors Tab; each group shows the missing node type, its instances (with locate buttons), and a Replace button. Added `useMissingNodeScan` composable to scan the graph for missing nodes and populate `executionErrorStore`. Added `removeMissingNodesByType()` to `executionErrorStore` so replaced nodes are pruned from the error list reactively. ## Bug Fixes Found During Implementation ### Bug 1: Replaced nodes render as empty shells until page refresh `replaceWithMapping()` directly mutates `_nodes[idx]`, bypassing the Vue rendering pipeline entirely. Because the replacement node reuses the same ID, `vueNodeData` retains the stale entry from the old placeholder (`hasErrors: true`, empty widgets/inputs). `graph.setDirtyCanvas()` only repaints the LiteGraph canvas and has no effect on Vue. **Fix**: After `replaceWithMapping()`, manually call `nodeGraph.onNodeAdded?.(newNode)` to trigger `handleNodeAdded` in `useGraphNodeManager`, which runs `extractVueNodeData(newNode)` and updates `vueNodeData` correctly. Also added a guard in `handleNodeAdded` to skip `layoutStore.createNode()` when a layout for the same ID already exists, preventing a duplicate `spatialIndex.insert()`. ### Bug 2: Missing node error list overwritten by incomplete server response Two compounding issues: (A) the server's `missing_node_type` error only reports the *first* missing node — the old handler parsed this and called `surfaceMissingNodes([singleNode])`, overwriting the full list collected at load time. (B) `queuePrompt()` calls `clearAllErrors()` before the API request; if the subsequent rescan used the stale `has_errors` flag and found nothing, the missing nodes were permanently lost. **Fix**: Created `useMissingNodeScan.ts` which scans `LiteGraph.registered_node_types` directly (not `has_errors`). The `missing_node_type` catch block in `app.ts` now calls `rescanAndSurfaceMissingNodes(this.rootGraph)` instead of parsing the server's partial response. ## Review Focus - `handleReplaceNode` removes the group from the store only when `replaceNodesInPlace` returns at least one replaced node — should we always clear, or only on full success? - `useMissingNodeScan` re-scans on every execution-error change; confirm no performance concerns for large graphs with many subgraphs. ## Screenshots https://github.com/user-attachments/assets/78310fc4-0424-4920-b369-cef60a123d50 https://github.com/user-attachments/assets/3d2fd5e1-5e85-4c20-86aa-8bf920e86987 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9253-feat-add-node-replacement-UI-to-Errors-Tab-3136d73d365081718d4ddfd628cb4449) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
80fe51bb8c |
feat: show missing node packs in Errors Tab with install support (#9213)
## Summary Surfaces missing node pack information in the Errors Tab, grouped by registry pack, with one-click install support via ComfyUI Manager. ## Changes - **What**: Errors Tab now groups missing nodes by their registry pack and shows a `MissingPackGroupRow` with pack name, node/pack counts, and an Install button that triggers Manager installation. A `MissingNodeCard` shows individual unresolvable nodes that have no associated pack. `useErrorGroups` was extended to resolve missing node types to their registry packs using the `/api/workflow/missing_nodes` endpoint. `executionErrorStore` was refactored to track missing node types separately from execution errors and expose them reactively. - **Breaking**: None ## Review Focus - `useErrorGroups.ts` — the new `resolveMissingNodePacks` logic fetches pack metadata and maps node types to pack IDs; edge cases around partial resolution (some nodes have a pack, some don't) produce both `MissingPackGroupRow` and `MissingNodeCard` entries - `executionErrorStore.ts` — the store now separates `missingNodeTypes` state from `errors`; the deferred-warnings path in `app.ts` now calls `setMissingNodeTypes` so the Errors Tab is populated even when a workflow loads without executing ## Screenshots (if applicable) https://github.com/user-attachments/assets/97f8d009-0cac-4739-8740-fd3333b5a85b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9213-feat-show-missing-node-packs-in-Errors-Tab-with-install-support-3126d73d36508197bc4bf8ebfd2125c8) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
4689581674 |
feat: enhance manager dialog with initial pack id support (#9169)
## Summary Adds `initialPackId` support to the manager dialog so callers can deep-link directly to a specific node pack — pre-filling the search query, switching to packs search mode, and auto-selecting the matching pack once results load. ## Changes - **ManagerDialog.vue**: Added `initialPackId` prop; wires it into `useRegistrySearch` (forces `packs` mode and pre-fills query) and uses VueUse `until()` to auto-select the target pack and open the right panel once `resultsWithKeys` is populated (one-shot, never re-triggers). Also fixes a latent bug where the effective initial tab (resolving the persisted tab) was not used when determining the initial search mode and query — previously `initialTab` (the raw prop) was checked directly, which would produce incorrect pre-fill when no tab prop was passed but a Missing tab was persisted. - **useManagerDialog.ts**: Threads `initialPackId` through `show()` into the dialog props - **useManagerState.ts**: Exposes `initialPackId` in `openManager` options and passes it to `managerDialog.show()`; also removes a stale fallback `show(ManagerTab.All)` call that was redundant for the legacy-only error path ### Refactor: remove `executionIdUtil.ts` and distribute its functions - **`getAncestorExecutionIds` / `getParentExecutionIds`** → moved to `src/types/nodeIdentification.ts`: both are pure `NodeExecutionId` string operations with no external dependencies, consistent with the existing `parseNodeExecutionId` / `createNodeExecutionId` helpers already in that file - **`buildSubgraphExecutionPaths`** → moved to `src/platform/workflow/validation/schemas/workflowSchema.ts`: operates entirely on `ComfyNode[]` and `SubgraphDefinition` (both defined there), and `isSubgraphDefinition` is already co-located in the same file - Tests redistributed accordingly: ancestor/parent ID tests into `nodeIdentification.test.ts`, `buildSubgraphExecutionPaths` tests into `workflowSchema.test.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9169-feat-enhance-manager-dialog-with-initial-pack-id-support-3116d73d365081f7b6a3cbfb2f2755bf) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
09989b7aff |
[refactor] Extract manager composables and execution utils (#9163)
## Summary Extracts inline logic from manager components into dedicated composables and utilities, and adds a cyclic subgraph fix. ## Changes - **`usePackInstall`**: New composable extracted from `PackInstallButton.vue` — handles conflict detection, payload construction, and `Promise.allSettled`-based batch installation - **`useApplyChanges`**: New shared composable extracted from `ManagerProgressToast.vue` — manages ComfyUI restart flow with reconnect timeout and post-reconnect refresh - **`executionIdUtil`**: New utility (`getAncestorExecutionIds`, `getParentExecutionIds`, `buildSubgraphExecutionPaths`) with unit tests; fixes infinite recursion on cyclic subgraph definitions ## Review Focus - `useApplyChanges` reconnect timeout (2 min) and setting restore logic - `buildSubgraphExecutionPaths` visited-set guard for cyclic subgraph defs ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9163-refactor-Extract-manager-composables-and-execution-utils-3116d73d365081f293d3d5484775ad48) by [Unito](https://www.unito.io) |
||
|
Johnpaul Chiwetelu
|
724827d8cc |
refactor: replace withDefaults with Vue 3.5 props destructuring (#9150)
## Summary - Replace all `withDefaults(defineProps<...>())` with Vue 3.5 reactive props destructuring across 14 components - Update `props.xxx` references to use destructured variables directly in script and template - Fixes #2334 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9150-refactor-replace-withDefaults-with-Vue-3-5-props-destructuring-3116d73d365081e7a721db3369600671) by [Unito](https://www.unito.io) |
||
|
Johnpaul Chiwetelu
|
02e926471f |
fix: replace as-unknown-as casts with safer patterns (#9107)
## Summary - Replace 83 `as unknown as` double casts with safer alternatives across 33 files - Use `as Partial<X> as X` pattern where TypeScript allows it - Create/reuse factory functions from `litegraphTestUtils.ts` for mock objects - Widen `getWorkflowDataFromFile` return type to include `ComfyMetadata` directly - Reduce total `as unknown as` count from ~153 to 71 The remaining 71 occurrences are genuinely necessary due to cross-schema casts, generic variance, missing index signatures, Float64Array-to-tuple conversions, and DOM type incompatibilities. ## Test plan - [x] `pnpm typecheck` passes - [x] `pnpm lint` passes - [x] All affected unit tests pass ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9107-fix-replace-as-unknown-as-casts-with-safer-patterns-3106d73d3650815cb5bcd613ad635bd7) by [Unito](https://www.unito.io) |
||
|
Jin Yi
|
44733f010d |
[refactor] Unify small modal dialog styles with showSmallLayoutDialog (#8834)
## Summary Extract a shared `showSmallLayoutDialog` utility and move dialog-specific logic into composables, unifying the duplicated `pt` configurations across small modal dialogs. ## Changes - **`showSmallLayoutDialog`**: Added to `dialogService.ts` with a single unified `pt` config for all small modal dialogs (missing nodes, missing models, import failed, node conflict) - **Composables**: Extracted 4 dialog functions from `dialogService` into dedicated composables following the `useSettingsDialog` / `useModelSelectorDialog` pattern: - `useMissingNodesDialog` - `useMissingModelsDialog` - `useImportFailedNodeDialog` - `useNodeConflictDialog` - Each composable uses direct imports, synchronous `show()`, `hide()`, and a `DIALOG_KEY` constant - Updated all call sites (`app.ts`, `useHelpCenter`, `PackEnableToggle`, `PackInstallButton`, `useImportFailedDetection`) ## Review Focus - Unified `pt` config removes minor style variations between dialogs — intentional design unification ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8834-refactor-Unify-small-modal-dialog-styles-with-showSmallLayoutDialog-3056d73d365081b6963beffc0e5943bf) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Jin Yi
|
553ea63357 | [refactor] Migrate SettingDialog to BaseModalLayout design system (#8270) | ||
|
Johnpaul Chiwetelu
|
4fc1d2ef5b |
feat: No Explicit Any (#8601)
## Summary - Add `typescript/no-explicit-any` rule to `.oxlintrc.json` to enforce no explicit `any` types - Fix all 40 instances of explicit `any` throughout the codebase - Improve type safety with proper TypeScript types ## Changes Made ### Configuration - Added `typescript/no-explicit-any` rule to `.oxlintrc.json` ### Type Fixes - Replaced `any` with `unknown` for truly unknown types - Updated generic type parameters to use `unknown` defaults instead of `any` - Fixed method `this` parameters to avoid variance issues - Updated component props to match new generic types - Fixed test mocks to use proper type assertions ### Key Files Modified - `src/types/treeExplorerTypes.ts`: Updated TreeExplorerNode interface generics - `src/platform/settings/types.ts`: Fixed SettingParams generic default - `src/lib/litegraph/src/LGraph.ts`: Fixed ParamsArray type constraint - `src/extensions/core/electronAdapter.ts`: Fixed onChange callbacks - `src/views/GraphView.vue`: Added proper type imports - Multiple test files: Fixed type assertions and mocks ## Test Plan - [x] All lint checks pass (`pnpm lint`) - [x] TypeScript compilation succeeds (`pnpm typecheck`) - [x] Pre-commit hooks pass - [x] No regression in functionality ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8601-feat-add-typescript-no-explicit-any-rule-and-fix-all-instances-2fd6d73d365081fd9beef75d5a6daf5b) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Csongor Czezar
|
422227d2fc |
fix: viewport overflow in manager (#7775)
### **Summary** Fixes viewport overflow in version selector dropdown when Manager dialog is positioned near edges **Changes** Removed popover arrow for visual consistency across all positions Implemented dialog boundary detection to constrain popover within Manager viewport **Testing** All existing unit tests pass (17/17) Visually tested across different screen positions    ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7775-fix-viewport-overflow-in-manager-2d76d73d365081a88c1df2a103c5925e) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
69c8c84aef |
fix: resolve i18n no-restricted-imports lint warnings (#8704)
## Summary
Fix all i18n `no-restricted-imports` lint warnings and upgrade rules
from `warn` to `error`.
## Changes
- **What**: Migrate Vue components from `import { t/d } from '@/i18n'`
to `const { t } = useI18n()`. Migrate non-component `.ts` files from
`useI18n()` to `import { t/d } from '@/i18n'`. Allow `st` import from
`@/i18n` in Vue components (it wraps `te`/`t` for safe fallback
translation). Remove `@deprecated` tag from `i18n.ts` global exports
(still used by `st` and non-component code). Upgrade both lint rules
from `warn` to `error`.
## Review Focus
- The `st` helper is intentionally excluded from the Vue component
restriction since it provides safe fallback translation needed for
custom node definitions.
Fixes #8701
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8704-fix-resolve-i18n-no-restricted-imports-lint-warnings-2ff6d73d365081ae84d8eb0dfef24323)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Christian Byrne
|
478cfc0b5e |
feat: replace puzzle icon with extensions-blocks icon for manager button (#8644)
## Summary Replace the manager button puzzle icon with a custom extensions-blocks SVG icon and add a "Manage Extensions" text label to the top bar button. ## Changes - **What**: Swap `icon-[lucide--puzzle]` → `icon-[comfy--extensions-blocks]` in TopMenuSection, ComfyMenuButton, and ManagerDialog. Add visible "Manage Extensions" label (hidden below md). Align tooltip with new label text. ## Review Focus - Visual appearance of the new icon at different sizes - Button layout with text label at md+ breakpoints - Red dot notification positioning with wider button Fixes COM-12161 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8644-feat-replace-puzzle-icon-with-extensions-blocks-icon-for-manager-button-2fe6d73d3650815c8867efc5a0842ef7) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Johnpaul Chiwetelu
|
90a701dd67 |
Road to No Explicit Any Part 11 (#8565)
## Summary
This PR removes `any` types from widgets, services, stores, and test
files, replacing them with proper TypeScript types.
### Key Changes
#### Type Safety Improvements
- Replaced `any` with `unknown`, explicit types, or proper interfaces
across widgets and services
- Added proper type imports (TgpuRoot, Point, StyleValue, etc.)
- Created typed interfaces (NumericWidgetOptions, TestWindow,
ImportFailureDetail, etc.)
- Fixed function return types to be non-nullable where appropriate
- Added type guards and null checks instead of non-null assertions
- Used `ComponentProps` from vue-component-type-helpers for component
testing
#### Widget System
- Added index signature to IWidgetOptions for Record compatibility
- Centralized disabled logic in WidgetInputNumberInput
- Moved template type assertions to computed properties
- Fixed ComboWidget getOptionLabel type assertions
- Improved remote widget type handling with runtime checks
#### Services & Stores
- Fixed getOrCreateViewer to return non-nullable values
- Updated addNodeOnGraph to use specific options type `{ pos?: Point }`
- Added proper type assertions for settings store retrieval
- Fixed executionIdToCurrentId return type (string | undefined)
#### Test Infrastructure
- Exported GraphOrSubgraph from litegraph barrel to avoid circular
dependencies
- Updated test fixtures with proper TypeScript types (TestInfo,
LGraphNode)
- Replaced loose Record types with ComponentProps in tests
- Added proper error handling in WebSocket fixture
#### Code Organization
- Created shared i18n-types module for locale data types
- Made ImportFailureDetail non-exported (internal use only)
- Added @public JSDoc tag to ElectronWindow type
- Fixed console.log usage in scripts to use allowed methods
### Files Changed
**Widgets & Components:**
-
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
- src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue
-
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
- src/lib/litegraph/src/widgets/ComboWidget.ts
- src/lib/litegraph/src/types/widgets.ts
- src/components/common/LazyImage.vue
- src/components/load3d/Load3dViewerContent.vue
**Services & Stores:**
- src/services/litegraphService.ts
- src/services/load3dService.ts
- src/services/colorPaletteService.ts
- src/stores/maskEditorStore.ts
- src/stores/nodeDefStore.ts
- src/platform/settings/settingStore.ts
- src/platform/workflow/management/stores/workflowStore.ts
**Composables & Utils:**
- src/composables/node/useWatchWidget.ts
- src/composables/useCanvasDrop.ts
- src/utils/widgetPropFilter.ts
- src/utils/queueDisplay.ts
- src/utils/envUtil.ts
**Test Files:**
- browser_tests/fixtures/ComfyPage.ts
- browser_tests/fixtures/ws.ts
- browser_tests/tests/actionbar.spec.ts
-
src/workbench/extensions/manager/components/manager/skeleton/PackCardGridSkeleton.test.ts
- src/lib/litegraph/src/subgraph/subgraphUtils.test.ts
- src/components/rightSidePanel/shared.test.ts
- src/platform/cloud/subscription/composables/useSubscription.test.ts
-
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts
**Scripts & Types:**
- scripts/i18n-types.ts (new shared module)
- scripts/diff-i18n.ts
- scripts/check-unused-i18n-keys.ts
- src/workbench/extensions/manager/types/conflictDetectionTypes.ts
- src/types/algoliaTypes.ts
- src/types/simplifiedWidget.ts
**Infrastructure:**
- src/lib/litegraph/src/litegraph.ts (added GraphOrSubgraph export)
- src/lib/litegraph/src/infrastructure/CustomEventTarget.ts
- src/platform/assets/services/assetService.ts
**Stories:**
- apps/desktop-ui/src/views/InstallView.stories.ts
- src/components/queue/job/JobDetailsPopover.stories.ts
**Extension Manager:**
- src/workbench/extensions/manager/composables/useConflictDetection.ts
- src/workbench/extensions/manager/composables/useManagerQueue.ts
- src/workbench/extensions/manager/services/comfyManagerService.ts
- src/workbench/extensions/manager/utils/conflictMessageUtil.ts
### Testing
- [x] All TypeScript type checking passes (`pnpm typecheck`)
- [x] ESLint passes without errors (`pnpm lint`)
- [x] Format checks pass (`pnpm format:check`)
- [x] Knip (unused exports) passes (`pnpm knip`)
- [x] Pre-commit and pre-push hooks pass
Part of the "Road to No Explicit Any" initiative.
### Previous PRs in this series:
- Part 2: #7401
- Part 3: #7935
- Part 4: #7970
- Part 5: #8064
- Part 6: #8083
- Part 7: #8092
- Part 8 Group 1: #8253
- Part 8 Group 2: #8258
- Part 8 Group 3: #8304
- Part 8 Group 4: #8314
- Part 8 Group 5: #8329
- Part 8 Group 6: #8344
- Part 8 Group 7: #8459
- Part 8 Group 8: #8496
- Part 9: #8498
- Part 10: #8499
---------
Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com>
Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
|
||
|
Christian Byrne
|
278d491030 |
refactor: move ellipsis and punctuation into i18n translation strings (#8573)
## Summary
Move ellipsis and punctuation characters into i18n translation strings
for proper internationalization support.
## Changes
- Add 12 new translation keys with punctuation included:
- Placeholder keys with trailing ellipsis (e.g.,
`searchNodesPlaceholder: "Search Nodes..."`)
- `downloadWithSize` with interpolation: `"Download ({size})"`
- `completedWithCheckmark`: `"Completed ✓"`
- Prompt keys with colons (e.g., `enterNewNamePrompt: "Enter new
name:"`)
- Update 20 files to use new translation keys instead of string
concatenation
## Review Focus
This eliminates string concatenation patterns like `$t('key') + '...'`
that break proper internationalization, since different languages may
use different punctuation or may not need ellipsis/colons in the same
contexts.
Fixes #7333
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Standardized localization across the app: unified search placeholders
and input hints; updated dialog prompt texts for renaming,
saving/exporting, and related prompts.
* **New Features**
* Download buttons now show file size via localized text.
* Completed status displays a localized label with a checkmark.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8573-refactor-move-ellipsis-and-punctuation-into-i18n-translation-strings-2fc6d73d365081828ad3f257bcac7799)
by [Unito](https://www.unito.io)
|
||
|
Jin Yi
|
eb14a2947f |
style: scale down node previews in manager info panel (#8579)
## Summary Scale down node previews in the manager info panel to 75% for better visual fit. ## Changes - Add 75% scale to node preview items in NodesTabPanel - Add gap between scaled items for better spacing <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Style** * Enhanced the visual layout of node items in the manager panel with improved spacing and individual item scaling for better visual presentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8579-style-scale-down-node-previews-in-manager-info-panel-2fc6d73d36508176a307d980cd3b9329) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
cbdc7d030f |
feat: code splitting optimization - reduce initial bundle by 36% (#8542)
## Summary Reduces initial module preload from 12.94 MB to 8.24 MB (-4.7 MB, -36%). ## Changes - Split vendor chunks for better cache isolation (firebase, sentry, i18n, zod, etc.) - Exclude heavy optional chunks from initial preload: THREE.js, xterm, tiptap, chart.js, yjs - Lazy load 16 dialog components in dialogService - Add \ endor-yjs\ chunk for CRDT library ## Metrics | Metric | Before | After | |--------|--------|-------| | Initial preload | 12.94 MB | 8.24 MB | | vendor-other | 4,006 KB | 2,156 KB | | dialogService | 1,860 KB | 1,304 KB | All excluded chunks still load on-demand when their features are used. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8542-feat-code-splitting-optimization-reduce-initial-bundle-by-36-2fb6d73d36508146aaf7fdaed3274033) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Alexander Brown
|
21492ecca5 |
fix: update imports for VueUse v14 compatibility (#8550)
Update imports for VueUse v14 compatibility: - `toValue` → import from `vue` (dropped from VueUse v14) - `MaybeRef` type → import from `vue` (dropped from VueUse v14) These APIs were deprecated in VueUse v12 and removed in v14 in favor of Vue's native exports. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8550-fix-update-imports-for-VueUse-v14-compatibility-2fb6d73d365081b0bac1d01d4092c176) by [Unito](https://www.unito.io) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated VueUse dependencies to newer versions for improved compatibility * Minor package entry reorganization (no functional changes) * **Refactor** * Consolidated imports to use native Vue utilities where applicable * **Tests** * Updated unit tests: improved outside-click simulation and cleanup; migrated tests to use the real store setup for more realistic test behavior <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Johnpaul Chiwetelu
|
2f8bd7b04f |
Road to No Explicit Any Part 9 (#8498)
## Summary
This PR removes `any` types from core source files and replaces them
with proper TypeScript types.
### Key Changes
#### Type Safety Improvements
- Replaced `any` with `unknown`, explicit types, or proper interfaces
across core files
- Introduced new type definitions: `SceneConfig`, `Load3DNode`,
`ElectronWindow`
- Used `Object.assign` instead of `as any` for dynamic property
assignment
- Replaced `as any` casts with proper type assertions
### Files Changed
Source files:
- src/extensions/core/widgetInputs.ts - Removed unnecessary `as any`
cast
- src/platform/cloud/onboarding/auth.ts - Used `Record<string, unknown>`
and Sentry types
- src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts -
Used `AuditLog[]` type
- src/platform/workflow/management/stores/workflowStore.ts - Used
`typeof ComfyWorkflow` constructor type
- src/scripts/app.ts - Used `ResultItem[]` for Clipspace images
- src/services/colorPaletteService.ts - Used `Object.assign` instead of
`as any`
- src/services/customerEventsService.ts - Used `unknown` instead of
`any`
- src/services/load3dService.ts - Added proper interface types for
Load3D nodes
- src/types/litegraph-augmentation.d.ts - Used `TWidgetValue[]` type
- src/utils/envUtil.ts - Added ElectronWindow interface
- src/workbench/extensions/manager/stores/comfyManagerStore.ts - Typed
event as `CustomEvent<{ ui_id?: string }>`
### Testing
- All TypeScript type checking passes (`pnpm typecheck`)
- Linting passes without errors (`pnpm lint`)
- Code formatting applied (`pnpm format`)
Part of the "Road to No Explicit Any" initiative.
### Previous PRs in this series:
- Part 2: #7401
- Part 3: #7935
- Part 4: #7970
- Part 5: #8064
- Part 6: #8083
- Part 7: #8092
- Part 8 Group 1: #8253
- Part 8 Group 2: #8258
- Part 8 Group 3: #8304
- Part 8 Group 4: #8314
- Part 8 Group 5: #8329
- Part 8 Group 6: #8344
- Part 8 Group 7: #8459
- Part 8 Group 8: #8496
- Part 9: #8498 (this PR)
|
||
|
Johnpaul Chiwetelu
|
cfdd002b7c |
Road to No Explicit Any Part 10 (#8499)
## Summary This PR removes `any` types from UI component files and replaces them with proper TypeScript types. ### Key Changes #### Type Safety Improvements - Replaced `any` with `unknown`, explicit types, or proper interfaces across UI components - Used `ComponentPublicInstance` with explicit method signatures for component refs - Used `Record<string, unknown>` for dynamic property access - Added generics for form components with flexible value types - Used `CSSProperties` for style objects ### Files Changed UI Components: - src/components/common/ComfyImage.vue - Used proper class prop type - src/components/common/DeviceInfo.vue - Used `string | number` for formatValue - src/components/common/FormItem.vue - Used `unknown` for model value - src/components/common/FormRadioGroup.vue - Added generic type parameter - src/components/common/TreeExplorer.vue - Used proper async function signature - src/components/custom/widget/WorkflowTemplateSelectorDialog.vue - Fixed duplicate import - src/components/graph/CanvasModeSelector.vue - Used `ComponentPublicInstance` for ref - src/components/node/NodePreview.vue - Changed `any` to `unknown` - src/components/queue/job/JobDetailsPopover.vue - Removed unnecessary casts - src/components/queue/job/JobFiltersBar.vue - Removed `as any` casts - src/platform/assets/components/MediaAssetContextMenu.vue - Added `ContextMenuInstance` type - src/renderer/extensions/minimap/MiniMapPanel.vue - Used `CSSProperties` - src/renderer/extensions/vueNodes/composables/useNodeTooltips.ts - Added `PrimeVueTooltipElement` interface - src/renderer/extensions/vueNodes/widgets/components/form/FormSelectButton.vue - Used `Record<string, unknown>` - src/workbench/extensions/manager/components/manager/infoPanel/tabs/DescriptionTabPanel.vue - Added `LicenseObject` interface ### Testing - All TypeScript type checking passes (`pnpm typecheck`) - Linting passes without errors (`pnpm lint`) Part of the "Road to No Explicit Any" initiative. ### Previous PRs in this series: - Part 2: #7401 - Part 3: #7935 - Part 4: #7970 - Part 5: #8064 - Part 6: #8083 - Part 7: #8092 - Part 8 Group 1: #8253 - Part 8 Group 2: #8258 - Part 8 Group 3: #8304 - Part 8 Group 4: #8314 - Part 8 Group 5: #8329 - Part 8 Group 6: #8344 - Part 8 Group 7: #8459 - Part 8 Group 8: #8496 - Part 9: #8498 - Part 10: #8499 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8499-Road-to-No-Explicit-Any-Part-10-2f86d73d365081aab129f165c7d02434) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
2ec954459c |
fix: show Missing Nodes dialog for missing node prompt errors (#8511)
## Summary When a workflow is queued with a missing node type, show the Missing Nodes dialog instead of a generic error toast. This gives users actionable options like "Open Manager" and "Install All". ## Changes - Detect `missing_node_type` error from backend in `queuePrompt()` catch block - Construct `MissingNodeType` object with class type and contextual hint - Reuse existing `showMissingNodesError()` to trigger the dialog ## Dependencies ⚠️ **Requires backend PR:** https://github.com/Comfy-Org/ComfyUI/pull/12177 The backend PR changes the error type from `invalid_prompt` to `missing_node_type` and adds `extra_info` with node details. ## Related - Fixes COM-12528 - Addresses ~49 GitHub issues with confusing "missing class_type" errors ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8511-fix-show-Missing-Nodes-dialog-for-missing-node-prompt-errors-2f96d73d3650812b95f0f08e51abaabb) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Christian Byrne
|
d34d6a8a92 |
feat: show legacy manager search tip in no-results empty state (#8537)
## Summary
Show a non-intrusive tip in the no-results empty state when users search
legacy manager-related terms in the new manager UI.
## Changes
- **What**: Add `useLegacySearchTip` composable that detects when search
query matches legacy manager keywords ("manager", "comfyui-manager",
etc.) and appends a tip to the empty state message suggesting the
`--enable-manager-legacy-ui` flag
- **Integration**: Tip appears as secondary muted text below "Try a
different search" message when no results found
- **Tests**: 9 test cases covering detection logic and edge cases
## Review Focus
- Keyword list covers common legacy manager search terms
- Non-intrusive approach: tip only shows in no-results state, no dismiss
button needed
Fixes COM-12509
|
||
|
Johnpaul Chiwetelu
|
a64c561a5f |
Road to No explicit any: Group 8 (part 8) test files (#8496)
## Summary
This PR removes unsafe type assertions ("as unknown as Type") from test
files and improves type safety across the codebase.
### Key Changes
#### Type Safety Improvements
- Removed improper `as unknown as Type` patterns from test files in
Group 8 part 8
- Replaced with proper TypeScript patterns using Pinia store testing
patterns
- Fixed parameter shadowing issue in typeGuardUtil.test.ts (constructor
→ nodeConstructor)
- Fixed stale mock values in useConflictDetection.test.ts using getter
functions
- Refactored useManagerState tests to follow proper Pinia store testing
patterns with createTestingPinia
### Files Changed
Test files (Group 8 part 8 - utils and manager composables):
- src/utils/typeGuardUtil.test.ts - Fixed parameter shadowing
- src/utils/graphTraversalUtil.test.ts - Removed unsafe type assertions
- src/utils/litegraphUtil.test.ts - Improved type handling
- src/workbench/extensions/manager/composables/useManagerState.test.ts -
Complete rewrite using Pinia testing patterns
-
src/workbench/extensions/manager/composables/useConflictDetection.test.ts
- Fixed stale mock values with getters
- src/workbench/extensions/manager/composables/useManagerQueue.test.ts -
Type safety improvements
-
src/workbench/extensions/manager/composables/nodePack/useMissingNodes.test.ts
- Removed unsafe casts
-
src/workbench/extensions/manager/composables/nodePack/usePacksSelection.test.ts
- Type improvements
-
src/workbench/extensions/manager/composables/nodePack/usePacksStatus.test.ts
- Type improvements
- src/workbench/extensions/manager/utils/versionUtil.test.ts - Type
safety fixes
Source files (minor type fixes):
- src/utils/fuseUtil.ts - Type improvements
- src/utils/linkFixer.ts - Type safety fixes
- src/utils/syncUtil.ts - Type improvements
-
src/workbench/extensions/manager/composables/nodePack/useWorkflowPacks.ts
- Type fix
-
src/workbench/extensions/manager/composables/useConflictAcknowledgment.ts
- Type fix
### Testing
- All TypeScript type checking passes (`pnpm typecheck`)
- All affected test files pass (`pnpm test:unit`)
- Linting passes without errors (`pnpm lint`)
- Code formatting applied (`pnpm format`)
Part of the "Road to No Explicit Any" initiative, cleaning up type
casting issues from branch `fix/remove-any-types-part8`.
### Previous PRs in this series:
- Part 2: #7401
- Part 3: #7935
- Part 4: #7970
- Part 5: #8064
- Part 6: #8083
- Part 7: #8092
- Part 8 Group 1: #8253
- Part 8 Group 2: #8258
- Part 8 Group 3: #8304
- Part 8 Group 4: #8314
- Part 8 Group 5: #8329
- Part 8 Group 6: #8344
- Part 8 Group 7: #8459
- Part 8 Group 8: #8496 (this PR)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8496-Road-to-No-explicit-any-Group-8-part-8-test-files-2f86d73d365081f3afdcf8d01fba81e1)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
|
||
|
Jin Yi
|
d7654baebf |
[feat] Show context-appropriate empty state messages in Manager tabs (#8415)
## Summary Shows tab-specific empty state messages in Node Manager instead of generic "No search results found" message. ## Changes - Added computed properties to determine empty state messages based on current tab and search state - Display tab-specific messages when a tab is empty without active search (e.g., "No Missing Nodes" for Missing tab) - Fall back to search-related messages only when there's an active search query - Added Korean translations for empty state messages | Tab | Empty State Title | |-----|-------------------| | All Installed | No Extensions Installed | | Update Available | All Up to Date | | Conflicting | No Conflicts Detected | | Workflow | No Extensions in Workflow | | Missing | No Missing Nodes | ## Review Focus - Verify i18n key structure matches existing patterns ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8415-feat-Show-context-appropriate-empty-state-messages-in-Manager-tabs-2f76d73d3650817ab8a0d41b45df3411) by [Unito](https://www.unito.io) |