fix(widgets): tolerate un-keyable widget ids in widget value store#13773
Merged
Conversation
Custom nodes register client-side widgets with empty or placeholder names (spacer/header/preview/button). Their widget id has an empty name segment, which parseWidgetId rejects by throwing. The throw aborted BaseWidget.setNodeId seeding (legacy) and tripped the Vue NodeWidgets error boundary, blanking the node's entire widget grid (e.g. rgthree Power Lora Loader rendered no widgets). Guard registerWidget/getWidget/deleteWidget with isWidgetId so the store declines an un-keyable id instead of throwing; registerWidget now returns undefined for such ids and BaseWidget keeps its local widget state so the widget still functions. One un-storable widget no longer blanks its siblings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe widget value store validates widget IDs before registration, lookup, and deletion. Tests cover rejected IDs and valid siblings, while ChangesWidget ID validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🎭 Playwright: ✅ 1714 passed, 0 failed · 2 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 7.91 MB gzip 🔴 +86 BDetailsSummary
Category Glance App Entry Points — 47.3 kB (baseline 47.3 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.25 MB (baseline 1.25 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 109 kB (baseline 109 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed / 3 unchanged Panels & Settings — 550 kB (baseline 550 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 29.1 kB (baseline 29.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 121 kB (baseline 121 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 5 added / 5 removed / 1 unchanged UI Components — 56.8 kB (baseline 56.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 273 kB (baseline 273 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 3 unchanged Utilities & Hooks — 3.4 MB (baseline 3.4 MB) • 🔴 +306 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 20 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 11.9 MB (baseline 11.9 MB) • ⚪ 0 BBundles that do not match a named category
Status: 66 added / 66 removed / 97 unchanged ⚡ Performance Report
Show regressions
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-07-18T03:45:09.847Z",
"gitSha": "895e5a888b13c3fb706711b2c60357c3edcd2707",
"branch": "fix/widget-store-custom-node-widgets",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2053.6610000000055,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.195,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 368.504,
"heapDeltaBytes": 3789668,
"heapUsedBytes": 72586980,
"domNodes": 18,
"jsHeapTotalBytes": 19677184,
"scriptDurationMs": 16.776000000000003,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-idle",
"durationMs": 2030.2749999999605,
"styleRecalcs": 7,
"styleRecalcDurationMs": 7.668999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 498.62300000000005,
"heapDeltaBytes": -21258924,
"heapUsedBytes": 47535972,
"domNodes": -267,
"jsHeapTotalBytes": 20328448,
"scriptDurationMs": 29.883999999999997,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2004.5160000000237,
"styleRecalcs": 75,
"styleRecalcDurationMs": 62.809000000000005,
"layouts": 12,
"layoutDurationMs": 4.53,
"taskDurationMs": 955.4800000000001,
"heapDeltaBytes": -15114844,
"heapUsedBytes": 53747448,
"domNodes": -267,
"jsHeapTotalBytes": 20852736,
"scriptDurationMs": 127.98199999999999,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1976.4940000000024,
"styleRecalcs": 76,
"styleRecalcDurationMs": 48.318,
"layouts": 12,
"layoutDurationMs": 3.8640000000000003,
"taskDurationMs": 957.243,
"heapDeltaBytes": -13201896,
"heapUsedBytes": 55688336,
"domNodes": -264,
"jsHeapTotalBytes": 20066304,
"scriptDurationMs": 141.991,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1739.091999999971,
"styleRecalcs": 31,
"styleRecalcDurationMs": 17.154,
"layouts": 6,
"layoutDurationMs": 0.7239999999999999,
"taskDurationMs": 307.494,
"heapDeltaBytes": 7088528,
"heapUsedBytes": 75894532,
"domNodes": 79,
"jsHeapTotalBytes": 19939328,
"scriptDurationMs": 18.748,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1750.5689999999277,
"styleRecalcs": 31,
"styleRecalcDurationMs": 21.833,
"layouts": 6,
"layoutDurationMs": 0.806,
"taskDurationMs": 442.936,
"heapDeltaBytes": 7403812,
"heapUsedBytes": 76141380,
"domNodes": 77,
"jsHeapTotalBytes": 19677184,
"scriptDurationMs": 29.897999999999996,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "dom-widget-clipping",
"durationMs": 639.9780000000135,
"styleRecalcs": 12,
"styleRecalcDurationMs": 8.328000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 385.243,
"heapDeltaBytes": -11633440,
"heapUsedBytes": 57244668,
"domNodes": 20,
"jsHeapTotalBytes": 19677184,
"scriptDurationMs": 63.777999999999984,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 773.8769999999704,
"styleRecalcs": 12,
"styleRecalcDurationMs": 11.514,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 438.509,
"heapDeltaBytes": -11395688,
"heapUsedBytes": 57635504,
"domNodes": 20,
"jsHeapTotalBytes": 19939328,
"scriptDurationMs": 75.105,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2054.1279999999915,
"styleRecalcs": 9,
"styleRecalcDurationMs": 12.693,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 559.913,
"heapDeltaBytes": 5222564,
"heapUsedBytes": 64374416,
"domNodes": -266,
"jsHeapTotalBytes": 5193728,
"scriptDurationMs": 88.84500000000001,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-idle",
"durationMs": 2031.4069999999447,
"styleRecalcs": 8,
"styleRecalcDurationMs": 9.104000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 801.7529999999999,
"heapDeltaBytes": 5177236,
"heapUsedBytes": 64531672,
"domNodes": -269,
"jsHeapTotalBytes": 4669440,
"scriptDurationMs": 120.922,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2202.9999999999745,
"styleRecalcs": 69,
"styleRecalcDurationMs": 16.000000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1092.31,
"heapDeltaBytes": -3714624,
"heapUsedBytes": 56580180,
"domNodes": -269,
"jsHeapTotalBytes": 5455872,
"scriptDurationMs": 373.596,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2311.7350000001124,
"styleRecalcs": 68,
"styleRecalcDurationMs": 16.707,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1318.836,
"heapDeltaBytes": -4076012,
"heapUsedBytes": 56452932,
"domNodes": -270,
"jsHeapTotalBytes": 5455872,
"scriptDurationMs": 440.27,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3132.7930000000492,
"styleRecalcs": 65,
"styleRecalcDurationMs": 15.777,
"layouts": 60,
"layoutDurationMs": 8.431,
"taskDurationMs": 1332.319,
"heapDeltaBytes": -5223692,
"heapUsedBytes": 56356624,
"domNodes": -271,
"jsHeapTotalBytes": 7290880,
"scriptDurationMs": 478.97499999999997,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-zoom",
"durationMs": 3621.171000000004,
"styleRecalcs": 64,
"styleRecalcDurationMs": 18.107,
"layouts": 60,
"layoutDurationMs": 8.940999999999999,
"taskDurationMs": 1660.2630000000001,
"heapDeltaBytes": 18024160,
"heapUsedBytes": 80044616,
"domNodes": -276,
"jsHeapTotalBytes": 7028736,
"scriptDurationMs": 593.348,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "minimap-idle",
"durationMs": 2029.9969999999803,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.326999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 553.822,
"heapDeltaBytes": 5433788,
"heapUsedBytes": 66327816,
"domNodes": -269,
"jsHeapTotalBytes": 3883008,
"scriptDurationMs": 89.723,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "minimap-idle",
"durationMs": 2067.470999999955,
"styleRecalcs": 6,
"styleRecalcDurationMs": 9.568999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 762.083,
"heapDeltaBytes": 5581308,
"heapUsedBytes": 66902708,
"domNodes": -271,
"jsHeapTotalBytes": 5193728,
"scriptDurationMs": 116.936,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 590.5409999999733,
"styleRecalcs": 48,
"styleRecalcDurationMs": 11.506,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.108,
"heapDeltaBytes": -11387872,
"heapUsedBytes": 57504592,
"domNodes": 22,
"jsHeapTotalBytes": 20463616,
"scriptDurationMs": 121.897,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 739.1920000000027,
"styleRecalcs": 47,
"styleRecalcDurationMs": 14.168000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 464.067,
"heapDeltaBytes": -11175916,
"heapUsedBytes": 57824312,
"domNodes": 20,
"jsHeapTotalBytes": 20201472,
"scriptDurationMs": 135.13799999999998,
"eventListeners": 8,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 2003.723999999977,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.177999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 395.32500000000005,
"heapDeltaBytes": -23416420,
"heapUsedBytes": 45664868,
"domNodes": 18,
"jsHeapTotalBytes": 19017728,
"scriptDurationMs": 13.703,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "subgraph-idle",
"durationMs": 2029.2670000000044,
"styleRecalcs": 9,
"styleRecalcDurationMs": 9.162000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 556.215,
"heapDeltaBytes": -20950044,
"heapUsedBytes": 48082596,
"domNodes": -266,
"jsHeapTotalBytes": 19542016,
"scriptDurationMs": 25.674000000000003,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1692.228,
"styleRecalcs": 75,
"styleRecalcDurationMs": 35.58,
"layouts": 16,
"layoutDurationMs": 4.442999999999999,
"taskDurationMs": 715.4019999999999,
"heapDeltaBytes": -19626656,
"heapUsedBytes": 49350272,
"domNodes": -266,
"jsHeapTotalBytes": 18493440,
"scriptDurationMs": 91.836,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1810.069999999996,
"styleRecalcs": 79,
"styleRecalcDurationMs": 48.43,
"layouts": 16,
"layoutDurationMs": 8.298,
"taskDurationMs": 862.694,
"heapDeltaBytes": -5395224,
"heapUsedBytes": 63411212,
"domNodes": 67,
"jsHeapTotalBytes": 20201472,
"scriptDurationMs": 113.064,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-transition-enter",
"durationMs": 1351.725999999985,
"styleRecalcs": 18,
"styleRecalcDurationMs": 32.43599999999999,
"layouts": 14,
"layoutDurationMs": 14.147000000000002,
"taskDurationMs": 887.3070000000002,
"heapDeltaBytes": 1647840,
"heapUsedBytes": 83945756,
"domNodes": 13673,
"jsHeapTotalBytes": 11534336,
"scriptDurationMs": 40.863,
"eventListeners": 2371,
"totalBlockingTimeMs": 131,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "viewport-pan-sweep",
"durationMs": 8151.789000000008,
"styleRecalcs": 250,
"styleRecalcDurationMs": 41.972,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3627.515,
"heapDeltaBytes": 15694764,
"heapUsedBytes": 74981316,
"domNodes": -267,
"jsHeapTotalBytes": 6709248,
"scriptDurationMs": 1166.819,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8773.682000000008,
"styleRecalcs": 250,
"styleRecalcDurationMs": 48.590999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4747.152000000001,
"heapDeltaBytes": 23919324,
"heapUsedBytes": 83442624,
"domNodes": -267,
"jsHeapTotalBytes": 6709248,
"scriptDurationMs": 1471.879,
"eventListeners": -113,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "vue-large-graph-idle",
"durationMs": 13649.737000000016,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13625.226999999999,
"heapDeltaBytes": -70352880,
"heapUsedBytes": 162968956,
"domNodes": -8311,
"jsHeapTotalBytes": -7016448,
"scriptDurationMs": 524.948,
"eventListeners": -16386,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 14577.636000000097,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14524.346,
"heapDeltaBytes": -50460980,
"heapUsedBytes": 177481020,
"domNodes": -8311,
"jsHeapTotalBytes": -5705728,
"scriptDurationMs": 601.092,
"eventListeners": -16382,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.776666666666642,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 16923.17099999997,
"styleRecalcs": 112,
"styleRecalcDurationMs": 17.396999999999995,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 16886.889000000003,
"heapDeltaBytes": -41912648,
"heapUsedBytes": 166158432,
"domNodes": -8311,
"jsHeapTotalBytes": -11472896,
"scriptDurationMs": 867.207,
"eventListeners": -16382,
"totalBlockingTimeMs": 23,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 18113.562,
"styleRecalcs": 126,
"styleRecalcDurationMs": 21.033000000000023,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 18073.475,
"heapDeltaBytes": -38639272,
"heapUsedBytes": 170175612,
"domNodes": -8311,
"jsHeapTotalBytes": -12607488,
"scriptDurationMs": 916.9350000000001,
"eventListeners": -16378,
"totalBlockingTimeMs": 113,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 118.86299999997618,
"styleRecalcs": 8,
"styleRecalcDurationMs": 15.075,
"layouts": 2,
"layoutDurationMs": 0.44399999999999995,
"taskDurationMs": 81.919,
"heapDeltaBytes": 3229456,
"heapUsedBytes": 71167272,
"domNodes": 119,
"jsHeapTotalBytes": 3407872,
"scriptDurationMs": 7.281,
"eventListeners": 33,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "workflow-execution",
"durationMs": 461.16600000004837,
"styleRecalcs": 12,
"styleRecalcDurationMs": 20.204,
"layouts": 3,
"layoutDurationMs": 0.7079999999999999,
"taskDurationMs": 123.91199999999999,
"heapDeltaBytes": -16477276,
"heapUsedBytes": 51637760,
"domNodes": 115,
"jsHeapTotalBytes": 6569984,
"scriptDurationMs": 10.636000000000001,
"eventListeners": 65,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/litegraph/src/widgets/BaseWidget.ts`:
- Around line 150-157: Add a focused regression test for BaseWidget.setNodeId
using an empty widget name, with registerWidget returning undefined. Verify that
the widget’s local _state and value remain usable after registration is
declined, covering the conditional assignment in setNodeId rather than only
testing the store directly.
In `@src/stores/widgetValueStore.test.ts`:
- Around line 230-261: Expand the “un-keyable widget ids” tests around
namelessId to include representative malformed and missing-segment IDs, not only
an empty widget name. Register a valid sibling before exercising getWidget,
setValue, and deleteWidget for each invalid ID, and assert every operation
declines safely while the sibling remains readable with its original value; keep
assertions behavioral rather than tied to internal implementation details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aab1cf09-1002-4ea8-aaaf-bd008b031096
📒 Files selected for processing (3)
src/lib/litegraph/src/widgets/BaseWidget.tssrc/stores/widgetValueStore.test.tssrc/stores/widgetValueStore.ts
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #13773 +/- ##
==========================================
- Coverage 78.29% 78.28% -0.02%
==========================================
Files 1683 1683
Lines 98509 98702 +193
Branches 35475 35524 +49
==========================================
+ Hits 77129 77267 +138
- Misses 20922 20972 +50
- Partials 458 463 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
christian-byrne
added
the
preview-cpu
DrJKL
previously approved these changes
Jul 17, 2026
DrJKL
reviewed
Jul 17, 2026
| init: WidgetStateInit<TValue> | ||
| ): WidgetState<TValue> { | ||
| ): WidgetState<TValue> | undefined { | ||
| if (!isWidgetId(widgetId)) return undefined |
AustinMroz
previously approved these changes
Jul 17, 2026
Address review feedback on the store's un-keyable-id handling: - registerWidget logs a console.warn when it declines an un-keyable id, so the drop is visible during debugging instead of silent. - Add a BaseWidget.setNodeId test with an empty widget name asserting the widget keeps its local state and stays readable/writable when the store returns undefined. - Expand the store suite with representative malformed ids (missing/extra segments, empty graph/node/name) and assert declined ops never disturb a valid sibling on the same node. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed the review feedback in ba810e1:
Keeping this in draft only until Denys confirms rgthree Power Lora renders on the 1.47 preview build; unit coverage + both approvals are in. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/stores/widgetValueStore.test.ts`:
- Around line 244-251: Update the malformed-ID test around registerWidget to
call it once per ID, capture its return value, and assert it is undefined
without separately asserting no throw. Spy on console.warn, verify the expected
warning is emitted for each malformed ID, and restore the spy after the test
completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6cb25485-12d9-45ea-af46-cdd9189f86d0
📒 Files selected for processing (3)
src/lib/litegraph/src/widgets/BaseWidget.test.tssrc/stores/widgetValueStore.test.tssrc/stores/widgetValueStore.ts
…13774) ## Summary On subgraph convert, a widget could render with the wrong type (a multiline text widget appeared as an int widget until a page reload) because the widget value store returned a stale entry keyed by `graphId:nodeId:name`. ## Changes - **What**: `registerWidget` returned the existing store entry unconditionally (idempotent to preserve edited values across re-render). But when a subgraph convert reuses the same `graphId:nodeId:name` key for a *different* widget, the stale entry won — wrong type, wrong value, only corrected by a reload that rehydrates the store from serialized data. Now the idempotent return applies only when `existing.type === init.type`; on a type mismatch the id belongs to a different widget, so the store overwrites with the live type/value. Same-type re-registration still preserves edited values (unchanged behavior). - **Breaking**: none. ## Review Focus - @DrJKL @AustinMroz — this changes the idempotency semantic in the #12617 widget store (#13073 collision class). The discriminator is `type`; please sanity-check that no legitimate re-registration changes `type` for the same logical widget. Reload already produces the correct result, which is the evidence the in-memory path diverges here. - Stacked on #13773 (base = `fix/widget-store-custom-node-widgets`). Rebase to `main` once that lands. QA source: staging 1.47 — "multiline text widget converts to int inside/outside nested subgraph; refresh fixes it", working on 1.45.21. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…13773) (#13779) ## Summary Playwright regression for #13773: a custom node that registers a client-side widget with an empty/placeholder name (an un-keyable widget id) must not trip the Vue `NodeWidgets` error boundary and blank the node's entire widget grid. ## Changes - **What**: `browser_tests/tests/vueNodes/widgets/emptyNameWidget.spec.ts` — a `registerExtension` `nodeCreated` hook adds an empty-name widget to a KSampler, then asserts the widget grid renders (not `.node-error`) and the schema siblings (seed/steps/cfg) stay visible. Behavioral, retrying assertions, no `waitForTimeout`; a precondition poll confirms the empty-name widget is actually present so the test can't pass vacuously. - **Breaking**: none (test only). ## Review Focus - Stacked on #13773 (base = `fix/widget-store-custom-node-widgets`); retarget to `main` when that merges. - Draft: written to pass with the #13773 fix and fail without it, but could not be run locally (no browser binary / local ComfyUI backend in the dev env). Relying on CI to validate — will iterate if the fixture selectors need adjustment. Locks in the defensive guard from #13773 regardless of the exact custom-node trigger. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Austin Mroz <austin@comfy.org>
AustinMroz
marked this pull request as ready for review
July 18, 2026 03:33
dosubot
Bot
added
the
size:S
AustinMroz
added
needs-backport
AustinMroz
approved these changes
Jul 18, 2026
Member
|
@christian-byrne Successfully backported to #13787 |
Member
|
@christian-byrne Successfully backported to #13788 |
github-actions
Bot
removed
the
needs-backport
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Custom-node widgets with empty/placeholder names no longer throw and blank the entire node's widget grid on 1.47+ (regression from the store-backed widget-state refactor #12617).
Changes
parseWidgetIdrejects ids whose name segment is empty by throwing. Custom nodes register client-side widgets with empty/placeholder names (spacer/header/preview/button), producing such ids. The throw abortedBaseWidget.setNodeIdseeding (legacy renderer) and tripped the VueNodeWidgetserror boundary — blanking the node's whole widget grid (e.g. rgthree Power Lora Loader rendered no widgets; VHS Load Video / Video Combine lost their injected widgets). GuardregisterWidget/getWidget/deleteWidgetwithisWidgetIdso the store declines an un-keyable id instead of throwing.registerWidgetnow returnsundefinedfor such ids, andBaseWidgetkeeps its correct local_stateso the widget still functions. One un-storable widget no longer blanks its siblings.useWidgetValueStore().registerWidgetreturn type is nowWidgetState | undefined(onlyBaseWidgetconsumes the return; handled).Review Focus
graphId:nodeId:with empty name) is simply not store-backed and keeps its local value.registerWidgetcollision (same root cause, Ensure dynamic combo children cleanup state #13073 class) and will follow in a second PR.QA source: staging 1.47 custom-node findings (rgthree / VHS / pythongosssss / Impact), all working on 1.45.21.