fix: stop the course outline scrolling back while you work in it [backport to verawood]

src/data/apiHooks.test.tsx

@@ -1,11 +1,14 @@
+import { useRef } from 'react';
+import userEvent from '@testing-library/user-event';
+
 import {
   initializeMocks,
   cleanup,
   screen,
   render,
   waitFor,
 } from '../testUtils';
-import { useWaffleFlags } from './apiHooks';
+import { createGlobalState, useWaffleFlags } from './apiHooks';
 import { getApiWaffleFlagsUrl } from './api';
 
 // A little component for testing our waffle flag hooks.
@@ -110,3 +113,60 @@ describe('useWaffleFlags', () => {
     expect(await screen.findByLabelText('useNewCourseOutlinePage')).toHaveTextContent('enabled');
   });
 });
+
+// A little component for testing the global state hooks.
+const useCounter = createGlobalState<{ count: number; }>(() => ['test', 'counter'], { count: 0 });
+
+const CounterComponent = () => {
+  const { data, setData, resetData } = useCounter();
+  const firstSetData = useRef(setData);
+  const firstResetData = useRef(resetData);
+  const callbacksKeptIdentity = setData === firstSetData.current && resetData === firstResetData.current;
+
+  return (
+    <ul>
+      <li aria-label="count">{data?.count ?? 'none'}</li>
+      <li aria-label="callbacks">{callbacksKeptIdentity ? 'same' : 'recreated'}</li>
+      <li>
+        <button type="button" onClick={() => setData({ count: (data?.count ?? 0) + 1 })}>increment</button>
+      </li>
+      <li>
+        <button
+          type="button"
+          onClick={() => {
+            void resetData();
+          }}
+        >
+          reset
+        </button>
+      </li>
+    </ul>
+  );
+};
+
+describe('createGlobalState', () => {
+  it('keeps its callbacks across renders, so effects depending on them do not re-run', async () => {
+    const user = userEvent.setup();
+    initializeMocks();
+    render(<CounterComponent />);
+    await waitFor(() => expect(screen.getByLabelText('count')).toHaveTextContent('0'));
+
+    await user.click(screen.getByRole('button', { name: 'increment' }));
+    await waitFor(() => expect(screen.getByLabelText('count')).toHaveTextContent('1'));
+
+    expect(screen.getByLabelText('callbacks')).toHaveTextContent('same');
+  });
+
+  it('resets the value it stores', async () => {
+    const user = userEvent.setup();
+    initializeMocks();
+    render(<CounterComponent />);
+    await waitFor(() => expect(screen.getByLabelText('count')).toHaveTextContent('0'));
+
+    await user.click(screen.getByRole('button', { name: 'increment' }));
+    await waitFor(() => expect(screen.getByLabelText('count')).toHaveTextContent('1'));
+
+    await user.click(screen.getByRole('button', { name: 'reset' }));
+    await waitFor(() => expect(screen.getByLabelText('count')).toHaveTextContent('0'));
+  });
+});

src/data/apiHooks.ts

@@ -1,3 +1,5 @@
+import { useCallback, useMemo } from 'react';
+
 import { getConfig } from '@edx/frontend-platform';
 import { getAuthenticatedUser } from '@edx/frontend-platform/auth';
 import { UserAgreement, UserAgreementRecord } from '@src/data/types';
@@ -146,15 +148,20 @@ export const useCourseDetails = (courseId: string) => {
 };
 
 /**
- * Create a global state function for a query.
+ * Create a global state function.
+ *
+ * This is sort of a hack for sharing state between components using React Query
+ * without actually loading data from an API. Instead of using this, please use
+ * a small context or `useSyncExternalStore`, and feel free to remove this once
+ * we've replaced the existing usage of it on the outline page.
  */
 export function createGlobalState<T>(
   queryKeyFn: (queryKeyArgs?: any) => unknown[],
   initialData: T | null = null,
 ) {
   return (queryKeyArgs?: any) => {
     const queryClient = useQueryClient();
-    const queryKey = queryKeyFn(queryKeyArgs);
+    const queryKey = useMemo(() => queryKeyFn(queryKeyArgs), [queryKeyArgs]);
 
     const { data } = useQuery({
       queryKey,
@@ -166,15 +173,13 @@ export function createGlobalState<T>(
       refetchIntervalInBackground: false,
     });
 
-    function setData(x: Partial<T>) {
+    const setData = useCallback((x: Partial<T>) => {
       queryClient.setQueryData(queryKey, x);
-    }
+    }, [queryClient, queryKey]);
 
-    async function resetData() {
-      await queryClient.invalidateQueries({
-        queryKey,
-      });
-    }
+    const resetData = useCallback(async () => {
+      await queryClient.invalidateQueries({ queryKey });
+    }, [queryClient, queryKey]);
 
     return { data, setData, resetData };
   };