test: Improve useLocationQuery with tests and types (#57823)

<!-- Describe your PR here. -->

static/app/components/feedback/list/feedbackListItem.tsx

@@ -13,10 +13,7 @@ import {IconPlay} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {trackAnalytics} from 'sentry/utils/analytics';
-import {
-  FeedbackItemLoaderQueryParams,
-  HydratedFeedbackItem,
-} from 'sentry/utils/feedback/item/types';
+import {HydratedFeedbackItem} from 'sentry/utils/feedback/item/types';
 import {decodeScalar} from 'sentry/utils/queryString';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -45,10 +42,10 @@ function UnreadBadge() {
 }
 
 function useIsSelectedFeedback({feedbackItem}: {feedbackItem: HydratedFeedbackItem}) {
-  const {feedbackSlug} = useLocationQuery<FeedbackItemLoaderQueryParams>({
+  const {feedbackSlug} = useLocationQuery({
     fields: {feedbackSlug: decodeScalar},
   });
-  const [, feedbackId] = feedbackSlug?.split(':') ?? [];
+  const [, feedbackId] = feedbackSlug.split(':') ?? [];
   return feedbackId === feedbackItem.feedback_id;
 }
 

static/app/components/feedback/useFeedbackListQueryView.tsx

@@ -7,7 +7,7 @@ export default function useFeedbackListQueryView({
 }: {
   queryReferrer: string;
 }): QueryView {
-  return useLocationQuery<QueryView>({
+  return useLocationQuery({
     fields: {
       queryReferrer,
       end: decodeScalar,

static/app/utils/url/useLocationQuery.spec.tsx

@@ -0,0 +1,151 @@
+import type {Location} from 'history';
+
+import {reactHooks} from 'sentry-test/reactTestingLibrary';
+
+import {decodeInteger, decodeList, decodeScalar} from 'sentry/utils/queryString';
+import useLocationQuery from 'sentry/utils/url/useLocationQuery';
+import {useLocation} from 'sentry/utils/useLocation';
+
+jest.mock('sentry/utils/useLocation');
+
+const mockLocation: Location = {
+  key: '',
+  search: '',
+  hash: '',
+  action: 'PUSH',
+  state: null,
+  query: {},
+  pathname: '/mock-pathname/',
+};
+
+describe('useLocationQuery', () => {
+  it('should read query values from the url', () => {
+    jest.mocked(useLocation).mockReturnValue({
+      ...mockLocation,
+      query: {
+        name: 'Adam',
+        age: '12',
+        titles: ['Mr.', 'Dr.'],
+        extra: 'foo bar',
+      },
+    } as Location);
+
+    const {result} = reactHooks.renderHook(useLocationQuery, {
+      initialProps: {
+        fields: {
+          name: decodeScalar,
+          age: decodeInteger,
+          titles: decodeList,
+        },
+      },
+    });
+
+    expect(result.current).toStrictEqual({
+      name: 'Adam',
+      age: 12,
+      titles: ['Mr.', 'Dr.'],
+    });
+  });
+
+  it('should return undefined if the url does not contain a requested field', () => {
+    jest.mocked(useLocation).mockReturnValue({
+      ...mockLocation,
+      query: {},
+    } as Location);
+
+    const {result} = reactHooks.renderHook(useLocationQuery, {
+      initialProps: {
+        fields: {
+          name: decodeScalar,
+          age: decodeInteger,
+          titles: decodeList,
+        },
+      },
+    });
+
+    expect(result.current).toStrictEqual({
+      name: '',
+      age: 0,
+      titles: [],
+    });
+  });
+
+  it('should pass-through static values along with decoded ones', () => {
+    jest.mocked(useLocation).mockReturnValueOnce({
+      ...mockLocation,
+      query: {
+        name: 'Adam',
+        titles: ['Mr.', 'Dr.'],
+      },
+    } as Location);
+
+    const {result} = reactHooks.renderHook(useLocationQuery, {
+      initialProps: {
+        fields: {
+          name: decodeScalar,
+          stringy: 'bar',
+          list: ['biz', 'baz'],
+          num: 12,
+        },
+      },
+    });
+
+    expect(result.current).toStrictEqual({
+      name: 'Adam',
+      stringy: 'bar',
+      list: ['biz', 'baz'],
+      num: 12,
+    });
+  });
+
+  it('should only change return object identity when values change', () => {
+    // 1st render:
+    jest.mocked(useLocation).mockReturnValueOnce({
+      ...mockLocation,
+      query: {
+        name: 'Adam',
+        titles: ['Mr.', 'Dr.'],
+      },
+    } as Location);
+    // 2nd render, same values (but the array is re-built, new object ref):
+    jest.mocked(useLocation).mockReturnValueOnce({
+      ...mockLocation,
+      query: {
+        name: 'Adam',
+        titles: ['Mr.', 'Dr.'],
+      },
+    } as Location);
+    // 3rd render, name is changed.
+    jest.mocked(useLocation).mockReturnValueOnce({
+      ...mockLocation,
+      query: {
+        name: 'Betty',
+      },
+    } as Location);
+
+    const {result, rerender} = reactHooks.renderHook(useLocationQuery, {
+      initialProps: {
+        fields: {
+          name: decodeScalar,
+          age: decodeInteger,
+          titles: decodeList,
+        },
+      },
+    });
+    const first = result.current;
+    rerender();
+    const second = result.current;
+    rerender();
+    const third = result.current;
+
+    expect(first.name).toBe('Adam');
+    expect(second.name).toBe('Adam');
+    expect(third.name).toBe('Betty');
+
+    // Must be strict object equality:
+    expect(first).toBe(second);
+
+    // Object has changed because a value has changed:
+    expect(first).not.toBe(third);
+  });
+});

static/app/utils/url/useLocationQuery.tsx

@@ -10,30 +10,52 @@ type Decoder = typeof decodeList | typeof decodeScalar | typeof decodeInteger;
  * Select and memoize query params from location.
  * This returns a new object only when one of the specified query fields is
  * updated. The object will remain stable otherwise, avoiding re-renders.
+ *
+ * You shouldn't need to manually set the `InferredRequestShape` or `InferredResponseShape`
+ * generics, instead type the left side of the statement.
+ *
+ * For example:
+ * ```
+ * type QueryFields = {statsPeriod: string};
+ * const query: QueryFields = useLocationQuery({
+ *   fields: {statsPeriod: decodeScalar}
+ * });
+ * ```
  */
 export default function useLocationQuery<
-  Fields extends Record<string, Scalar | Scalar[]>,
->({fields}: {fields: Record<keyof Fields, Fields[string] | Decoder>}): Fields {
+  InferredRequestShape extends Record<string, Scalar | Scalar[] | Decoder>,
+  InferredResponseShape extends {
+    readonly [Property in keyof InferredRequestShape]: InferredRequestShape[Property] extends Decoder
+      ? ReturnType<InferredRequestShape[Property]>
+      : InferredRequestShape[Property];
+  },
+>({fields}: {fields: InferredRequestShape}): InferredResponseShape {
   const location = useLocation();
 
   const locationFields = {};
-  const staticFields = {};
+  const forwardedFields = {};
   Object.entries(fields).forEach(([field, decoderOrValue]) => {
     if (typeof decoderOrValue === 'function') {
-      locationFields[field] = decoderOrValue(location.query[field]);
+      if (decoderOrValue === decodeScalar) {
+        locationFields[field] = decoderOrValue(location.query[field], '');
+      } else if (decoderOrValue === decodeInteger) {
+        locationFields[field] = decoderOrValue(location.query[field], 0);
+      } else {
+        locationFields[field] = decoderOrValue(location.query[field]);
+      }
     } else {
-      staticFields[field] = decoderOrValue;
+      forwardedFields[field] = decoderOrValue;
     }
   }, {});
 
-  const stringyFields = JSON.stringify(locationFields);
-  const objFields = useMemo(() => JSON.parse(stringyFields), [stringyFields]);
+  const stringyForwardedFields = JSON.stringify(forwardedFields);
+  const stringyLocationFields = JSON.stringify(locationFields);
 
   return useMemo(
     () => ({
-      ...objFields,
-      ...staticFields,
+      ...(forwardedFields as any),
+      ...(locationFields as any),
     }),
-    [objFields] // eslint-disable-line react-hooks/exhaustive-deps
+    [stringyForwardedFields, stringyLocationFields] // eslint-disable-line
   );
 }