Comfy-Org / Comfy-Org/ComfyUI_frontend

nodeApi: decompose defsRegistry.applyTo(), now 693 lines

Open
#16,853 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

Follow-up owned from review of #16790 (thread on `defsRegistry.ts`).

## Context

`applyTo()` is the function that walks every registration matching a node definition, collects the bound callbacks, and installs them onto the node class prototype. The review flagged it at 548 lines. It is now **693** — the review fixes themselves added to it, which is the argument for the finding rather than against it.

Everything a pack can declare passes through this one function: widgets, hidden widgets, 14 lifecycle callbacks, menu items, serializers, prompt projectors, connection vetoes, frontend-execution resolvers. Each is accumulated into its own local array, then installed in a second pass, with re-entrancy guards and legacy-result preservation threaded between them.

## Decision

Ticket rather than fix in #16790. It is a pure refactor of the single riskiest function in a 20k-line change; doing it in the PR under review would make the diff unreadable and put the extension ecosystem's behaviour at risk with no test-visible benefit. The coverage to refactor against already exists — `defsRegistry.test.ts` is ~1,900 lines.

## Done looks like

- Collection separated from installation: gathering bound callbacks per hook is data, installing them on the prototype is behaviour, and they do not need to share a scope
- Each hook family (lifecycle, drag/drop, connections, widgets, menu, serialization, execution) extractable and testable on its own
- The re-entrancy guard and legacy-result preservation stated once rather than per hook
- No behaviour change; the existing suite is the contract

## Risk if deferred

It grows. It gained 145 lines during one round of review fixes, and every new published callback lands here.

Contributor guide

Open the contributing guide

Research direction

Read defsRegistry.ts at applyTo() and use defsRegistry.test.ts as the behavioral contract; run the existing suite first. Separate collection from installation by hook family, state the re-entrancy guard and legacy-result preservation once, and confirm there is no behavior change with the existing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.