Comfy-Org / Comfy-Org/ComfyUI_frontend
nodeApi: decompose defsRegistry.applyTo(), now 693 lines
- 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
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