ref(ts): Convert `eventsRequest` and `actionCreators/events` t… (#14251)

Also adds `@types/echart`

package.json

@@ -21,6 +21,7 @@
     "@sentry/browser": "5.6.0-beta.4",
     "@sentry/integrations": "5.6.0-beta.4",
     "@sentry/typescript": "^5.3.0",
+    "@types/echarts": "^4.1.10",
     "@types/jquery": "^2.0.53",
     "@types/lodash": "^4.14.134",
     "@types/moment-timezone": "^0.5.12",

src/sentry/static/sentry/app/actionCreators/events.jsx → src/sentry/static/sentry/app/actionCreators/events.tsx

@@ -1,16 +1,31 @@
 import {canIncludePreviousPeriod} from 'app/views/events/utils/canIncludePreviousPeriod';
 import {getPeriod} from 'app/utils/getPeriod';
+import {EventsStats, Organization} from 'app/types';
 
-const BASE_URL = org => `/organizations/${org.slug}/events-stats/`;
+const getBaseUrl = (org: Organization) => `/organizations/${org.slug}/events-stats/`;
+
+type Options = {
+  organization: Organization;
+  project?: number[];
+  environment?: string[];
+  period?: string;
+  start?: Date;
+  end?: Date;
+  interval?: string;
+  includePrevious?: boolean;
+  limit?: number;
+  query?: string;
+  yAxis?: 'event_count' | 'user_count';
+};
 
 /**
- * Make requests to `health` endpoint
+ * Make requests to `events-stats` endpoint
  *
  * @param {Object} api API client instance
  * @param {Object} options Request parameters
  * @param {Object} options.organization Organization object
- * @param {Number[]} options.projects List of project ids
- * @param {String[]} options.environments List of environments to query for
+ * @param {Number[]} options.project List of project ids
+ * @param {String[]} options.environment List of environments to query for
  * @param {String} options.period Time period to query for, in the format: <integer><units> where units are "d" or "h"
  * @param {String} options.interval Time interval to group results in, in the format: <integer><units> where units are "d", "h", "m", "s"
  * @param {Boolean} options.includePrevious Should request also return reqsults for previous period?
@@ -18,7 +33,8 @@ const BASE_URL = org => `/organizations/${org.slug}/events-stats/`;
  * @param {String} options.query Search query
  */
 export const doEventsRequest = (
-  api,
+  // TODO(ts): Update when we type `app/api`
+  api: any,
   {
     organization,
     project,
@@ -30,8 +46,8 @@ export const doEventsRequest = (
     includePrevious,
     query,
     yAxis,
-  }
-) => {
+  }: Options
+): Promise<EventsStats> => {
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
   const urlQuery = {
     interval,
@@ -46,7 +62,7 @@ export const doEventsRequest = (
   // the tradeoff for now.
   const periodObj = getPeriod({period, start, end}, {shouldDoublePeriod});
 
-  return api.requestPromise(`${BASE_URL(organization)}`, {
+  return api.requestPromise(`${getBaseUrl(organization)}`, {
     query: {
       ...urlQuery,
       ...periodObj,

src/sentry/static/sentry/app/types/echarts.tsx

@@ -1,124 +1,15 @@
-export type Finder =
-  | string
-  | {
-      seriesIndex?: number;
-      seriesId?: string;
-      seriesName?: string;
-      geoIndex?: number;
-      geoId?: string;
-      geoName?: string;
-      xAxisIndex?: number;
-      xAxisId?: string;
-      xAxisName?: string;
-      yAxisIndex?: number;
-      yAxisId?: string;
-      yAxisName?: string;
-      gridIndex?: number;
-      gridId?: string;
-      gridName?: string;
-    };
-type PixelValue = Array<number | string> | number | string;
+import {ECharts} from 'echarts';
 
-export type EchartsInstance = {
-  group: string | number;
-  setOption:
-    | ((option: Object, notMerge?: boolean, lazyUpdate?: boolean) => void)
-    | ((option: Object, opts?: Object) => void);
-  getWidth: () => number;
-  getHeight: () => number;
-  getDom: () => HTMLCanvasElement | HTMLDivElement;
-  getOption: () => Object;
-
-  // See: https://echarts.apache.org/en/api.html#echartsInstance.resize
-  // Unsure of return value
-  resize: (opts?: {
-    width?: number | string;
-    height?: number | string;
-    silent?: boolean;
-  }) => void;
-
-  dispatch: (payload: Object) => void;
-
-  on:
-    | ((eventName: string, handler: Function, context?: Object) => void)
-    | ((
-        eventName: string,
-        query: string | Object,
-        handler: Function,
-        context?: Object
-      ) => void);
-
-  off: (eventName: string, handler?: Function) => void;
-
-  convertToPixel: (
-    // finder is used to indicate in which coordinate system conversion is performed.
-    // Generally, index or id or name can be used to specify coordinate system.
-    finder: Finder,
-    // The value to be converted.
-    value: PixelValue
-  ) => // Conversion result, in pixel coordinate system, where the origin ([0, 0])
-  // is on the left-top of the main dom of echarts instance.
-  PixelValue;
-
-  convertFromPixel: (
-    // finder is used to indicate in which coordinate system conversion is performed.
-    // Generally, index or id or name can be used to specify coordinate system.
-    finder: Finder,
-    // The value to be converted, in pixel coordinate system, where the origin ([0, 0])
-    // is on the left-top of the main dom of echarts instance.
-    value: PixelValue
-  ) => // Conversion result
-  PixelValue;
-
-  containPixel: (
-    // finder is used to specify coordinate systems or series on which the judgement performed.
-    // Generally, index or id or name can be used to specify coordinate system.
-    finder: Finder,
-    // The value to be judged, in pixel coordinate system, where the origin ([0, 0])
-    // is on the left-top of the main dom of echarts instance.
-    value: PixelValue
-  ) => boolean;
-
-  showLoading: (type?: string, opts?: Object) => void;
-
-  hideLoading: () => void;
-
-  getDataURL: (opts: {
-    // Exporting format, can be either png, or jpeg
-    type?: string;
-    // Resolution ratio of exporting image, 1 by default.
-    pixelRatio?: number;
-    // Background color of exporting image, use backgroundColor in option by default.
-    backgroundColor?: string;
-    // Excluded components list. e.g. ['toolbox']
-    excludeComponents?: Array<string>;
-  }) => string;
-
-  getConnectedDataURL: (opts: {
-    // Exporting format, can be either png, or jpeg
-    type?: string;
-    // Resolution ratio of exporting image, 1 by default.
-    pixelRatio?: number;
-    // Background color of exporting image, use backgroundColor in option by default.
-    backgroundColor?: string;
-    // Excluded components list. e.g. ['toolbox']
-    excludeComponents?: Array<string>;
-  }) => string;
-
-  appendData: (opts: {
-    // Specify which series the data will be appended to.
-    seriesIndex?: string;
-    // The data to be appended.
-    data?: Array<any>;
-  }) => string;
-
-  clear: () => void;
-
-  isDisposed: () => boolean;
+export type SeriesDataUnit = {
+  value: number;
+  name: string | number; // number because we sometimes use timestamps
+};
 
-  dispose: () => void;
+export type Series = {
+  seriesName: string;
+  data: SeriesDataUnit[];
 };
 
 export type ReactEchartsRef = {
-  getEchartsInstance: () => EchartsInstance;
+  getEchartsInstance: () => ECharts;
 };

src/sentry/static/sentry/app/types/index.tsx

@@ -43,3 +43,10 @@ export type Event = {
   message: string;
   platform?: string;
 };
+
+export type EventsStatsData = [number, {count: number}[]][];
+
+export type EventsStats = {
+  data: EventsStatsData;
+  totals?: {count: number};
+};

src/sentry/static/sentry/app/views/events/eventsChart.jsx

@@ -4,7 +4,6 @@ import React from 'react';
 import styled from 'react-emotion';
 
 import {getInterval} from 'app/components/charts/utils';
-import {t} from 'app/locale';
 import ChartZoom from 'app/components/charts/chartZoom';
 import LineChart from 'app/components/charts/lineChart';
 import LoadingPanel, {LoadingMask} from 'app/views/events/loadingPanel';
@@ -16,8 +15,6 @@ import withGlobalSelection from 'app/utils/withGlobalSelection';
 import EventsRequest from './utils/eventsRequest';
 import YAxisSelector from './yAxisSelector';
 
-const DEFAULT_GET_CATEGORY = () => t('Events');
-
 class EventsLineChart extends React.Component {
   static propTypes = {
     loading: PropTypes.bool,
@@ -150,7 +147,6 @@ class EventsChart extends React.Component {
             interval={getInterval(this.props, true)}
             showLoading={false}
             query={query}
-            getCategory={DEFAULT_GET_CATEGORY}
             includePrevious={includePrevious}
             yAxis={this.state.yAxis}
           >

src/sentry/static/sentry/app/views/events/utils/eventsRequest.jsx → src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx

@@ -2,6 +2,9 @@ import {isEqual, omitBy} from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import {Organization, EventsStats, EventsStatsData} from 'app/types';
+import {Series, SeriesDataUnit} from 'app/types/echarts';
+
 import {addErrorMessage} from 'app/actionCreators/indicator';
 import {canIncludePreviousPeriod} from 'app/views/events/utils/canIncludePreviousPeriod';
 import {doEventsRequest} from 'app/actionCreators/events';
@@ -10,11 +13,54 @@ import SentryTypes from 'app/sentryTypes';
 
 import LoadingPanel from '../loadingPanel';
 
+type RenderProps = {
+  loading: boolean;
+  reloading: boolean;
+
+  // timeseries data
+  timeseriesData?: Series[];
+  allTimeseriesData?: EventsStatsData;
+  originalTimeseriesData?: EventsStatsData;
+  timeseriesTotals?: object;
+  originalPreviousTimeseriesData?: EventsStatsData | null;
+  previousTimeseriesData?: Series | null;
+  timeAggregatedData?: Series | {};
+};
+
+type EventsRequestProps = {
+  // TODO(ts): Update when we type `app/api`
+  api: object;
+  organization: Organization;
+  timeAggregationSeriesName: string;
+
+  project?: number[];
+  environment?: string[];
+  period?: string;
+  start?: any;
+  end?: any;
+  interval?: string;
+
+  limit?: number;
+  query?: string;
+  includePrevious?: boolean;
+  includeTransformedData?: boolean;
+  includeTimeAggregation?: boolean;
+  loading?: boolean;
+  showLoading?: boolean;
+  yAxis?: 'event_count' | 'user_count';
+  children: (renderProps: RenderProps) => React.ReactNode;
+};
+
+type EventsRequestState = {
+  reloading: boolean;
+  timeseriesData: null | EventsStats;
+};
+
 const propNamesToIgnore = ['api', 'children', 'organization', 'loading'];
-const omitIgnoredProps = props =>
+const omitIgnoredProps = (props: EventsRequestProps) =>
   omitBy(props, (_value, key) => propNamesToIgnore.includes(key));
 
-class EventsRequest extends React.PureComponent {
+class EventsRequest extends React.PureComponent<EventsRequestProps, EventsRequestState> {
   static propTypes = {
     /**
      * API client instance
@@ -112,45 +158,38 @@ class EventsRequest extends React.PureComponent {
     end: null,
     interval: '1d',
     limit: 15,
-    getCategory: i => i,
     query: '',
 
     includePrevious: true,
     includeTransformedData: true,
   };
 
-  constructor(props) {
-    super(props);
-    this.state = {
-      reloading: false || props.loading,
-      timeseriesData: null,
-    };
-  }
+  state = {
+    reloading: !!this.props.loading,
+    timeseriesData: null,
+  };
 
   componentDidMount() {
     this.fetchData();
   }
-
-  componentDidUpdate(prevProps) {
+  componentDidUpdate(prevProps: EventsRequestProps) {
     if (isEqual(omitIgnoredProps(prevProps), omitIgnoredProps(this.props))) {
       return;
     }
-
     this.fetchData();
   }
-
   componentWillUnmount() {
     this.unmounting = true;
   }
 
+  private unmounting: boolean = false;
+
   fetchData = async () => {
     const {api, ...props} = this.props;
-    let timeseriesData;
-
+    let timeseriesData: EventsStats | null;
     this.setState(state => ({
       reloading: state.timeseriesData !== null,
     }));
-
     try {
       timeseriesData = await doEventsRequest(api, props);
     } catch (resp) {
@@ -161,55 +200,55 @@ class EventsRequest extends React.PureComponent {
       }
       timeseriesData = null;
     }
-
     if (this.unmounting) {
       return;
     }
-
     this.setState({
       reloading: false,
       timeseriesData,
     });
   };
-
   /**
    * Retrieves data set for the current period (since data can potentially contain previous period's data), as
    * well as the previous period if possible.
    *
    * Returns `null` if data does not exist
    */
-  getData = data => {
+  getData = (
+    data: EventsStatsData
+  ): {previous: EventsStatsData | null; current: EventsStatsData} => {
     const {period, includePrevious} = this.props;
 
-    if (!data) {
-      return {
-        previous: null,
-        current: null,
-      };
-    }
-
     const hasPreviousPeriod = canIncludePreviousPeriod(includePrevious, period);
     // Take the floor just in case, but data should always be divisible by 2
     const dataMiddleIndex = Math.floor(data.length / 2);
-
     return {
-      previous: hasPreviousPeriod ? data.slice(0, dataMiddleIndex) : null,
       current: hasPreviousPeriod ? data.slice(dataMiddleIndex) : data,
+      previous: hasPreviousPeriod ? data.slice(0, dataMiddleIndex) : null,
     };
   };
 
   // This aggregates all values per `timestamp`
-  calculateTotalsPerTimestamp = (data, getName = timestamp => timestamp * 1000) => {
-    return data.map(([timestamp, countArray], i) => ({
+  calculateTotalsPerTimestamp = (
+    data: EventsStatsData,
+    getName: (
+      timestamp: number,
+      countArray: {count: number}[],
+      i: number
+    ) => number = timestamp => timestamp * 1000
+  ): SeriesDataUnit[] =>
+    data.map(([timestamp, countArray], i) => ({
       name: getName(timestamp, countArray, i),
       value: countArray.reduce((acc, {count}) => acc + count, 0),
     }));
-  };
 
   /**
    * Get previous period data, but transform timestampts so that data fits unto the current period's data axis
    */
-  transformPreviousPeriodData = (current, previous) => {
+  transformPreviousPeriodData = (
+    current: EventsStatsData,
+    previous: EventsStatsData | null
+  ): Series | null => {
     // Need the current period data array so we can take the timestamp
     // so we can be sure the data lines up
     if (!previous) {
@@ -224,25 +263,19 @@ class EventsRequest extends React.PureComponent {
       ),
     };
   };
-
   /**
    * Aggregate all counts for each time stamp
    */
-  transformAggregatedTimeseries = (data, seriesName) => {
-    if (!data) {
-      return null;
-    }
-
+  transformAggregatedTimeseries = (data: EventsStatsData, seriesName: string): Series => {
     return {
       seriesName,
       data: this.calculateTotalsPerTimestamp(data),
     };
   };
-
   /**
    * Transforms query response into timeseries data to be used in a chart
    */
-  transformTimeseriesData = data => {
+  transformTimeseriesData = (data: EventsStatsData): [Series] => {
     return [
       {
         seriesName: 'Current Period',
@@ -254,31 +287,27 @@ class EventsRequest extends React.PureComponent {
     ];
   };
 
-  transformData = data => {
-    if (!data) {
-      return null;
+  processData(response: EventsStats | null) {
+    if (!response) {
+      return {};
     }
 
-    return this.transformTimeseriesData(data);
-  };
-
-  processData({data, totals} = {}) {
+    const {data, totals} = response;
     const {
       includeTransformedData,
       includeTimeAggregation,
       timeAggregationSeriesName,
     } = this.props;
     const {current, previous} = this.getData(data);
-    const transformedData = includeTransformedData ? this.transformData(current) : null;
-
+    const transformedData = includeTransformedData
+      ? this.transformTimeseriesData(current)
+      : [];
     const previousData = includeTransformedData
       ? this.transformPreviousPeriodData(current, previous)
       : null;
-
     const timeAggregatedData = includeTimeAggregation
       ? this.transformAggregatedTimeseries(current, timeAggregationSeriesName)
-      : null;
-
+      : {};
     return {
       data: transformedData,
       allData: data,
@@ -289,12 +318,9 @@ class EventsRequest extends React.PureComponent {
       timeAggregatedData,
     };
   }
-
   render() {
     const {children, showLoading, ...props} = this.props;
-
     const {timeseriesData, reloading} = this.state;
-
     // Is "loading" if data is null
     const loading = this.props.loading || timeseriesData === null;
 
@@ -310,12 +336,11 @@ class EventsRequest extends React.PureComponent {
       originalPreviousData: originalPreviousTimeseriesData,
       previousData: previousTimeseriesData,
       timeAggregatedData,
-    } = (timeseriesData && this.processData(timeseriesData, true)) || {};
+    } = this.processData(timeseriesData);
 
     return children({
       loading,
       reloading,
-
       // timeseries data
       timeseriesData: transformedTimeseriesData,
       allTimeseriesData,
@@ -324,11 +349,9 @@ class EventsRequest extends React.PureComponent {
       originalPreviousTimeseriesData,
       previousTimeseriesData,
       timeAggregatedData,
-
       // sometimes we want to reference props that were given to EventsRequest
       ...props,
     });
   }
 }
-
 export default EventsRequest;

tests/js/spec/views/events/utils/eventsRequest.spec.jsx

@@ -213,7 +213,7 @@ describe('EventsRequest', function() {
 
       expect(mock).toHaveBeenLastCalledWith(
         expect.objectContaining({
-          timeAggregatedData: null,
+          timeAggregatedData: {},
         })
       );
 
@@ -242,11 +242,7 @@ describe('EventsRequest', function() {
       );
 
       wrapper = mount(
-        <EventsRequest
-          {...DEFAULTS}
-          includeTimeseries={true}
-          getCategory={() => 'static-category'}
-        >
+        <EventsRequest {...DEFAULTS} includeTimeseries={true}>
           {mock}
         </EventsRequest>
       );
@@ -256,7 +252,7 @@ describe('EventsRequest', function() {
 
       expect(mock).toHaveBeenLastCalledWith(
         expect.objectContaining({
-          timeAggregatedData: null,
+          timeAggregatedData: {},
         })
       );
 

yarn.lock

@@ -1736,6 +1736,13 @@
     "@svgr/plugin-svgo" "^4.0.3"
     loader-utils "^1.1.0"
 
+"@types/echarts@^4.1.10":
+  version "4.1.10"
+  resolved "https://registry.yarnpkg.com/@types/echarts/-/echarts-4.1.10.tgz#ee71911eb8b1717c7c12c0bd81fc83db872f4d3b"
+  integrity sha512-AadUmn1mHp0RGNFwcDnNdVbz5LNZZla7V9B+3ptyEp31qiyRK0Hfr2yQLD26/4ojcLDSmUzyGfVrHrEXxgUmuA==
+  dependencies:
+    "@types/zrender" "*"
+
 "@types/eslint-visitor-keys@^1.0.0":
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/@types/eslint-visitor-keys/-/eslint-visitor-keys-1.0.0.tgz#1ee30d79544ca84d68d4b3cdb0af4f205663dd2d"
@@ -1889,6 +1896,11 @@
     "@types/unist" "*"
     "@types/vfile-message" "*"
 
+"@types/zrender@*":
+  version "4.0.0"
+  resolved "https://registry.yarnpkg.com/@types/zrender/-/zrender-4.0.0.tgz#a6806f12ec4eccaaebd9b0d816f049aca6188fbd"
+  integrity sha512-s89GOIeKFiod2KSqHkfd2rzx+T2DVu7ihZCBEBnhFrzvQPUmzvDSBot9Fi1DfMQm9Odg+rTqoMGC38RvrwJK2w==
+
 "@typescript-eslint/eslint-plugin@^1.11.0":
   version "1.11.0"
   resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-1.11.0.tgz#870f752c520db04db6d3668af7479026a6f2fb9a"