[backport rh-test] Fix type on LoadClip being marked as asset (#6214)

Backport of #6207 to `rh-test`

Automatically created by backport workflow.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6214-backport-rh-test-Fix-type-on-LoadClip-being-marked-as-asset-2956d73d36508180b4ccd59a40672327)
by [Unito](https://www.unito.io)

Co-authored-by: AustinMroz <austin@comfy.org>
This commit is contained in:
Comfy Org PR Bot
2025-10-23 12:56:53 +09:00
committed by GitHub
parent f0a19ebb1d
commit dd1af641db
6 changed files with 45 additions and 40 deletions

View File

@@ -113,11 +113,16 @@ function createAssetService() {
* Checks if a widget input should use the asset browser based on both input name and node comfyClass * Checks if a widget input should use the asset browser based on both input name and node comfyClass
* *
* @param nodeType - The ComfyUI node comfyClass (e.g., 'CheckpointLoaderSimple', 'LoraLoader') * @param nodeType - The ComfyUI node comfyClass (e.g., 'CheckpointLoaderSimple', 'LoraLoader')
* @param widgetName - The name of the widget to check (e.g., 'ckpt_name')
* @returns true if this input should use asset browser * @returns true if this input should use asset browser
*/ */
function isAssetBrowserEligible(nodeType: string = ''): boolean { function isAssetBrowserEligible(
nodeType: string | undefined,
widgetName: string
): boolean {
if (!nodeType || !widgetName) return false
return ( return (
!!nodeType && useModelToNodeStore().getRegisteredNodeTypes().has(nodeType) useModelToNodeStore().getRegisteredNodeTypes()[nodeType] === widgetName
) )
} }

View File

@@ -356,7 +356,10 @@ const addComboWidget = (
): IBaseWidget => { ): IBaseWidget => {
const settingStore = useSettingStore() const settingStore = useSettingStore()
const isUsingAssetAPI = settingStore.get('Comfy.Assets.UseAssetAPI') const isUsingAssetAPI = settingStore.get('Comfy.Assets.UseAssetAPI')
const isEligible = assetService.isAssetBrowserEligible(node.comfyClass) const isEligible = assetService.isAssetBrowserEligible(
node.comfyClass,
inputSpec.name
)
if (isUsingAssetAPI && isEligible) { if (isUsingAssetAPI && isEligible) {
const currentValue = getDefaultValue(inputSpec) const currentValue = getDefaultValue(inputSpec)

View File

@@ -25,12 +25,12 @@ export const useModelToNodeStore = defineStore('modelToNode', () => {
const haveDefaultsLoaded = ref(false) const haveDefaultsLoaded = ref(false)
/** Internal computed for reactive caching of registered node types */ /** Internal computed for reactive caching of registered node types */
const registeredNodeTypes = computed(() => { const registeredNodeTypes = computed<Record<string, string>>(() => {
return new Set( return Object.fromEntries(
Object.values(modelToNodeMap.value) Object.values(modelToNodeMap.value)
.flat() .flat()
.filter((provider) => !!provider.nodeDef) .filter((provider) => !!provider.nodeDef)
.map((provider) => provider.nodeDef.name) .map((provider) => [provider.nodeDef.name, provider.key])
) )
}) })
@@ -51,7 +51,7 @@ export const useModelToNodeStore = defineStore('modelToNode', () => {
}) })
/** Get set of all registered node types for efficient lookup */ /** Get set of all registered node types for efficient lookup */
function getRegisteredNodeTypes(): Set<string> { function getRegisteredNodeTypes(): Record<string, string> {
registerDefaults() registerDefaults()
return registeredNodeTypes.value return registeredNodeTypes.value
} }

View File

@@ -163,7 +163,8 @@ describe('useComboWidget', () => {
) )
expect(mockSettingStoreGet).toHaveBeenCalledWith('Comfy.Assets.UseAssetAPI') expect(mockSettingStoreGet).toHaveBeenCalledWith('Comfy.Assets.UseAssetAPI')
expect(vi.mocked(assetService.isAssetBrowserEligible)).toHaveBeenCalledWith( expect(vi.mocked(assetService.isAssetBrowserEligible)).toHaveBeenCalledWith(
'CheckpointLoaderSimple' 'CheckpointLoaderSimple',
'ckpt_name'
) )
expect(widget).toBe(mockWidget) expect(widget).toBe(mockWidget)
}) })

View File

@@ -16,15 +16,12 @@ const mockGetCategoryForNodeType = vi.fn()
vi.mock('@/stores/modelToNodeStore', () => ({ vi.mock('@/stores/modelToNodeStore', () => ({
useModelToNodeStore: vi.fn(() => ({ useModelToNodeStore: vi.fn(() => ({
getRegisteredNodeTypes: vi.fn( getRegisteredNodeTypes: vi.fn(() => ({
() => CheckpointLoaderSimple: 'ckpt_name',
new Set([ LoraLoader: 'lora_name',
'CheckpointLoaderSimple', VAELoader: 'vae_name',
'LoraLoader', TestNode: ''
'VAELoader', })),
'TestNode'
])
),
getCategoryForNodeType: mockGetCategoryForNodeType, getCategoryForNodeType: mockGetCategoryForNodeType,
modelToNodeMap: { modelToNodeMap: {
checkpoints: [{ nodeDef: { name: 'CheckpointLoaderSimple' } }], checkpoints: [{ nodeDef: { name: 'CheckpointLoaderSimple' } }],
@@ -191,19 +188,19 @@ describe('assetService', () => {
}) })
describe('isAssetBrowserEligible', () => { describe('isAssetBrowserEligible', () => {
it('should return true for registered node types', () => { it.for<[string, string, boolean, string]>([
expect( ['CheckpointLoaderSimple', 'ckpt_name', true, 'valid inputs'],
assetService.isAssetBrowserEligible('CheckpointLoaderSimple') ['LoraLoader', 'lora_name', true, 'valid inputs'],
).toBe(true) ['VAELoader', 'vae_name', true, 'valid inputs'],
expect(assetService.isAssetBrowserEligible('LoraLoader')).toBe(true) ['CheckpointLoaderSimple', 'type', false, 'other combo widgets'],
expect(assetService.isAssetBrowserEligible('VAELoader')).toBe(true) ['UnknownNode', 'widget', false, 'unregistered types'],
}) ['NotRegistered', 'widget', false, 'unregistered types']
])(
it('should return false for unregistered node types', () => { 'isAssetBrowserEligible("%s", "%s") should return %s for %s',
expect(assetService.isAssetBrowserEligible('UnknownNode')).toBe(false) ([type, name, expected]) => {
expect(assetService.isAssetBrowserEligible('NotRegistered')).toBe(false) expect(assetService.isAssetBrowserEligible(type, name)).toBe(expected)
expect(assetService.isAssetBrowserEligible('')).toBe(false) }
}) )
}) })
describe('getAssetsForNodeType', () => { describe('getAssetsForNodeType', () => {

View File

@@ -288,7 +288,7 @@ describe('useModelToNodeStore', () => {
// Non-existent nodes are filtered out from registered types // Non-existent nodes are filtered out from registered types
const types = modelToNodeStore.getRegisteredNodeTypes() const types = modelToNodeStore.getRegisteredNodeTypes()
expect(types.has('NonExistentLoader')).toBe(false) expect(types['NonExistentLoader']).toBe(undefined)
expect( expect(
modelToNodeStore.getCategoryForNodeType('NonExistentLoader') modelToNodeStore.getCategoryForNodeType('NonExistentLoader')
@@ -347,13 +347,13 @@ describe('useModelToNodeStore', () => {
}) })
describe('getRegisteredNodeTypes', () => { describe('getRegisteredNodeTypes', () => {
it('should return a Set instance', () => { it('should return an object', () => {
const modelToNodeStore = useModelToNodeStore() const modelToNodeStore = useModelToNodeStore()
const result = modelToNodeStore.getRegisteredNodeTypes() const result = modelToNodeStore.getRegisteredNodeTypes()
expect(result).toBeInstanceOf(Set) expect(result).toBeTypeOf('object')
}) })
it('should return empty set when nodeDefStore is empty', () => { it('should return empty Record when nodeDefStore is empty', () => {
// Create fresh Pinia for this test to avoid state persistence // Create fresh Pinia for this test to avoid state persistence
setActivePinia(createPinia()) setActivePinia(createPinia())
@@ -363,7 +363,7 @@ describe('useModelToNodeStore', () => {
const modelToNodeStore = useModelToNodeStore() const modelToNodeStore = useModelToNodeStore()
const result = modelToNodeStore.getRegisteredNodeTypes() const result = modelToNodeStore.getRegisteredNodeTypes()
expect(result.size).toBe(0) expect(result).toStrictEqual({})
// Restore original mock for subsequent tests // Restore original mock for subsequent tests
vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({ vi.mocked(useNodeDefStore, { partial: true }).mockReturnValue({
@@ -371,16 +371,15 @@ describe('useModelToNodeStore', () => {
}) })
}) })
it('should contain node types for efficient Set.has() lookups', () => { it('should contain node types to resolve widget name', () => {
const modelToNodeStore = useModelToNodeStore() const modelToNodeStore = useModelToNodeStore()
modelToNodeStore.registerDefaults() modelToNodeStore.registerDefaults()
const result = modelToNodeStore.getRegisteredNodeTypes() const result = modelToNodeStore.getRegisteredNodeTypes()
// Test Set.has() functionality which assetService depends on expect(result['CheckpointLoaderSimple']).toBe('ckpt_name')
expect(result.has('CheckpointLoaderSimple')).toBe(true) expect(result['LoraLoader']).toBe('lora_name')
expect(result.has('LoraLoader')).toBe(true) expect(result['NonExistentNode']).toBe(undefined)
expect(result.has('NonExistentNode')).toBe(false)
}) })
}) })