-
Notifications
You must be signed in to change notification settings - Fork 534
feat: add WidgetValueStore for centralized widget value management #8594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
80a5108
feat: add WidgetValueStore for centralized widget values
DrJKL fcd83d8
feat: integrate BaseWidget with WidgetValueStore
DrJKL 85c6bd1
feat: LGraphNode.addCustomWidget wires widget to node ID
DrJKL 61403ea
test: add Pinia setup to proxyWidget tests for WidgetValueStore
DrJKL b1e066e
feat: integrate WidgetValueStore with BaseWidget for centralized reac…
DrJKL 3988c4f
feat: expand widgetValueStore with full widget state management
DrJKL 36387fa
feat: integrate BaseWidget metadata properties with widgetValueStore
DrJKL f9db91d
feat: implement automatic widget registration in setNodeId()
DrJKL 4a491c5
feat: add useWidget composable and WidgetItemByKey component
DrJKL bb13bec
refactor(litegraph): add widget filtering helper methods
DrJKL ee8500d
refactor: simplify SafeWidgetData by leveraging widgetValueStore
DrJKL 616b2d6
Knip fix
DrJKL 6e3d6f6
Remove redundant state from Widget store.
DrJKL 4ac436c
fix: sync DOM widget values with WidgetValueStore
DrJKL 687c77f
refactor: derive WidgetState from IBaseWidget via Pick
DrJKL a9b0def
refactor: remove redundant setters from widgetValueStore
DrJKL ea26a16
refactor: consolidate BaseWidget fields with WidgetState
DrJKL 627ca0b
YAGNI: useWidget
DrJKL 8473ecd
Re-bind the state to the reactive object proxy.
DrJKL 6f06ce7
Keep the widget array itself reactive, Add proxy support (to be clean…
DrJKL 644295c
fix: clean up widget state when nodes are removed
DrJKL c89ec31
refactor: use store-backed widget.hidden/advanced instead of legacy p…
DrJKL ffac1c1
test: replace as any with proper BaseWidget type assertions
DrJKL 2288c25
fix: use correct widget name for proxy widget matching
DrJKL 88eb864
refactor: remove unused widgetValueStore methods
DrJKL 395940c
Enforce calling the setter to keep the store synced as the source of …
DrJKL e26261c
[automated] Update test expectations
invalid-email-address e1206d3
Merge branch 'main' into drjkl/widget-store
DrJKL bd39cad
Mark the nodeId as optional during setup of the widget.
DrJKL 7bc3936
merge main
DrJKL 5d1e9f6
feat: share LGraphState between root graph and subgraphs
DrJKL 09b347a
[automated] Apply ESLint and Oxfmt fixes
actions-user 3e5ed3c
feat: add ensureGlobalIdUniqueness for cross-graph ID deduplication
DrJKL e6e3164
[automated] Update test expectations
invalid-email-address b274c6f
fix: use getNodeRefsByType for subgraph nodes in E2E tests
DrJKL 2478af2
fix: remap floating link node IDs in ensureGlobalIdUniqueness
DrJKL c116f3c
fix: Widgets need different names to be able to have different hidden…
DrJKL ac8d762
Update uniqueness conversion.
DrJKL c9f249e
test: add Playwright tests for subgraph duplicate ID remapping
DrJKL ea2fe2b
[automated] Apply ESLint and Oxfmt fixes
actions-user 215bc8c
refactor: clean up subgraph duplicate ID spec
DrJKL b0e03c9
Merge remote-tracking branch 'origin/main' into drjkl/widget-store
DrJKL 2a9d1d1
Merge remote-tracking branch 'origin/main' into drjkl/widget-store
DrJKL 117f6a0
[automated] Update test expectations
invalid-email-address 18e75ca
fix: correct widget advanced/read_only option handling
DrJKL b477d00
Merge branch 'main' into drjkl/widget-store
DrJKL 82681db
refactor: extract NodeBindable interface and isNodeBindable type guard
DrJKL 269ef6c
fix: avoid ID collisions in ensureGlobalIdUniqueness
DrJKL f2a0395
Merge branch 'main' into drjkl/widget-store
DrJKL 66e75c2
Merge branch 'main' into drjkl/widget-store
DrJKL 52107d0
feat: gate ensureGlobalIdUniqueness behind experimental setting
DrJKL d695060
merge: resolve conflict with origin/main, keep 1.40.0 versionAdded
DrJKL 67a0ff0
fix: remove hidden and advanced from widgetValueStore
DrJKL 2cd525a
fix: restore hidden/advanced reads from widget.options to match main
DrJKL 790f74a
Remove option to not dedupe node IDs since it's required for WidgetVa…
DrJKL 0a611ae
Merge branch 'main' into drjkl/widget-store
DrJKL File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
Binary file modified
BIN
-40 Bytes
(100%)
...oad/uploadWidgets.spec.ts-snapshots/vue-nodes-upload-widgets-chromium-linux.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,49 +1,76 @@ | ||
| import { setActivePinia } from 'pinia' | ||
| import { createTestingPinia } from '@pinia/testing' | ||
| import { describe, expect, it, vi } from 'vitest' | ||
| import { nextTick, watch } from 'vue' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { computed, nextTick, watch } from 'vue' | ||
|
|
||
| import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager' | ||
| import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' | ||
| import { BaseWidget, LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' | ||
| import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums' | ||
| import { useWidgetValueStore } from '@/stores/widgetValueStore' | ||
|
|
||
| setActivePinia(createTestingPinia()) | ||
| describe('Node Reactivity', () => { | ||
| beforeEach(() => { | ||
| setActivePinia(createTestingPinia({ stubActions: false })) | ||
| }) | ||
|
|
||
| function createTestGraph() { | ||
| const graph = new LGraph() | ||
| const node = new LGraphNode('test') | ||
| node.addInput('input', 'INT') | ||
| node.addWidget('number', 'testnum', 2, () => undefined, {}) | ||
| graph.add(node) | ||
| function createTestGraph() { | ||
| const graph = new LGraph() | ||
| const node = new LGraphNode('test') | ||
| node.addInput('input', 'INT') | ||
| node.addWidget('number', 'testnum', 2, () => undefined, {}) | ||
| graph.add(node) | ||
|
|
||
| const { vueNodeData } = useGraphNodeManager(graph) | ||
| const onReactivityUpdate = vi.fn() | ||
| watch(vueNodeData, onReactivityUpdate) | ||
| const { vueNodeData } = useGraphNodeManager(graph) | ||
|
|
||
| return [node, graph, onReactivityUpdate] as const | ||
| } | ||
| return { node, graph, vueNodeData } | ||
| } | ||
|
|
||
| describe('Node Reactivity', () => { | ||
| it('should trigger on callback', async () => { | ||
| const [node, , onReactivityUpdate] = createTestGraph() | ||
| it('widget values are reactive through the store', async () => { | ||
| const { node } = createTestGraph() | ||
| const store = useWidgetValueStore() | ||
| const widget = node.widgets![0] | ||
|
|
||
| // Verify widget is a BaseWidget with correct value and node assignment | ||
| expect(widget).toBeInstanceOf(BaseWidget) | ||
| expect(widget.value).toBe(2) | ||
| expect((widget as BaseWidget).node.id).toBe(node.id) | ||
|
|
||
| // Initial value should be in store after setNodeId was called | ||
| expect(store.getWidget(node.id, 'testnum')?.value).toBe(2) | ||
|
|
||
| node.widgets![0].callback!(2) | ||
| const onValueChange = vi.fn() | ||
| const widgetValue = computed( | ||
| () => store.getWidget(node.id, 'testnum')?.value | ||
| ) | ||
| watch(widgetValue, onValueChange) | ||
|
|
||
| widget.value = 42 | ||
| await nextTick() | ||
| expect(onReactivityUpdate).toHaveBeenCalledTimes(1) | ||
|
|
||
| expect(widgetValue.value).toBe(42) | ||
| expect(onValueChange).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| it('should remain reactive after a connection is made', async () => { | ||
| const [node, graph, onReactivityUpdate] = createTestGraph() | ||
| it('widget values remain reactive after a connection is made', async () => { | ||
| const { node, graph } = createTestGraph() | ||
| const store = useWidgetValueStore() | ||
| const onValueChange = vi.fn() | ||
|
|
||
| graph.trigger('node:slot-links:changed', { | ||
| nodeId: '1', | ||
| nodeId: String(node.id), | ||
| slotType: NodeSlotType.INPUT | ||
| }) | ||
| await nextTick() | ||
| onReactivityUpdate.mockClear() | ||
|
|
||
| node.widgets![0].callback!(2) | ||
| const widgetValue = computed( | ||
| () => store.getWidget(node.id, 'testnum')?.value | ||
| ) | ||
| watch(widgetValue, onValueChange) | ||
|
|
||
| node.widgets![0].value = 99 | ||
| await nextTick() | ||
| expect(onReactivityUpdate).toHaveBeenCalledTimes(1) | ||
|
|
||
| expect(onValueChange).toHaveBeenCalledTimes(1) | ||
| expect(widgetValue.value).toBe(99) | ||
| }) | ||
| }) |
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit/question: Ah, didn't see this when I made previous comment. Wonder if it's the best solution for wiring inputSpec onto the widgets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost certainly not. we'd probably want to compose this with the
nodeDefstore. Part of that will also be eliminating ProxyWidgets so that we can avoid having to follow the subgraph chain through to the concrete widget.