diff --git a/lms/static/scripts/frontend_apps/utils/api.ts b/lms/static/scripts/frontend_apps/utils/api.ts index a4b1c382ba..ef851a88a9 100644 --- a/lms/static/scripts/frontend_apps/utils/api.ts +++ b/lms/static/scripts/frontend_apps/utils/api.ts @@ -259,6 +259,9 @@ export function usePolledAPIFetch({ // Track loading state separately, to avoid flickering UIs due to // intermediate state changes between refreshes const [isLoading, setIsLoading] = useState(result.isLoading); + // Track data from previous fetch, so that we can expose it and differentiate + // the first fetch from subsequent refreshes. + const [prevData, setPrevData] = useState(result.data); const timeout = useRef | null>(null); const resetTimeout = useCallback(() => { @@ -269,6 +272,10 @@ export function usePolledAPIFetch({ }, [_clearTimeout]); useEffect(() => { + if (result.data) { + setPrevData(result.data); + } + // When the actual request is loading, transition to loading, in case // a new request was triggered by a path/fetch key change if (result.isLoading) { @@ -279,6 +286,9 @@ export function usePolledAPIFetch({ // Transition to not-loading only once we no longer have to refresh if (!shouldRefresh(result)) { setIsLoading(false); + // Reset prev data once we finish refreshing, so that we don't + // accidentally leak data from a different request on subsequent ones + setPrevData(null); return () => {}; } @@ -294,6 +304,9 @@ export function usePolledAPIFetch({ return { ...result, + // If the fetch result data is available, return it. Otherwise, fall back + // to the data loaded in previous fetch + data: result.data ?? prevData, isLoading, }; } diff --git a/lms/static/scripts/frontend_apps/utils/test/api-test.js b/lms/static/scripts/frontend_apps/utils/test/api-test.js index 756954c034..b22fc1b908 100644 --- a/lms/static/scripts/frontend_apps/utils/test/api-test.js +++ b/lms/static/scripts/frontend_apps/utils/test/api-test.js @@ -1,5 +1,6 @@ import { delay, waitFor } from '@hypothesis/frontend-testing'; import { mount } from 'enzyme'; +import { useMemo } from 'preact/hooks'; import { Config } from '../../config'; import { APIError } from '../../errors'; @@ -700,9 +701,19 @@ describe('usePolledAPIFetch', () => { _setTimeout: callback => setTimeout(callback), _clearTimeout: fakeClearTimeout, }); + const status = useMemo(() => { + if (result.isLoading && !result.data) { + return 'loading'; + } + if (result.isLoading && result.data) { + return 'refreshing'; + } + + return 'loaded'; + }, [result.data, result.isLoading]); return ( -
+
{result.isLoading && 'Loading'} {!result.isLoading && 'Loaded'}
@@ -755,19 +766,26 @@ describe('usePolledAPIFetch', () => { const wrapper = createComponent(shouldRefresh); const isLoading = () => wrapper.find('[data-testid="main-content"]').text() === 'Loading'; + const isRefreshing = () => + wrapper.find('[data-testid="main-content"]').prop('data-status') === + 'refreshing'; + // Initially, `isLoading` is `true`, and we don't have access to `data`. assert.isTrue(isLoading()); + assert.isFalse(isRefreshing()); - // Even if we make the API call finish, the status will continue being - // loading as long as shouldRefresh returns true + // If we make the API call finish, `isLoading` will continue being `true` + // as long as shouldRefresh returns true, but now we have access to `data`. mockFetchFinished(); reRender(wrapper); assert.isTrue(isLoading()); + assert.isTrue(isRefreshing()); // Once shouldRefresh returns false, the state will transition to // `isLoading: false` shouldRefresh.returns(false); reRender(wrapper); assert.isFalse(isLoading()); + assert.isFalse(isRefreshing()); }); });