Track previous fetch data in usePolledAPIFetch (#6696)

lms/static/scripts/frontend_apps/utils/api.ts

@@ -259,6 +259,9 @@ export function usePolledAPIFetch<T = unknown>({
   // 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<ReturnType<typeof _setTimeout> | null>(null);
   const resetTimeout = useCallback(() => {
@@ -269,6 +272,10 @@ export function usePolledAPIFetch<T = unknown>({
   }, [_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<T = unknown>({
     // 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<T = unknown>({
 
   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,
   };
 }

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 (
-      <div data-testid="main-content">
+      <div data-testid="main-content" data-status={status}>
         {result.isLoading && 'Loading'}
         {!result.isLoading && 'Loaded'}
       </div>
@@ -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());
   });
 });