diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index 6897940e4..1681712c4 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -17,7 +17,7 @@ jobs: steps: - uses: Comfy-Org/ComfyUI_frontend_setup_action@v2.2 with: - devtools_ref: 8ff192366066cd44bfc2a25713c9957ccf665938 + devtools_ref: 7b81139e904519db8e5481899ef36bbb4393cb6b - name: Run Jest tests run: | npm run test:generate @@ -29,7 +29,7 @@ jobs: steps: - uses: Comfy-Org/ComfyUI_frontend_setup_action@v2.2 with: - devtools_ref: 8ff192366066cd44bfc2a25713c9957ccf665938 + devtools_ref: 7b81139e904519db8e5481899ef36bbb4393cb6b - name: Install Playwright Browsers run: npx playwright install chromium --with-deps working-directory: ComfyUI_frontend @@ -48,7 +48,7 @@ jobs: steps: - uses: Comfy-Org/ComfyUI_frontend_setup_action@v2.2 with: - devtools_ref: 8ff192366066cd44bfc2a25713c9957ccf665938 + devtools_ref: 7b81139e904519db8e5481899ef36bbb4393cb6b - name: Install Playwright Browsers run: npx playwright install chromium --with-deps working-directory: ComfyUI_frontend @@ -67,7 +67,7 @@ jobs: steps: - uses: Comfy-Org/ComfyUI_frontend_setup_action@v2.2 with: - devtools_ref: 8ff192366066cd44bfc2a25713c9957ccf665938 + devtools_ref: 7b81139e904519db8e5481899ef36bbb4393cb6b - name: Install Playwright Browsers run: npx playwright install chromium --with-deps working-directory: ComfyUI_frontend diff --git a/browser_tests/remoteWidgets.spec.ts b/browser_tests/remoteWidgets.spec.ts index 466649710..285570fd9 100644 --- a/browser_tests/remoteWidgets.spec.ts +++ b/browser_tests/remoteWidgets.spec.ts @@ -30,9 +30,24 @@ test.describe('Remote COMBO Widget', () => { }, nodeName) } + const getWidgetValue = async (comfyPage: ComfyPage, nodeName: string) => { + return await comfyPage.page.evaluate((name) => { + const node = window['app'].graph.nodes.find((node) => node.title === name) + return node.widgets[0].value + }, nodeName) + } + + const clickRefreshButton = (comfyPage: ComfyPage, nodeName: string) => { + return comfyPage.page.evaluate((name) => { + const node = window['app'].graph.nodes.find((node) => node.title === name) + const buttonWidget = node.widgets.find((w) => w.name === 'refresh') + return buttonWidget?.callback() + }, nodeName) + } + const waitForWidgetUpdate = async (comfyPage: ComfyPage) => { // Force re-render to trigger first access of widget's options - await comfyPage.page.mouse.click(100, 100) + await comfyPage.page.mouse.click(400, 300) await comfyPage.page.waitForTimeout(256) } @@ -179,7 +194,7 @@ test.describe('Remote COMBO Widget', () => { const initialOptions = await getWidgetOptions(comfyPage, nodeName) // Wait for the refresh (TTL) to expire - await comfyPage.page.waitForTimeout(302) + await comfyPage.page.waitForTimeout(512) await comfyPage.page.mouse.click(100, 100) const refreshedOptions = await getWidgetOptions(comfyPage, nodeName) @@ -221,14 +236,79 @@ test.describe('Remote COMBO Widget', () => { const nodeName = 'Remote Widget Node' await addRemoteWidgetNode(comfyPage, nodeName) + await waitForWidgetUpdate(comfyPage) - // Wait for a few retries - await comfyPage.page.waitForTimeout(1024) + // Wait for timeout and backoff, then force re-render, repeat + const requestTimeout = 512 + await comfyPage.page.waitForTimeout(requestTimeout) + await waitForWidgetUpdate(comfyPage) + await comfyPage.page.waitForTimeout(requestTimeout * 2) + await waitForWidgetUpdate(comfyPage) + await comfyPage.page.waitForTimeout(requestTimeout * 3) // Verify exponential backoff between retries const intervals = timestamps.slice(1).map((t, i) => t - timestamps[i]) expect(intervals[1]).toBeGreaterThan(intervals[0]) }) + + test('clicking refresh button forces a refresh', async ({ comfyPage }) => { + await comfyPage.page.route( + '**/api/models/checkpoints**', + async (route) => { + await route.fulfill({ + body: JSON.stringify([`${Date.now()}`]), + status: 200 + }) + } + ) + + const nodeName = 'Remote Widget Node With Refresh Button' + + // Trigger initial fetch when adding node to the graph + await addRemoteWidgetNode(comfyPage, nodeName) + await waitForWidgetUpdate(comfyPage) + const initialOptions = await getWidgetOptions(comfyPage, nodeName) + + // Click refresh button + await clickRefreshButton(comfyPage, nodeName) + + // Verify refresh occurred + const refreshedOptions = await getWidgetOptions(comfyPage, nodeName) + expect(refreshedOptions).not.toEqual(initialOptions) + }) + + test('control_after_refresh is applied after refresh', async ({ + comfyPage + }) => { + const options = [ + ['first option', 'second option', 'third option'], + ['new first option', 'first option', 'second option', 'third option'] + ] + await comfyPage.page.route( + '**/api/models/checkpoints**', + async (route) => { + const next = options.shift() + await route.fulfill({ + body: JSON.stringify(next), + status: 200 + }) + } + ) + + const nodeName = + 'Remote Widget Node With Refresh Button and Control After Refresh' + + // Trigger initial fetch when adding node to the graph + await addRemoteWidgetNode(comfyPage, nodeName) + await waitForWidgetUpdate(comfyPage) + + // Click refresh button + await clickRefreshButton(comfyPage, nodeName) + + // Verify the selected value of the widget is the first option in the refreshed list + const refreshedValue = await getWidgetValue(comfyPage, nodeName) + expect(refreshedValue).toEqual('new first option') + }) }) test.describe('Cache Behavior', () => { diff --git a/src/composables/widgets/useRemoteWidget.ts b/src/composables/widgets/useRemoteWidget.ts index 957fe6165..765605ddc 100644 --- a/src/composables/widgets/useRemoteWidget.ts +++ b/src/composables/widgets/useRemoteWidget.ts @@ -1,17 +1,21 @@ +import { LGraphNode } from '@comfyorg/litegraph' +import { IWidget } from '@comfyorg/litegraph' import axios from 'axios' -import { useWidgetStore } from '@/stores/widgetStore' import type { InputSpec, RemoteWidgetConfig } from '@/types/apiTypes' +const MAX_RETRIES = 5 +const TIMEOUT = 4096 + export interface CacheEntry { - data: T[] - timestamp: number - loading: boolean - error: Error | null - fetchPromise?: Promise + data: T + timestamp?: number + error?: Error | null + fetchPromise?: Promise controller?: AbortController - lastErrorTime: number - retryCount: number + lastErrorTime?: number + retryCount?: number + failed?: boolean } const dataCache = new Map>() @@ -27,31 +31,55 @@ const createCacheKey = (config: RemoteWidgetConfig): string => { return [route, `r=${refresh}`, paramsKey].join(';') } -const getBackoff = (retryCount: number) => { - return Math.min(1000 * Math.pow(2, retryCount), 512) -} +const getBackoff = (retryCount: number) => + Math.min(1000 * Math.pow(2, retryCount), 512) -async function fetchData( +const isInitialized = (entry: CacheEntry | undefined) => + entry?.data && entry?.timestamp && entry.timestamp > 0 + +const isStale = (entry: CacheEntry | undefined, ttl: number) => + entry?.timestamp && Date.now() - entry.timestamp >= ttl + +const isFetching = (entry: CacheEntry | undefined) => + entry?.fetchPromise !== undefined + +const isFailed = (entry: CacheEntry | undefined) => + entry?.failed === true + +const isBackingOff = (entry: CacheEntry | undefined) => + entry?.error && + entry?.lastErrorTime && + Date.now() - entry.lastErrorTime < getBackoff(entry.retryCount || 0) + +const fetchData = async ( config: RemoteWidgetConfig, controller: AbortController -): Promise { - const { route, response_key, query_params } = config +) => { + const { route, response_key, query_params, timeout = TIMEOUT } = config const res = await axios.get(route, { params: query_params, signal: controller.signal, - validateStatus: (status) => status === 200 + timeout }) return response_key ? res.data[response_key] : res.data } -export function useRemoteWidget(inputData: InputSpec) { +export function useRemoteWidget< + T extends string | number | boolean | object +>(options: { + inputData: InputSpec + defaultValue: T + node: LGraphNode + widget: IWidget +}) { + const { inputData, defaultValue, node, widget } = options const config: RemoteWidgetConfig = inputData[1].remote - const { refresh = 0 } = config + const { refresh = 0, max_retries = MAX_RETRIES } = config const isPermanent = refresh <= 0 const cacheKey = createCacheKey(config) - const defaultValue = useWidgetStore().getDefaultValue(inputData) + let isLoaded = false - const setSuccess = (entry: CacheEntry, data: T[]) => { + const setSuccess = (entry: CacheEntry, data: T) => { entry.retryCount = 0 entry.lastErrorTime = 0 entry.error = null @@ -64,57 +92,45 @@ export function useRemoteWidget(inputData: InputSpec) { entry.lastErrorTime = Date.now() entry.error = error instanceof Error ? error : new Error(String(error)) entry.data ??= defaultValue - } - - const isInitialized = () => { - const entry = dataCache.get(cacheKey) - return entry?.data && entry.timestamp > 0 - } - - const isStale = () => { - const entry = dataCache.get(cacheKey) - return entry?.timestamp && Date.now() - entry.timestamp >= refresh - } - - const isFetching = () => { - const entry = dataCache.get(cacheKey) - return entry?.fetchPromise - } - - const isBackingOff = () => { - const entry = dataCache.get(cacheKey) - return ( - entry?.error && - entry.lastErrorTime && - Date.now() - entry.lastErrorTime < getBackoff(entry.retryCount) - ) - } - - const fetchOptions = async () => { - const entry = dataCache.get(cacheKey) - - const isValid = isInitialized() && (isPermanent || !isStale()) - if (isValid || isBackingOff()) return entry!.data - if (isFetching()) return entry!.fetchPromise - - const currentEntry: CacheEntry = entry || { - data: defaultValue, - timestamp: 0, - loading: false, - error: null, - fetchPromise: undefined, - controller: undefined, - retryCount: 0, - lastErrorTime: 0 + entry.fetchPromise = undefined + if (entry.retryCount >= max_retries) { + setFailed(entry) } + } + + const setFailed = (entry: CacheEntry) => { + dataCache.set(cacheKey, { + data: entry.data ?? defaultValue, + failed: true + }) + } + + const isFirstLoad = () => { + return !isLoaded && isInitialized(dataCache.get(cacheKey)) + } + + const onFirstLoad = (data: T[]) => { + isLoaded = true + widget.value = data[0] + widget.callback?.(widget.value) + node.graph?.setDirtyCanvas(true) + } + + const fetchValue = async () => { + const entry = dataCache.get(cacheKey) + + if (isFailed(entry)) return entry!.data + + const isValid = + isInitialized(entry) && (isPermanent || !isStale(entry, refresh)) + if (isValid || isBackingOff(entry) || isFetching(entry)) return entry!.data + + const currentEntry: CacheEntry = entry || { data: defaultValue } dataCache.set(cacheKey, currentEntry) try { - currentEntry.loading = true - currentEntry.error = null currentEntry.controller = new AbortController() - - currentEntry.fetchPromise = fetchData(config, currentEntry.controller) + currentEntry.fetchPromise = fetchData(config, currentEntry.controller) const data = await currentEntry.fetchPromise setSuccess(currentEntry, data) @@ -123,21 +139,83 @@ export function useRemoteWidget(inputData: InputSpec) { setError(currentEntry, err) return currentEntry.data } finally { - currentEntry.loading = false currentEntry.fetchPromise = undefined currentEntry.controller = undefined } } + const onRefresh = () => { + if (config.control_after_refresh) { + const data = getCachedValue() + if (!Array.isArray(data)) return // control_after_refresh is only supported for array values + + switch (config.control_after_refresh) { + case 'first': + widget.value = data[0] ?? defaultValue + break + case 'last': + widget.value = data.at(-1) ?? defaultValue + break + } + widget.callback?.(widget.value) + node.graph?.setDirtyCanvas(true) + } + } + + /** + * Clear the widget's cached value, forcing a refresh on next access (e.g., a new render) + */ + const clearCachedValue = () => { + const entry = dataCache.get(cacheKey) + if (!entry) return + if (entry.fetchPromise) entry.controller?.abort() // Abort in-flight request + dataCache.delete(cacheKey) + } + + /** + * Get the cached value of the widget without starting a new fetch. + * @returns the most recently computed value of the widget. + */ + function getCachedValue() { + return dataCache.get(cacheKey)?.data as T + } + + /** + * Getter of the remote property of the widget (e.g., options.values, value, etc.). + * Starts the fetch process then returns the cached value immediately. + * @param onFulfilled - Optional callback to be called when the fetch is resolved. + * @returns the most recent value of the widget. + */ + function getValue(onFulfilled?: () => void) { + fetchValue().then((data) => { + if (isFirstLoad()) onFirstLoad(data) + onFulfilled?.() + }) + return getCachedValue() ?? defaultValue + } + + /** + * Force the widget to refresh its value + */ + function refreshValue() { + clearCachedValue() + getValue(onRefresh) + } + + /** + * Add a refresh button to the node that, when clicked, will force the widget to refresh + */ + function addRefreshButton() { + node.addWidget('button', 'refresh', 'refresh', refreshValue) + } + return { - getCacheKey: () => cacheKey, + getCachedValue, + getValue, + refreshValue, + addRefreshButton, getCacheEntry: () => dataCache.get(cacheKey), - forceUpdate: () => { - const entry = dataCache.get(cacheKey) - if (entry?.fetchPromise) entry.controller?.abort() // Abort in-flight request - dataCache.delete(cacheKey) - }, - fetchOptions, - defaultValue + + cacheKey } } diff --git a/src/scripts/widgets.ts b/src/scripts/widgets.ts index 3d4dd84c1..d22d82507 100644 --- a/src/scripts/widgets.ts +++ b/src/scripts/widgets.ts @@ -281,7 +281,13 @@ export const ComfyWidgets: Record = { } if (remote) { - const remoteWidget = useRemoteWidget(inputData) + const remoteWidget = useRemoteWidget({ + inputData, + defaultValue, + node, + widget: res.widget + }) + if (remote.refresh_button) remoteWidget.addRefreshButton() const origOptions = res.widget.options res.widget.options = new Proxy( @@ -289,22 +295,7 @@ export const ComfyWidgets: Record = { { get(target, prop: string | symbol) { if (prop !== 'values') return target[prop] - - remoteWidget.fetchOptions().then((options) => { - if (!options || !options.length) return - - const isUninitialized = - res.widget.value === remoteWidget.defaultValue && - !res.widget.options.values?.includes(remoteWidget.defaultValue) - if (isUninitialized) { - res.widget.value = options[0] - res.widget.callback?.(options[0]) - node.graph?.setDirtyCanvas(true) - } - }) - - const current = remoteWidget.getCacheEntry() - return current?.data || widgetStore.getDefaultValue(inputData) + return remoteWidget.getValue() } } ) diff --git a/src/types/apiTypes.ts b/src/types/apiTypes.ts index 594dfa282..7222edc56 100644 --- a/src/types/apiTypes.ts +++ b/src/types/apiTypes.ts @@ -273,7 +273,11 @@ const zRemoteWidgetConfig = z.object({ route: z.string().url().or(z.string().startsWith('/')), refresh: z.number().gte(128).safe().or(z.number().lte(0).safe()).optional(), response_key: z.string().optional(), - query_params: z.record(z.string(), z.string()).optional() + query_params: z.record(z.string(), z.string()).optional(), + refresh_button: z.boolean().optional(), + control_after_refresh: z.enum(['first', 'last']).optional(), + timeout: z.number().gte(0).optional(), + max_retries: z.number().gte(0).optional() }) const zBaseInputSpecValue = z diff --git a/tests-ui/tests/composables/widgets/useRemoteWidget.test.ts b/tests-ui/tests/composables/widgets/useRemoteWidget.test.ts index 284a90246..06895bc31 100644 --- a/tests-ui/tests/composables/widgets/useRemoteWidget.test.ts +++ b/tests-ui/tests/composables/widgets/useRemoteWidget.test.ts @@ -21,14 +21,8 @@ jest.mock('@/stores/settingStore', () => ({ }) })) -jest.mock('@/stores/widgetStore', () => ({ - useWidgetStore: () => ({ - widgets: {}, - getDefaultValue: jest.fn().mockReturnValue('Loading...') - }) -})) - const FIRST_BACKOFF = 1000 // backoff is 1s on first retry +const DEFAULT_VALUE = 'Loading...' function createMockInputData(overrides = {}): ComboInputSpecV2 { return [ @@ -44,6 +38,13 @@ function createMockInputData(overrides = {}): ComboInputSpecV2 { ] } +const createMockOptions = (inputOverrides = {}) => ({ + inputData: createMockInputData(inputOverrides), + defaultValue: DEFAULT_VALUE, + node: {} as any, + widget: {} as any +}) + function mockAxiosResponse(data: unknown, status = 200) { jest.mocked(axios.get).mockResolvedValueOnce({ data, status }) } @@ -55,16 +56,25 @@ function mockAxiosError(error: Error | string) { function createHookWithData(data: unknown, inputOverrides = {}) { mockAxiosResponse(data) - const hook = useRemoteWidget(createMockInputData(inputOverrides)) + const hook = useRemoteWidget(createMockOptions(inputOverrides)) return hook } async function setupHookWithResponse(data: unknown, inputOverrides = {}) { const hook = createHookWithData(data, inputOverrides) - const result = await hook.fetchOptions() + const result = await getResolvedValue(hook) return { hook, result } } +async function getResolvedValue(hook: ReturnType) { + // Create a promise that resolves when the fetch is complete + const responsePromise = new Promise((resolve) => { + hook.getValue(() => resolve()) + }) + await responsePromise + return hook.getCachedValue() +} + describe('useRemoteWidget', () => { let mockInputData: ComboInputSpecV2 @@ -86,25 +96,26 @@ describe('useRemoteWidget', () => { describe('initialization', () => { it('should create hook with default values', () => { - const hook = useRemoteWidget(mockInputData) - expect(hook.getCacheEntry()).toBeUndefined() - expect(hook.defaultValue).toBe('Loading...') + const hook = useRemoteWidget(createMockOptions()) + expect(hook.getCachedValue()).toBeUndefined() + expect(hook.getValue()).toBe('Loading...') }) it('should generate consistent cache keys', () => { - const hook1 = useRemoteWidget(mockInputData) - const hook2 = useRemoteWidget(mockInputData) - expect(hook1.getCacheKey()).toBe(hook2.getCacheKey()) + const options = createMockOptions() + const hook1 = useRemoteWidget(options) + const hook2 = useRemoteWidget(options) + expect(hook1.cacheKey).toBe(hook2.cacheKey) }) it('should handle query params in cache key', () => { const hook1 = useRemoteWidget( - createMockInputData({ query_params: { a: 1 } }) + createMockOptions({ query_params: { a: 1 } }) ) const hook2 = useRemoteWidget( - createMockInputData({ query_params: { a: 2 } }) + createMockOptions({ query_params: { a: 2 } }) ) - expect(hook1.getCacheKey()).not.toBe(hook2.getCacheKey()) + expect(hook1.cacheKey).not.toBe(hook2.cacheKey) }) }) @@ -114,7 +125,7 @@ describe('useRemoteWidget', () => { const { hook, result } = await setupHookWithResponse(mockData) expect(result).toEqual(mockData) expect(jest.mocked(axios.get)).toHaveBeenCalledWith( - hook.getCacheKey().split(';')[0], // Get the route part from cache key + hook.cacheKey.split(';')[0], // Get the route part from cache key expect.any(Object) ) }) @@ -140,9 +151,7 @@ describe('useRemoteWidget', () => { const error = new Error('Network error') mockAxiosError(error) - const hook = useRemoteWidget(mockInputData) - const data = await hook.fetchOptions() - expect(data).toBe('Loading...') + const { hook } = await setupHookWithResponse([]) const entry = hook.getCacheEntry() expect(entry?.error).toBeTruthy() @@ -155,26 +164,22 @@ describe('useRemoteWidget', () => { }) it('should handle malformed response data', async () => { - const hook = useRemoteWidget(mockInputData) - const { defaultValue } = hook + const hook = useRemoteWidget(createMockOptions()) mockAxiosResponse(null) - const data1 = await hook.fetchOptions() + const data1 = hook.getValue() mockAxiosResponse(undefined) - const data2 = await hook.fetchOptions() + const data2 = hook.getValue() - expect(data1).toBe(defaultValue) - expect(data2).toBe(defaultValue) + expect(data1).toBe(DEFAULT_VALUE) + expect(data2).toBe(DEFAULT_VALUE) }) it('should handle non-200 status codes', async () => { mockAxiosError('Request failed with status code 404') - const hook = useRemoteWidget(mockInputData) - const data = await hook.fetchOptions() - - expect(data).toBe('Loading...') + const { hook } = await setupHookWithResponse([]) const entry = hook.getCacheEntry() expect(entry?.error?.message).toBe('Request failed with status code 404') }) @@ -195,34 +200,34 @@ describe('useRemoteWidget', () => { const mockData = ['data that is permanent after initialization'] const { hook } = await setupHookWithResponse(mockData) - await hook.fetchOptions() - await hook.fetchOptions() + await getResolvedValue(hook) + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) }) - it('permanent widgets should re-fetch if forceUpdate is called', async () => { + it('permanent widgets should re-fetch if refreshValue is called', async () => { const mockData = ['data that is permanent after initialization'] const { hook } = await setupHookWithResponse(mockData) - await hook.fetchOptions() + await getResolvedValue(hook) const refreshedData = ['data that user forced to be fetched'] mockAxiosResponse(refreshedData) - await hook.forceUpdate() - const data = await hook.fetchOptions() + hook.refreshValue() + const data = await getResolvedValue(hook) expect(data).toEqual(refreshedData) }) it('permanent widgets should still retry if request fails', async () => { mockAxiosError('Network error') - const hook = useRemoteWidget(mockInputData) - await hook.fetchOptions() + const hook = useRemoteWidget(createMockOptions()) + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) jest.setSystemTime(Date.now() + FIRST_BACKOFF) - const secondData = await hook.fetchOptions() + const secondData = await getResolvedValue(hook) expect(secondData).toBe('Loading...') expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(2) }) @@ -230,8 +235,8 @@ describe('useRemoteWidget', () => { it('should treat empty refresh field as permanent', async () => { const { hook } = await setupHookWithResponse(['data that is permanent']) - await hook.fetchOptions() - await hook.fetchOptions() + await getResolvedValue(hook) + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) }) @@ -246,7 +251,7 @@ describe('useRemoteWidget', () => { mockAxiosResponse(mockData2) jest.setSystemTime(Date.now() + refresh) - const newData = await hook.fetchOptions() + const newData = await getResolvedValue(hook) expect(newData).toEqual(mockData2) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(2) @@ -258,7 +263,7 @@ describe('useRemoteWidget', () => { }) jest.setSystemTime(Date.now() + 128) - await hook.fetchOptions() + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) }) @@ -271,12 +276,12 @@ describe('useRemoteWidget', () => { mockAxiosError('Network error') jest.setSystemTime(Date.now() + refresh) - await hook.fetchOptions() + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(2) mockAxiosResponse(['second success']) jest.setSystemTime(Date.now() + FIRST_BACKOFF) - const thirdData = await hook.fetchOptions() + const thirdData = await getResolvedValue(hook) expect(thirdData).toEqual(['second success']) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(3) }) @@ -289,7 +294,7 @@ describe('useRemoteWidget', () => { mockAxiosError('Network error') jest.setSystemTime(Date.now() + refresh) - const secondData = await hook.fetchOptions() + const secondData = await getResolvedValue(hook) expect(secondData).toEqual(['a valid value']) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(2) @@ -308,33 +313,33 @@ describe('useRemoteWidget', () => { it('should implement exponential backoff on errors', async () => { mockAxiosError('Network error') - const hook = useRemoteWidget(mockInputData) - await hook.fetchOptions() + const hook = useRemoteWidget(createMockOptions()) + await getResolvedValue(hook) const entry1 = hook.getCacheEntry() expect(entry1?.error).toBeTruthy() - await hook.fetchOptions() + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) jest.setSystemTime(Date.now() + 500) - await hook.fetchOptions() + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) // Still backing off jest.setSystemTime(Date.now() + 3000) - await hook.fetchOptions() + await getResolvedValue(hook) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(2) expect(entry1?.data).toBeDefined() }) it('should reset error state on successful fetch', async () => { mockAxiosError('Network error') - const hook = useRemoteWidget(mockInputData) - const firstData = await hook.fetchOptions() + const hook = useRemoteWidget(createMockOptions()) + const firstData = await getResolvedValue(hook) expect(firstData).toBe('Loading...') jest.setSystemTime(Date.now() + 3000) mockAxiosResponse(['option1']) - const secondData = await hook.fetchOptions() + const secondData = await getResolvedValue(hook) expect(secondData).toEqual(['option1']) const entry = hook.getCacheEntry() @@ -344,14 +349,14 @@ describe('useRemoteWidget', () => { it('should save successful data after backoff', async () => { mockAxiosError('Network error') - const hook = useRemoteWidget(mockInputData) - await hook.fetchOptions() + const hook = useRemoteWidget(createMockOptions()) + await getResolvedValue(hook) const entry1 = hook.getCacheEntry() expect(entry1?.error).toBeTruthy() jest.setSystemTime(Date.now() + 3000) mockAxiosResponse(['success after backoff']) - const secondData = await hook.fetchOptions() + const secondData = await getResolvedValue(hook) expect(secondData).toEqual(['success after backoff']) const entry2 = hook.getCacheEntry() @@ -363,24 +368,24 @@ describe('useRemoteWidget', () => { mockAxiosError('Network error') mockAxiosError('Network error') mockAxiosError('Network error') - const hook = useRemoteWidget(mockInputData) - await hook.fetchOptions() + const hook = useRemoteWidget(createMockOptions()) + await getResolvedValue(hook) const entry1 = hook.getCacheEntry() expect(entry1?.error).toBeTruthy() jest.setSystemTime(Date.now() + 3000) - const secondData = await hook.fetchOptions() + const secondData = await getResolvedValue(hook) expect(secondData).toBe('Loading...') expect(entry1?.error).toBeDefined() jest.setSystemTime(Date.now() + 9000) - const thirdData = await hook.fetchOptions() + const thirdData = await getResolvedValue(hook) expect(thirdData).toBe('Loading...') expect(entry1?.error).toBeDefined() jest.setSystemTime(Date.now() + 120_000) mockAxiosResponse(['success after multiple backoffs']) - const fourthData = await hook.fetchOptions() + const fourthData = await getResolvedValue(hook) expect(fourthData).toEqual(['success after multiple backoffs']) const entry2 = hook.getCacheEntry() @@ -392,20 +397,20 @@ describe('useRemoteWidget', () => { describe('cache management', () => { it('should clear cache entries', async () => { const { hook } = await setupHookWithResponse(['to be cleared']) - expect(hook.getCacheEntry()).toBeDefined() + expect(hook.getCachedValue()).toBeDefined() - hook.forceUpdate() - expect(hook.getCacheEntry()).toBeUndefined() + hook.refreshValue() + expect(hook.getCachedValue()).toBe(DEFAULT_VALUE) }) it('should prevent duplicate in-flight requests', async () => { const promise = Promise.resolve({ data: ['non-duplicate'] }) jest.mocked(axios.get).mockImplementationOnce(() => promise) - const hook = useRemoteWidget(mockInputData) + const hook = useRemoteWidget(createMockOptions()) const [result1, result2] = await Promise.all([ - hook.fetchOptions(), - hook.fetchOptions() + getResolvedValue(hook), + getResolvedValue(hook) ]) expect(result1).toBe(result2) @@ -416,40 +421,43 @@ describe('useRemoteWidget', () => { describe('concurrent access and multiple instances', () => { it('should handle concurrent hook instances with same route', async () => { mockAxiosResponse(['shared data']) - const hook1 = useRemoteWidget(mockInputData) - const hook2 = useRemoteWidget(mockInputData) + const options = createMockOptions() + const hook1 = useRemoteWidget(options) + const hook2 = useRemoteWidget(options) - const [data1, data2] = await Promise.all([ - hook1.fetchOptions(), - hook2.fetchOptions() - ]) + // Since they have the same route, only one request will be made + await Promise.race([getResolvedValue(hook1), getResolvedValue(hook2)]) + + const data1 = hook1.getValue() + const data2 = hook2.getValue() expect(data1).toEqual(['shared data']) expect(data2).toEqual(['shared data']) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) - expect(hook1.getCacheEntry()).toBe(hook2.getCacheEntry()) + expect(hook1.getCachedValue()).toBe(hook2.getCachedValue()) }) it('should use shared cache across multiple hooks', async () => { mockAxiosResponse(['shared data']) - const hook1 = useRemoteWidget(mockInputData) - const hook2 = useRemoteWidget(mockInputData) - const hook3 = useRemoteWidget(mockInputData) - const hook4 = useRemoteWidget(mockInputData) + const options = createMockOptions() + const hook1 = useRemoteWidget(options) + const hook2 = useRemoteWidget(options) + const hook3 = useRemoteWidget(options) + const hook4 = useRemoteWidget(options) - const data1 = await hook1.fetchOptions() - const data2 = await hook2.fetchOptions() - const data3 = await hook3.fetchOptions() - const data4 = await hook4.fetchOptions() + const data1 = await getResolvedValue(hook1) + const data2 = await getResolvedValue(hook2) + const data3 = await getResolvedValue(hook3) + const data4 = await getResolvedValue(hook4) expect(data1).toEqual(['shared data']) expect(data2).toBe(data1) expect(data3).toBe(data1) expect(data4).toBe(data1) expect(jest.mocked(axios.get)).toHaveBeenCalledTimes(1) - expect(hook1.getCacheEntry()).toBe(hook2.getCacheEntry()) - expect(hook2.getCacheEntry()).toBe(hook3.getCacheEntry()) - expect(hook3.getCacheEntry()).toBe(hook4.getCacheEntry()) + expect(hook1.getCachedValue()).toBe(hook2.getCachedValue()) + expect(hook2.getCachedValue()).toBe(hook3.getCachedValue()) + expect(hook3.getCachedValue()).toBe(hook4.getCachedValue()) }) it('should handle rapid cache clearing during fetch', async () => { @@ -460,15 +468,17 @@ describe('useRemoteWidget', () => { jest.mocked(axios.get).mockImplementationOnce(() => delayedPromise) - const hook = useRemoteWidget(mockInputData) - const fetchPromise = hook.fetchOptions() - hook.forceUpdate() + const hook = useRemoteWidget(createMockOptions()) + hook.getValue() + hook.refreshValue() resolvePromise!({ data: ['delayed data'] }) - const data = await fetchPromise + const data = await getResolvedValue(hook) - expect(data).toEqual(['delayed data']) - expect(hook.getCacheEntry()).toBeUndefined() + // The value should be the default value because the refreshValue + // clears the cache and the fetch is aborted + expect(data).toEqual(DEFAULT_VALUE) + expect(hook.getCachedValue()).toBe(DEFAULT_VALUE) }) it('should handle widget destroyed during fetch', async () => { @@ -479,8 +489,8 @@ describe('useRemoteWidget', () => { jest.mocked(axios.get).mockImplementationOnce(() => delayedPromise) - let hook = useRemoteWidget(mockInputData) - const fetchPromise = hook.fetchOptions() + let hook = useRemoteWidget(createMockOptions()) + const fetchPromise = hook.getValue() hook = null as any @@ -488,10 +498,10 @@ describe('useRemoteWidget', () => { await fetchPromise expect(hook).toBeNull() - hook = useRemoteWidget(mockInputData) + hook = useRemoteWidget(createMockOptions()) - const data2 = await hook.fetchOptions() - expect(data2).toEqual(['delayed data']) + const data2 = await getResolvedValue(hook) + expect(data2).toEqual(DEFAULT_VALUE) }) }) })