Browse Source

ref(perf): Start replacing *DiscoverQuery with RQ (#38025)

This is an example of replacing our fetch-as-component pattern with
hooks, as was eventually intended. In this case using React Query to try
it out on the performance part of the product, since almost every single
request we make is through *DiscoverQuery so it would be easy to test it
out with this hook and swap up React Query for another approach under
the hood if we'd like to at a later point.

Since this is touching our bundle sizes, you know the drill:
- Around ~**10kb** for pages that choose to use this chunk. Given our low
correlation of asset size to lcp value this should in theory have no
impact on our LCP etc. for these pages.
Kev 2 years ago
parent
commit
d7e0b08f84

+ 1 - 0
package.json

@@ -41,6 +41,7 @@
     "@sentry/release-parser": "^1.3.1",
     "@sentry/tracing": "7.13.0",
     "@sentry/utils": "7.13.0",
+    "@tanstack/react-query": "^4.2.1",
     "@testing-library/jest-dom": "^5.16.4",
     "@testing-library/react": "^12.1.2",
     "@testing-library/react-hooks": "^8.0.1",

+ 22 - 2
static/app/utils/discover/discoverQuery.tsx

@@ -1,10 +1,10 @@
 import {EventsMetaType, MetaType} from 'sentry/utils/discover/eventView';
-import withApi from 'sentry/utils/withApi';
 import {TransactionThresholdMetric} from 'sentry/views/performance/transactionSummary/transactionThresholdModal';
 
 import GenericDiscoverQuery, {
   DiscoverQueryProps,
   GenericChildrenProps,
+  useGenericDiscoverQuery,
 } from './genericDiscoverQuery';
 
 /**
@@ -76,4 +76,24 @@ function DiscoverQuery(props: DiscoverQueryComponentProps) {
   );
 }
 
-export default withApi(DiscoverQuery);
+export function useDiscoverQuery(props: Omit<DiscoverQueryComponentProps, 'children'>) {
+  const endpoint = props.useEvents ? 'events' : 'eventsv2';
+  const afterFetch = props.useEvents
+    ? (data, _) => {
+        const {fields, ...otherMeta} = data.meta ?? {};
+        return {
+          ...data,
+          meta: {...fields, ...otherMeta},
+        };
+      }
+    : undefined;
+
+  return useGenericDiscoverQuery<TableData, DiscoverQueryPropsWithThresholds>({
+    route: endpoint,
+    shouldRefetchData,
+    afterFetch,
+    ...props,
+  });
+}
+
+export default DiscoverQuery;

+ 54 - 32
static/app/utils/discover/genericDiscoverQuery.tsx

@@ -1,4 +1,5 @@
 import {Component, useContext} from 'react';
+import {useQuery} from '@tanstack/react-query';
 import {Location} from 'history';
 
 import {EventQuery} from 'sentry/actionCreators/events';
@@ -13,6 +14,8 @@ import {QueryBatching} from 'sentry/utils/performance/contexts/genericQueryBatch
 import {PerformanceEventViewContext} from 'sentry/utils/performance/contexts/performanceEventViewContext';
 import {OrganizationContext} from 'sentry/views/organizationContext';
 
+import useApi from '../useApi';
+
 export class QueryError {
   message: string;
   private originalError: any; // For debugging in case parseError picks a value that doesn't make sense.
@@ -51,7 +54,6 @@ type OptionalContextProps = {
 };
 
 type BaseDiscoverQueryProps = {
-  api: Client;
   /**
    * Used as the default source for cursor values.
    */
@@ -142,6 +144,7 @@ type Props<T, P> = InnerRequestProps<P> & ReactProps<T> & ComponentProps<T, P>;
 type OuterProps<T, P> = OuterRequestProps<P> & ReactProps<T> & ComponentProps<T, P>;
 
 type State<T> = {
+  api: Client;
   tableFetchID: symbol | undefined;
 } & GenericChildrenProps<T>;
 
@@ -156,6 +159,7 @@ class _GenericDiscoverQuery<T, P> extends Component<Props<T, P>, State<T>> {
 
     tableData: null,
     pageLinks: null,
+    api: new Client(),
   };
 
   componentDidMount() {
@@ -179,36 +183,9 @@ class _GenericDiscoverQuery<T, P> extends Component<Props<T, P>, State<T>> {
     }
   }
 
-  getPayload(props: Props<T, P>) {
-    const {cursor, limit, noPagination, referrer} = props;
-    const payload = this.props.getRequestPayload
-      ? this.props.getRequestPayload(props)
-      : props.eventView.getEventsAPIPayload(
-          props.location,
-          props.forceAppendRawQueryString
-        );
-
-    if (cursor) {
-      payload.cursor = cursor;
-    }
-    if (limit) {
-      payload.per_page = limit;
-    }
-    if (noPagination) {
-      payload.noPagination = noPagination;
-    }
-    if (referrer) {
-      payload.referrer = referrer;
-    }
-
-    Object.assign(payload, props.queryExtras ?? {});
-
-    return payload;
-  }
-
   _shouldRefetchData = (prevProps: Props<T, P>): boolean => {
-    const thisAPIPayload = this.getPayload(this.props);
-    const otherAPIPayload = this.getPayload(prevProps);
+    const thisAPIPayload = getPayload(this.props);
+    const otherAPIPayload = getPayload(prevProps);
 
     return (
       !isAPIPayloadSimilar(thisAPIPayload, otherAPIPayload) ||
@@ -246,7 +223,6 @@ class _GenericDiscoverQuery<T, P> extends Component<Props<T, P>, State<T>> {
 
   fetchData = async () => {
     const {
-      api,
       queryBatching,
       beforeFetch,
       afterFetch,
@@ -256,6 +232,7 @@ class _GenericDiscoverQuery<T, P> extends Component<Props<T, P>, State<T>> {
       route,
       setError,
     } = this.props;
+    const {api} = this.state;
 
     if (!eventView.isValid()) {
       return;
@@ -263,7 +240,7 @@ class _GenericDiscoverQuery<T, P> extends Component<Props<T, P>, State<T>> {
 
     const url = `/organizations/${orgSlug}/${route}/`;
     const tableFetchID = Symbol(`tableFetchID`);
-    const apiPayload: Partial<EventQuery & LocationQuery> = this.getPayload(this.props);
+    const apiPayload: Partial<EventQuery & LocationQuery> = getPayload(this.props);
 
     this.setState({isLoading: true, tableFetchID});
 
@@ -371,4 +348,49 @@ export function doDiscoverQuery<T>(
   });
 }
 
+function getPayload<T, P>(props: Props<T, P>) {
+  const {
+    cursor,
+    limit,
+    noPagination,
+    referrer,
+    getRequestPayload,
+    eventView,
+    location,
+    forceAppendRawQueryString,
+  } = props;
+  const payload = getRequestPayload
+    ? getRequestPayload(props)
+    : eventView.getEventsAPIPayload(location, forceAppendRawQueryString);
+
+  if (cursor) {
+    payload.cursor = cursor;
+  }
+  if (limit) {
+    payload.per_page = limit;
+  }
+  if (noPagination) {
+    payload.noPagination = noPagination;
+  }
+  if (referrer) {
+    payload.referrer = referrer;
+  }
+
+  Object.assign(payload, props.queryExtras ?? {});
+
+  return payload;
+}
+
+export function useGenericDiscoverQuery<T, P>(props: Props<T, P>) {
+  const api = useApi();
+  const {orgSlug, route} = props;
+  const url = `/organizations/${orgSlug}/${route}/`;
+  const apiPayload = getPayload<T, P>(props);
+
+  return useQuery<T, QueryError>([route, apiPayload], async () => {
+    const [resp] = await doDiscoverQuery<T>(api, url, apiPayload, props.queryBatching);
+    return resp;
+  });
+}
+
 export default GenericDiscoverQuery;

+ 1 - 2
static/app/utils/performance/anomalies/anomaliesQuery.tsx

@@ -3,7 +3,6 @@ import GenericDiscoverQuery, {
   DiscoverQueryProps,
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
-import withApi from 'sentry/utils/withApi';
 import {ANOMALY_FLAG} from 'sentry/views/performance/transactionSummary/transactionAnomalies/utils';
 
 type AnomaliesProps = {};
@@ -93,4 +92,4 @@ function AnomaliesSeriesQuery(props: Props) {
   );
 }
 
-export default withApi(AnomaliesSeriesQuery);
+export default AnomaliesSeriesQuery;

+ 1 - 2
static/app/utils/performance/histogram/histogramQuery.tsx

@@ -6,7 +6,6 @@ import GenericDiscoverQuery, {
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
 import {DataFilter, HistogramData} from 'sentry/utils/performance/histogram/types';
-import withApi from 'sentry/utils/withApi';
 
 type Histograms = Record<string, HistogramData>;
 
@@ -96,4 +95,4 @@ function HistogramQuery(props: Props) {
   );
 }
 
-export default withApi(HistogramQuery);
+export default HistogramQuery;

+ 1 - 2
static/app/utils/performance/histogram/spanCountHistogramQuery.tsx

@@ -5,7 +5,6 @@ import GenericDiscoverQuery, {
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
 import {DataFilter, HistogramData} from 'sentry/utils/performance/histogram/types';
-import withApi from 'sentry/utils/withApi';
 
 type HistogramProps = {
   numBuckets: number;
@@ -63,4 +62,4 @@ function SpanCountHistogramQuery(props: Props) {
   );
 }
 
-export default withApi(SpanCountHistogramQuery);
+export default SpanCountHistogramQuery;

+ 1 - 2
static/app/utils/performance/histogram/spanHistogramQuery.tsx

@@ -5,7 +5,6 @@ import GenericDiscoverQuery, {
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
 import {DataFilter, HistogramData} from 'sentry/utils/performance/histogram/types';
-import withApi from 'sentry/utils/withApi';
 
 import {SpanSlug} from '../suspectSpans/types';
 
@@ -65,4 +64,4 @@ function SpanHistogramQuery(props: Props) {
   );
 }
 
-export default withApi(SpanHistogramQuery);
+export default SpanHistogramQuery;

+ 0 - 3
static/app/utils/performance/metricsEnhanced/metricsCompatibilityQuery.tsx

@@ -5,7 +5,6 @@ import GenericDiscoverQuery, {
   DiscoverQueryProps,
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
-import useApi from 'sentry/utils/useApi';
 
 export interface MetricsCompatibilityData {
   compatible_projects?: number[];
@@ -30,13 +29,11 @@ function getRequestPayload({
 }
 
 export default function MetricsCompatibilityQuery({children, ...props}: QueryProps) {
-  const api = useApi();
   return (
     <GenericDiscoverQuery<MetricsCompatibilityData, {}>
       route="metrics-compatibility-sums"
       getRequestPayload={getRequestPayload}
       {...props}
-      api={api}
     >
       {({tableData, ...rest}) => {
         return children({

+ 0 - 3
static/app/utils/performance/metricsEnhanced/metricsCompatibilityQuerySums.tsx

@@ -5,7 +5,6 @@ import GenericDiscoverQuery, {
   DiscoverQueryProps,
   GenericChildrenProps,
 } from 'sentry/utils/discover/genericDiscoverQuery';
-import useApi from 'sentry/utils/useApi';
 
 export interface MetricsCompatibilitySumData {
   sum: {
@@ -33,13 +32,11 @@ function getRequestPayload({
 }
 
 export default function MetricsCompatibilitySumsQuery({children, ...props}: QueryProps) {
-  const api = useApi();
   return (
     <GenericDiscoverQuery<MetricsCompatibilitySumData, {}>
       route="metrics-compatibility"
       getRequestPayload={getRequestPayload}
       {...props}
-      api={api}
     >
       {({tableData, ...rest}) => {
         return children({

+ 5 - 10
static/app/utils/performance/quickTrace/traceFullQuery.tsx

@@ -14,7 +14,6 @@ import {
   getTraceRequestPayload,
   makeEventView,
 } from 'sentry/utils/performance/quickTrace/utils';
-import withApi from 'sentry/utils/withApi';
 
 type AdditionalQueryProps = {
   detailed?: boolean;
@@ -97,14 +96,10 @@ function GenericTraceFullQuery<T>({
   );
 }
 
-export const TraceFullQuery = withApi(
-  (props: Omit<QueryProps<TraceFull[]>, 'detailed'>) => (
-    <GenericTraceFullQuery<TraceFull[]> {...props} detailed={false} />
-  )
+export const TraceFullQuery = (props: Omit<QueryProps<TraceFull[]>, 'detailed'>) => (
+  <GenericTraceFullQuery<TraceFull[]> {...props} detailed={false} />
 );
 
-export const TraceFullDetailedQuery = withApi(
-  (props: Omit<QueryProps<TraceFullDetailed[]>, 'detailed'>) => (
-    <GenericTraceFullQuery<TraceFullDetailed[]> {...props} detailed />
-  )
-);
+export const TraceFullDetailedQuery = (
+  props: Omit<QueryProps<TraceFullDetailed[]>, 'detailed'>
+) => <GenericTraceFullQuery<TraceFullDetailed[]> {...props} detailed />;

Some files were not shown because too many files changed in this diff