fix usePageQueryParams accessing stale value (#940)

Author: Assem-Uber

package-lock.json

@@ -87,7 +87,7 @@
     "jest-environment-jsdom": "^29.7.0",
     "jest-fixed-jsdom": "^0.0.8",
     "msw": "^2.6.0",
-    "next-router-mock": "^0.9.13",
+    "next-router-mock": "^1.0.2",
     "pino-pretty": "^11.2.2",
     "prettier": "3.2.5",
     "styletron-engine-snapshot": "^1.0.2",

package.json

@@ -1,33 +1,23 @@
-import * as nextRouterMock from 'next-router-mock';
-
 import { renderHook, act } from '@/test-utils/rtl';
 
 import { queryParamsConfig } from '../__fixtures__/page-query-params.fixtures';
 import usePageQueryParams from '../use-page-query-params';
 
-jest.mock('next/router', () => nextRouterMock);
-
-//TODO: @assem.hafez refactor all next/navigations to use the same mock
-jest.mock('next/navigation', () => {
-  const { useRouter } = nextRouterMock;
-  const usePathname = () => {
-    const router = useRouter();
-    return router.pathname;
-  };
+jest.mock('next/navigation', () => require('next-router-mock/navigation'));
 
-  const useSearchParams = () => {
-    const router = useRouter();
-    return new URLSearchParams(router.asPath); // use asPath to get the url as set by our implementation, using router.query or router.search return multivalues in a different format than the actual url
-  };
+describe('usePageQueryParams', () => {
+  const originalWindowLocation = window.location;
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
 
-  return {
-    useRouter,
-    usePathname,
-    useSearchParams,
-  };
-});
+  afterEach(() => {
+    Object.defineProperty(window, 'location', {
+      value: originalWindowLocation,
+      writable: true,
+    });
+  });
 
-describe('usePageQueryParams', () => {
   it('should return default values when search is empty', () => {
     const { result } = renderHook(() => usePageQueryParams(queryParamsConfig));
     const [values] = result.current;
@@ -40,6 +30,7 @@ describe('usePageQueryParams', () => {
       multiValDefaulted: ['a'],
     });
   });
+
   it('should update values by calling the single key setter method', async () => {
     const { result } = renderHook(() => usePageQueryParams(queryParamsConfig));
     const [, setValues] = result.current;
@@ -84,4 +75,26 @@ describe('usePageQueryParams', () => {
       multiValDefaulted: ['a', 'a'],
     });
   });
+
+  it('should get search values from window.location.search on first client side render', async () => {
+    Object.defineProperty(window, 'location', {
+      value: {
+        ...originalWindowLocation,
+        search: '?sortBy=sortByValue1',
+      },
+      writable: true,
+    });
+
+    const { result } = renderHook(() => usePageQueryParams(queryParamsConfig));
+    const [values] = result.current;
+
+    expect(values).toStrictEqual({
+      sortBy: 'sortByValue1',
+      aliased: undefined,
+      defaulted: 'defaultValue',
+      parsed: undefined,
+      parsedMultiVal: undefined,
+      multiValDefaulted: ['a'],
+    });
+  });
 });

src/hooks/use-page-query-params/__tests__/use-page-query-params.test.tsx

@@ -31,10 +31,14 @@ export default function usePageQueryParams<P extends PageQueryParams>(
   const prevSearchQueryParam = usePreviousValue(searchQueryParams);
 
   const search = useMemo(() => {
-    // get changed value from searchQueryParams if it was changed
+    // get changed value from searchQueryParams if it was changed or in case of server side rendering
     // otherwise change would be due history state change and search value is available in window.location.search
-    if (prevSearchQueryParam !== searchQueryParams)
+    if (
+      typeof window === 'undefined' ||
+      (prevSearchQueryParam && prevSearchQueryParam !== searchQueryParams)
+    )
       return searchQueryParams.toString();
+
     return window.location.search;
     // stateUrl is needed in deps to recalculate window.location.search
     // eslint-disable-next-line react-hooks/exhaustive-deps

src/hooks/use-page-query-params/use-page-query-params.tsx

@@ -5,7 +5,7 @@ import {
   type QueryClientConfig,
   QueryClientProvider,
 } from '@tanstack/react-query';
-import { MemoryRouterProvider } from 'next-router-mock/MemoryRouterProvider';
+import { MemoryRouterProvider } from 'next-router-mock/MemoryRouterProvider/next-13.5'; // TODO: @assem.hafez remove the dependency on next-router-mock
 // @ts-expect-error Could not find a declaration file for module 'styletron-engine-snapshot'
 import { StyletronSnapshotEngine } from 'styletron-engine-snapshot';
 
@@ -15,7 +15,7 @@ import StyletronProvider from '@/providers/styletron-provider';
 import MSWMockHandlers from './msw-mock-handlers/msw-mock-handlers';
 import { type Props } from './test-provider.types';
 
-jest.mock('next/router', () => jest.requireActual('next-router-mock'));
+jest.mock('next/router', () => require('next-router-mock'));
 
 const snapshotEngine = new StyletronSnapshotEngine();