Comfy-Org / Comfy-Org/ComfyUI_frontend
Audit: Replace plain `string` node ID typings with shared `NodeId` type (pre-work for adr008)
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
## Summary
This is an audit/cleanup task to replace bare `string` (and Vue prop `String`) typings for node IDs throughout the codebase with the appropriate shared `NodeId` type. This will make it significantly easier to implement adr008 (centralized stable identification) later, because a single type change will propagate everywhere rather than requiring a grep-and-replace across 150+ sites.
## Background
Identified during code review on PR #11423 (comment: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11423#discussion_r3107604798).
Requested by: @christian-byrne
## Current State — Three competing `NodeId` definitions
There are currently **three** separate `NodeId` type aliases in the codebase, which leads to inconsistent imports and silent type mismatches:
| Source | Definition |
|---|---|
| `src/lib/litegraph/src/LGraphNode.ts` | `type NodeId = number \| string` |
| `src/platform/workflow/validation/schemas/workflowSchema.ts` | `type NodeId = z.infer` → `number \| string` |
| `src/renderer/core/layout/types.ts` | `type NodeId = string` |
Files already importing a shared `NodeId` do so inconsistently (some from litegraph, some from workflowSchema, some from layout/types).
## Scope
A quick search shows **~151 sites** where `nodeId` is typed as a bare `string` or Vue prop `String` in `.ts` files alone (not counting `.vue` SFCs). Examples include:
- Vue component `nodeId` props typed as `String` instead of the shared type
- Function parameters typed as `string` for node IDs
- Store state/getters using `string` for node IDs
- Composable arguments typed as `string` for node IDs
## Suggested Steps
1. **Decide on a single canonical `NodeId` type** (or a small layered set, e.g., `LGraphNodeId` vs `RendererNodeId`) and document it — ideally as part of or alongside adr008.
2. **Consolidate** the three existing definitions into the canonical location and re-export from the others for backward compatibility during migration.
3. **Replace** all bare `string`/`String` node-ID typings with the canonical type.
4. **Update Vue component props** (`nodeId: { type: String }`) to use the canonical type with a runtime validator or typed prop where feasible.
5. **CI guard** — consider adding a lint rule or type alias to prevent regressions.
## References
- PR #11423 — fix: include actual slot index in InputSlot/OutputSlot keys (where this was first noticed)
- adr008 — centralized stable identification (future ADR)
┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-11428-Audit-Replace-plain-string-node-ID-typings-with-shared-NodeId-type-pre-work-for-3476d73d36508108ae66cbf2eb2c80f4) by [Unito](https://www.unito.io)
Contributor guide
Assessment
This issue has not been assessed yet.