mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-01 19:20:10 +00:00
fix: resolve bookmark lookup for subgraph blueprints (#9057)
## Summary Bookmarked subgraph blueprint nodes were not appearing in the favorites tree because `buildBookmarkTree` looked up nodes only in `nodeDefsByName`, which excludes subgraph blueprints. ## Changes - **What**: Added `allNodeDefsByName` computed in `nodeDefStore` that includes both regular nodes and subgraph blueprints. Updated `nodeBookmarkStore.buildBookmarkTree` to use it for bookmark resolution. ## Review Focus - `allNodeDefsByName` iterates over `nodeDefs` (which merges `subgraphBlueprints` + `nodeDefsByName`). Confirm this doesn't introduce performance concerns for large node sets. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9057-fix-resolve-bookmark-lookup-for-subgraph-blueprints-30e6d73d365081259bcae9d0c197de43) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -53,7 +53,7 @@ export const useNodeBookmarkStore = defineStore('nodeBookmark', () => {
|
||||
const parts = bookmark.split('/')
|
||||
const name = parts.pop() ?? ''
|
||||
const category = parts.join('/')
|
||||
const srcNodeDef = nodeDefStore.nodeDefsByName[name]
|
||||
const srcNodeDef = nodeDefStore.allNodeDefsByName[name]
|
||||
if (!srcNodeDef) {
|
||||
return null
|
||||
}
|
||||
|
||||
@@ -279,6 +279,58 @@ describe('useNodeDefStore', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('allNodeDefsByName', () => {
|
||||
it('should include all node defs by name', () => {
|
||||
const node1 = createMockNodeDef({ name: 'Node1' })
|
||||
const node2 = createMockNodeDef({ name: 'Node2' })
|
||||
|
||||
store.updateNodeDefs([node1, node2])
|
||||
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Node1')
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Node2')
|
||||
expect(store.allNodeDefsByName['Node1'].name).toBe('Node1')
|
||||
expect(store.allNodeDefsByName['Node2'].name).toBe('Node2')
|
||||
})
|
||||
|
||||
it('should include deprecated and experimental nodes', () => {
|
||||
const normal = createMockNodeDef({ name: 'Normal' })
|
||||
const deprecated = createMockNodeDef({
|
||||
name: 'Deprecated',
|
||||
deprecated: true
|
||||
})
|
||||
const experimental = createMockNodeDef({
|
||||
name: 'Experimental',
|
||||
experimental: true
|
||||
})
|
||||
|
||||
store.updateNodeDefs([normal, deprecated, experimental])
|
||||
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Normal')
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Deprecated')
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Experimental')
|
||||
})
|
||||
|
||||
it('should include nodes filtered out of visibleNodeDefs', () => {
|
||||
const normal = createMockNodeDef({
|
||||
name: 'Normal',
|
||||
deprecated: false
|
||||
})
|
||||
const deprecated = createMockNodeDef({
|
||||
name: 'Deprecated',
|
||||
deprecated: true
|
||||
})
|
||||
|
||||
store.updateNodeDefs([normal, deprecated])
|
||||
|
||||
// visibleNodeDefs filters out deprecated by default
|
||||
expect(store.visibleNodeDefs).toHaveLength(1)
|
||||
|
||||
// allNodeDefsByName includes all
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Normal')
|
||||
expect(store.allNodeDefsByName).toHaveProperty('Deprecated')
|
||||
})
|
||||
})
|
||||
|
||||
describe('performance', () => {
|
||||
it('should perform single traversal for multiple filters', () => {
|
||||
let filterCallCount = 0
|
||||
|
||||
@@ -371,6 +371,14 @@ export const useNodeDefStore = defineStore('nodeDef', () => {
|
||||
}
|
||||
return types
|
||||
})
|
||||
const allNodeDefsByName = computed(() => {
|
||||
const map: Record<string, ComfyNodeDefImpl> = {}
|
||||
for (const nodeDef of nodeDefs.value) {
|
||||
map[nodeDef.name] = nodeDef
|
||||
}
|
||||
return map
|
||||
})
|
||||
|
||||
const visibleNodeDefs = computed(() => {
|
||||
return nodeDefs.value.filter((nodeDef) =>
|
||||
nodeDefFilters.value.every((filter) => filter.predicate(nodeDef))
|
||||
@@ -496,6 +504,7 @@ export const useNodeDefStore = defineStore('nodeDef', () => {
|
||||
return {
|
||||
nodeDefsByName,
|
||||
nodeDefsByDisplayName,
|
||||
allNodeDefsByName,
|
||||
showDeprecated,
|
||||
showExperimental,
|
||||
showDevOnly,
|
||||
|
||||
Reference in New Issue
Block a user