mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-23 22:25:05 +00:00
refactor(world): delete bridge, revert BaseWidget/useUpstreamValue
Phase 2b of temp/plans/world-consolidation.md (completes Strategy A bridge elimination begun in Phase 2a). Now that useWidgetValueStore is a World facade (Phase 2a), the bridge is redundant. Deletions: - src/world/widgetWorldBridge.ts + widgetWorldBridge.test.ts deleted. Their register/getNodeWidgets coverage rolled into widgetValueStore facade tests in Phase 2a; widgetParent tests already moved to src/stores/widgetComponents.test.ts in Phase 1. Reverts (back to pre-slice-1 baseline): - BaseWidget.setNodeId no longer double-writes via registerWidgetInWorld; the store IS the World writer now. Drops three @/world/* imports. - useUpstreamValue reads via useWidgetValueStore().getNodeWidgets(). Drops three @/world/* imports. Test updates: - useUpstreamValue.test.ts setup uses useWidgetValueStore().registerWidget instead of registerWidgetInWorld(getWorld(), ...). Hoists setActivePinia(createTestingPinia) + resetWorldInstance into beforeEach. - widgetComponents.test.ts setup inlines a 4-line widget registration to replace the deleted bridge import. - entityIds.ts: GraphId type un-exported (no external consumer; YAGNI; re-export when slice 2 needs it). End state: - src/world/ is pure substrate (5 source files: brand, entityIds, componentKey, world, worldInstance). No bridge, no components dir. - BaseWidget.ts byte-identical with pre-slice-1 form at the setNodeId seam. - useWidgetValueStore is the sole owner of widget value state; Vue tracking flows naturally through reactive(Map) inside the World. Verification: - pnpm typecheck, format:check, knip clean. - 52 tests pass across src/world, src/stores/widgetValueStore, src/stores/widgetComponents, src/composables/useUpstreamValue, src/lib/litegraph/src/widgets/BaseWidget. - rg "@/world" src/lib/litegraph/src/widgets/BaseWidget.ts returns 0. - rg "@/world" src/composables/useUpstreamValue.ts returns 0. - rg "registerWidgetInWorld|getNodeWidgetsThroughWorld|widgetWorldBridge" src/ returns 0. Combined Phase 2 non-test diff -53 LOC (under 150 LOC budget). BaseWidget._state shared reactive identity contract preserved end-to-end. Amp-Thread-ID: https://ampcode.com/threads/T-019dd146-a3ad-734d-9825-0ab356454dd5 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -1,11 +1,12 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { reactive } from 'vue'
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { UUID } from '@/lib/litegraph/src/utils/uuid'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { asGraphId } from '@/world/entityIds'
|
||||
import { registerWidgetInWorld } from '@/world/widgetWorldBridge'
|
||||
import { getWorld, resetWorldInstance } from '@/world/worldInstance'
|
||||
import { resetWorldInstance } from '@/world/worldInstance'
|
||||
|
||||
import {
|
||||
boundsExtractor,
|
||||
@@ -133,18 +134,21 @@ describe('boundsExtractor', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('useUpstreamValue (World-backed read path)', () => {
|
||||
it('reads upstream node widgets via the World, not the Pinia store', () => {
|
||||
describe('useUpstreamValue (store-backed read path)', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
resetWorldInstance()
|
||||
const graphId = asGraphId('00000000-0000-0000-0000-000000000001')
|
||||
const state = reactive<WidgetState>({
|
||||
})
|
||||
|
||||
it('reads upstream node widgets via the widget value store', () => {
|
||||
const graphId = '00000000-0000-0000-0000-000000000001' as UUID
|
||||
const state = useWidgetValueStore().registerWidget(graphId, {
|
||||
nodeId: 'upstream-1' as NodeId,
|
||||
name: 'value',
|
||||
type: 'number',
|
||||
value: 7,
|
||||
options: {}
|
||||
})
|
||||
registerWidgetInWorld(getWorld(), graphId, state)
|
||||
|
||||
const upstreamValue = useUpstreamValue<number>(
|
||||
() => ({ nodeId: 'upstream-1', outputName: 'value' }),
|
||||
@@ -157,7 +161,6 @@ describe('useUpstreamValue (World-backed read path)', () => {
|
||||
})
|
||||
|
||||
it('returns undefined when no upstream linkage is provided', () => {
|
||||
resetWorldInstance()
|
||||
const upstreamValue = useUpstreamValue(
|
||||
() => undefined,
|
||||
singleValueExtractor((v): v is number => typeof v === 'number')
|
||||
|
||||
@@ -1,12 +1,10 @@
|
||||
import { computed } from 'vue'
|
||||
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
import type { Bounds } from '@/renderer/core/layout/types'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import type { LinkedUpstreamInfo } from '@/types/simplifiedWidget'
|
||||
import { asGraphId } from '@/world/entityIds'
|
||||
import { getNodeWidgetsThroughWorld } from '@/world/widgetWorldBridge'
|
||||
import { getWorld } from '@/world/worldInstance'
|
||||
|
||||
type ValueExtractor<T = unknown> = (
|
||||
widgets: WidgetState[],
|
||||
@@ -18,17 +16,14 @@ export function useUpstreamValue<T>(
|
||||
extractValue: ValueExtractor<T>
|
||||
) {
|
||||
const canvasStore = useCanvasStore()
|
||||
const widgetValueStore = useWidgetValueStore()
|
||||
|
||||
return computed(() => {
|
||||
const upstream = getLinkedUpstream()
|
||||
if (!upstream) return undefined
|
||||
const graphId = canvasStore.canvas?.graph?.rootGraph.id
|
||||
if (!graphId) return undefined
|
||||
const widgets = getNodeWidgetsThroughWorld(
|
||||
getWorld(),
|
||||
asGraphId(graphId),
|
||||
upstream.nodeId
|
||||
)
|
||||
const widgets = widgetValueStore.getNodeWidgets(graphId, upstream.nodeId)
|
||||
return extractValue(widgets, upstream.outputName)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -20,9 +20,6 @@ import type {
|
||||
import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import { asGraphId } from '@/world/entityIds'
|
||||
import { registerWidgetInWorld } from '@/world/widgetWorldBridge'
|
||||
import { getWorld } from '@/world/worldInstance'
|
||||
|
||||
export interface DrawWidgetOptions {
|
||||
/** The width of the node where this widget will be displayed. */
|
||||
@@ -152,11 +149,6 @@ export abstract class BaseWidget<TWidget extends IBaseWidget = IBaseWidget>
|
||||
value: this.value,
|
||||
nodeId
|
||||
})
|
||||
registerWidgetInWorld(
|
||||
getWorld(),
|
||||
asGraphId(graphId),
|
||||
this._state as WidgetState
|
||||
)
|
||||
}
|
||||
|
||||
constructor(widget: TWidget & { node: LGraphNode })
|
||||
|
||||
@@ -2,10 +2,14 @@ import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
import { asGraphId, nodeEntityId, widgetEntityId } from '@/world/entityIds'
|
||||
import { registerWidgetInWorld } from '@/world/widgetWorldBridge'
|
||||
import { createWorld } from '@/world/world'
|
||||
|
||||
import { widgetParent } from './widgetComponents'
|
||||
import type { WidgetValue } from './widgetComponents'
|
||||
import {
|
||||
WidgetContainerComponent,
|
||||
WidgetValueComponent,
|
||||
widgetParent
|
||||
} from './widgetComponents'
|
||||
|
||||
const graphId = asGraphId('00000000-0000-0000-0000-000000000001')
|
||||
|
||||
@@ -16,9 +20,14 @@ function makeState(nodeId: string, name: string, value: unknown): WidgetState {
|
||||
describe('widgetParent', () => {
|
||||
it('returns the owning node entity for a widget', () => {
|
||||
const world = createWorld()
|
||||
registerWidgetInWorld(world, graphId, makeState('node-1', 'seed', 1))
|
||||
const widgetId = widgetEntityId(graphId, 'node-1', 'seed')
|
||||
expect(widgetParent(world, widgetId)).toBe(nodeEntityId(graphId, 'node-1'))
|
||||
const state = makeState('node-1', 'seed', 1)
|
||||
const widgetId = widgetEntityId(graphId, state.nodeId, state.name)
|
||||
const ownerId = nodeEntityId(graphId, state.nodeId)
|
||||
world.setComponent(widgetId, WidgetValueComponent, state as WidgetValue)
|
||||
world.setComponent(ownerId, WidgetContainerComponent, {
|
||||
widgetIds: [widgetId]
|
||||
})
|
||||
expect(widgetParent(world, widgetId)).toBe(ownerId)
|
||||
})
|
||||
|
||||
it('returns undefined when no container references the widget', () => {
|
||||
|
||||
@@ -13,7 +13,7 @@ import type { UUID } from '@/lib/litegraph/src/utils/uuid'
|
||||
|
||||
import type { Brand } from './brand'
|
||||
|
||||
export type GraphId = Brand<UUID, 'GraphId'>
|
||||
type GraphId = Brand<UUID, 'GraphId'>
|
||||
export type NodeEntityId = Brand<string, 'NodeEntityId'>
|
||||
export type WidgetEntityId = Brand<string, 'WidgetEntityId'>
|
||||
|
||||
|
||||
@@ -1,88 +0,0 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
WidgetContainerComponent,
|
||||
WidgetValueComponent
|
||||
} from '@/stores/widgetComponents'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
|
||||
import { asGraphId, nodeEntityId, widgetEntityId } from './entityIds'
|
||||
import {
|
||||
getNodeWidgetsThroughWorld,
|
||||
registerWidgetInWorld
|
||||
} from './widgetWorldBridge'
|
||||
import { createWorld } from './world'
|
||||
|
||||
function makeState(nodeId: string, name: string, value: unknown): WidgetState {
|
||||
return { nodeId, name, type: 'number', value, options: {} }
|
||||
}
|
||||
|
||||
const graphId = asGraphId('00000000-0000-0000-0000-000000000001')
|
||||
|
||||
describe('registerWidgetInWorld', () => {
|
||||
it('writes WidgetValue and updates WidgetContainer on the node', () => {
|
||||
const world = createWorld()
|
||||
const state = makeState('node-1', 'seed', 100)
|
||||
|
||||
registerWidgetInWorld(world, graphId, state)
|
||||
|
||||
const widgetId = widgetEntityId(graphId, 'node-1', 'seed')
|
||||
const nodeId = nodeEntityId(graphId, 'node-1')
|
||||
expect(world.getComponent(widgetId, WidgetValueComponent)?.value).toBe(100)
|
||||
expect(
|
||||
world.getComponent(nodeId, WidgetContainerComponent)?.widgetIds
|
||||
).toEqual([widgetId])
|
||||
})
|
||||
|
||||
it('shares object identity with the registered state (reactive bridge)', () => {
|
||||
const world = createWorld()
|
||||
const state = makeState('node-1', 'seed', 100)
|
||||
registerWidgetInWorld(world, graphId, state)
|
||||
|
||||
state.value = 200
|
||||
const widgetId = widgetEntityId(graphId, 'node-1', 'seed')
|
||||
expect(world.getComponent(widgetId, WidgetValueComponent)?.value).toBe(200)
|
||||
})
|
||||
|
||||
it('appends additional widgets to the same node container', () => {
|
||||
const world = createWorld()
|
||||
registerWidgetInWorld(world, graphId, makeState('node-1', 'seed', 1))
|
||||
registerWidgetInWorld(world, graphId, makeState('node-1', 'cfg', 7))
|
||||
|
||||
const nodeId = nodeEntityId(graphId, 'node-1')
|
||||
const ids = world.getComponent(nodeId, WidgetContainerComponent)?.widgetIds
|
||||
expect(ids).toEqual([
|
||||
widgetEntityId(graphId, 'node-1', 'seed'),
|
||||
widgetEntityId(graphId, 'node-1', 'cfg')
|
||||
])
|
||||
})
|
||||
|
||||
it('does not duplicate widgetIds when the same widget re-registers', () => {
|
||||
const world = createWorld()
|
||||
const state = makeState('node-1', 'seed', 1)
|
||||
registerWidgetInWorld(world, graphId, state)
|
||||
registerWidgetInWorld(world, graphId, state)
|
||||
|
||||
const nodeId = nodeEntityId(graphId, 'node-1')
|
||||
expect(
|
||||
world.getComponent(nodeId, WidgetContainerComponent)?.widgetIds
|
||||
).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getNodeWidgetsThroughWorld', () => {
|
||||
it('returns all widget states attached to a node', () => {
|
||||
const world = createWorld()
|
||||
registerWidgetInWorld(world, graphId, makeState('node-1', 'seed', 1))
|
||||
registerWidgetInWorld(world, graphId, makeState('node-1', 'cfg', 7))
|
||||
registerWidgetInWorld(world, graphId, makeState('node-2', 'seed', 99))
|
||||
|
||||
const widgets = getNodeWidgetsThroughWorld(world, graphId, 'node-1')
|
||||
expect(widgets.map((w) => w.name).sort()).toEqual(['cfg', 'seed'])
|
||||
})
|
||||
|
||||
it('returns an empty array for unknown nodes', () => {
|
||||
const world = createWorld()
|
||||
expect(getNodeWidgetsThroughWorld(world, graphId, 'missing')).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -1,62 +0,0 @@
|
||||
import type { NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { WidgetValue } from '@/stores/widgetComponents'
|
||||
import {
|
||||
WidgetContainerComponent,
|
||||
WidgetValueComponent
|
||||
} from '@/stores/widgetComponents'
|
||||
import type { WidgetState } from '@/stores/widgetValueStore'
|
||||
|
||||
import type { GraphId } from './entityIds'
|
||||
import { nodeEntityId, widgetEntityId } from './entityIds'
|
||||
import type { World } from './world'
|
||||
|
||||
/**
|
||||
* Slice 1 bridge: writes widget entities into the World whenever
|
||||
* `WidgetValueStore.registerWidget` runs. The `state` argument is the
|
||||
* SAME reactive object the store holds — sharing identity preserves Vue
|
||||
* tracking across both read paths.
|
||||
*/
|
||||
export function registerWidgetInWorld(
|
||||
world: World,
|
||||
graphId: GraphId,
|
||||
state: WidgetState
|
||||
): void {
|
||||
const widgetId = widgetEntityId(graphId, state.nodeId, state.name)
|
||||
// `state` IS the reactive object owned by `WidgetValueStore`; sharing the
|
||||
// reference is intentional — Vue tracking flows through both read paths
|
||||
// during the slice-1 bridge window. The wider WidgetState shape collapses
|
||||
// to `WidgetValue` at the component-key boundary.
|
||||
world.setComponent(widgetId, WidgetValueComponent, state as WidgetValue)
|
||||
|
||||
const nodeId = nodeEntityId(graphId, state.nodeId)
|
||||
const container = world.getComponent(nodeId, WidgetContainerComponent)
|
||||
if (!container) {
|
||||
world.setComponent(nodeId, WidgetContainerComponent, {
|
||||
widgetIds: [widgetId]
|
||||
})
|
||||
return
|
||||
}
|
||||
if (!container.widgetIds.includes(widgetId)) {
|
||||
container.widgetIds.push(widgetId)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Look up all widget value states attached to a node, going through the
|
||||
* World rather than the Pinia store. Used by `useUpstreamValue`.
|
||||
*/
|
||||
export function getNodeWidgetsThroughWorld(
|
||||
world: World,
|
||||
graphId: GraphId,
|
||||
nodeId: NodeId
|
||||
): WidgetState[] {
|
||||
const owner = nodeEntityId(graphId, nodeId)
|
||||
const container = world.getComponent(owner, WidgetContainerComponent)
|
||||
if (!container) return []
|
||||
const widgets: WidgetState[] = []
|
||||
for (const widgetId of container.widgetIds) {
|
||||
const value = world.getComponent(widgetId, WidgetValueComponent)
|
||||
if (value) widgets.push(value as unknown as WidgetState)
|
||||
}
|
||||
return widgets
|
||||
}
|
||||
Reference in New Issue
Block a user