Browse Source

ref(ts) Convert RichListField to typescript (#23513)

I tried to make this work without `any` but the `FieldFromConfig` types
for several properties are not compatible with types expected elsewhere.
I made some minor changes to the default props as they wouldn't work
that well.

Co-authored-by: Priscila Oliveira <priscilawebdev@gmail.com>
Co-authored-by: Matej Minar <matej.minar@sentry.io>
Mark Story 4 years ago
parent
commit
bc22b0b880

+ 1 - 0
src/sentry/static/sentry/app/actionCreators/modal.tsx

@@ -191,6 +191,7 @@ export type SentryAppDetailsModalOptions = {
 type DebugFileSourceModalOptions = {
   sourceType: DebugFileSource;
   onSave: (data: Record<string, string>) => void;
+  sourceConfig?: Record<string, string>;
 };
 
 export async function openDebugFileSourceModal(options: DebugFileSourceModalOptions) {

+ 23 - 12
src/sentry/static/sentry/app/data/forms/projectDebugFiles.jsx → src/sentry/static/sentry/app/data/forms/projectDebugFiles.tsx

@@ -8,12 +8,19 @@ import Feature from 'app/components/acl/feature';
 import FeatureDisabled from 'app/components/acl/featureDisabled';
 import {DEBUG_SOURCE_TYPES} from 'app/data/debugFileSources';
 import {t} from 'app/locale';
+import {Choices} from 'app/types';
+import {BuiltinSymbolSource} from 'app/types/debugFiles';
+import {Field} from 'app/views/settings/components/forms/type';
 import TextBlock from 'app/views/settings/components/text/textBlock';
 
+type SymbolSourceOptions = {
+  builtinSymbolSources: BuiltinSymbolSource[];
+};
+
 // Export route to make these forms searchable by label/help
 export const route = '/settings/:orgId/projects/:projectId/debug-symbols/';
 
-function flattenKeys(obj) {
+function flattenKeys(obj: any): Record<string, string> {
   const result = {};
   forEach(obj, (value, key) => {
     if (isObject(value)) {
@@ -27,7 +34,7 @@ function flattenKeys(obj) {
   return result;
 }
 
-function unflattenKeys(obj) {
+function unflattenKeys(obj: Record<string, string>): Record<string, any> {
   const result = {};
   forEach(obj, (value, key) => {
     set(result, key.split('.'), value);
@@ -35,7 +42,7 @@ function unflattenKeys(obj) {
   return result;
 }
 
-export const fields = {
+export const fields: Record<string, Field> = {
   builtinSymbolSources: {
     name: 'builtinSymbolSources',
     type: 'select',
@@ -44,8 +51,8 @@ export const fields = {
     help: t(
       'Configures which built-in repositories Sentry should use to resolve debug files.'
     ),
-    formatMessageValue: (value, {builtinSymbolSources}) => {
-      const rv = [];
+    formatMessageValue: (value, {builtinSymbolSources}: SymbolSourceOptions) => {
+      const rv: string[] = [];
       value.forEach(key => {
         builtinSymbolSources.forEach(source => {
           if (source.sentry_key === key) {
@@ -55,11 +62,11 @@ export const fields = {
       });
       return rv.join(', ');
     },
-    choices: ({builtinSymbolSources}) =>
-      builtinSymbolSources &&
-      builtinSymbolSources
-        .filter(source => !source.hidden)
-        .map(source => [source.sentry_key, t(source.name)]),
+    choices: ({builtinSymbolSources}) => {
+      return (builtinSymbolSources as BuiltinSymbolSource[])
+        ?.filter(source => !source.hidden)
+        .map(source => [source.sentry_key, t(source.name)]) as Choices;
+    },
   },
   symbolSources: {
     name: 'symbolSources',
@@ -106,7 +113,12 @@ export const fields = {
     },
 
     getValue: sources => JSON.stringify(sources.map(unflattenKeys)),
-    setValue: raw => (JSON.parse(raw || null) || []).map(flattenKeys),
+    setValue: (raw: string) => {
+      if (!raw) {
+        return [];
+      }
+      return (JSON.parse(raw) || []).map(flattenKeys);
+    },
 
     renderItem(item) {
       return item.name ? <span>{item.name}</span> : <em>{t('<Unnamed Repository>')}</em>;
@@ -128,7 +140,6 @@ export const fields = {
     },
 
     removeConfirm: {
-      title: t('Remove Repository?'),
       confirmText: t('Remove Repository'),
       message: (
         <React.Fragment>

+ 4 - 1
src/sentry/static/sentry/app/views/settings/components/forms/fieldFromConfig.tsx

@@ -90,7 +90,10 @@ export default class FieldFromConfig extends React.Component<Props> {
         }
         return <RadioField {...props} choices={choices} />;
       case 'rich_list':
-        return <RichListField {...props} />;
+        // TODO(ts) The switch on field.type is not resolving
+        // the Field union for this component. It isn't clear why the spread
+        // works for project_mapper but not this component.
+        return <RichListField {...(props as any)} />;
       case 'table':
         return <TableField {...props} />;
       case 'project_mapper':

+ 100 - 79
src/sentry/static/sentry/app/views/settings/components/forms/richListField.jsx → src/sentry/static/sentry/app/views/settings/components/forms/richListField.tsx

@@ -1,126 +1,147 @@
 import React from 'react';
 import styled from '@emotion/styled';
-import pickBy from 'lodash/pickBy';
-import PropTypes from 'prop-types';
 
 import Button from 'app/components/button';
 import Confirm from 'app/components/confirm';
 import DropdownAutoComplete from 'app/components/dropdownAutoComplete';
+import {Item as ListItem} from 'app/components/dropdownAutoComplete/types';
 import DropdownButton from 'app/components/dropdownButton';
 import {IconAdd, IconDelete, IconSettings} from 'app/icons';
 import {t} from 'app/locale';
 import InputField from 'app/views/settings/components/forms/inputField';
 
-const RichListProps = {
+type ConfirmProps = Partial<React.ComponentProps<typeof Confirm>>;
+type DropdownProps = Omit<React.ComponentProps<typeof DropdownAutoComplete>, 'children'>;
+
+type UpdatedItem = ListItem | Record<string, string>;
+
+type DefaultProps = {
   /**
    * Text used for the add item button.
    */
-  addButtonText: PropTypes.node,
+  addButtonText: string;
 
   /**
-   * Configuration for the add item dropdown.
+   * Callback invoked when an item is added via the dropdown menu.
+   *
+   * The callback is expected to call `addItem(item)`
    */
-  addDropdown: PropTypes.object.isRequired,
+  onAddItem: RichListCallback;
 
+  /**
+   * Callback invoked when an item is removed.
+   *
+   * The callback is expected to call `removeItem(item)`
+   */
+  onRemoveItem: RichListCallback;
+};
+
+const defaultProps: DefaultProps = {
+  addButtonText: t('Add item'),
+  onAddItem: (item, addItem) => addItem(item),
+  onRemoveItem: (item, removeItem) => removeItem(item),
+};
+
+/**
+ * You can get better typing by specifying the item type
+ * when using this component.
+ *
+ * The callback parameter accepts a more general type than `ListItem` as the
+ * callback handler can perform arbitrary logic and extend the payload in
+ * ways that are hard to type.
+ */
+export type RichListCallback = (
+  item: ListItem,
+  callback: (item: UpdatedItem) => void
+) => void;
+
+export type RichListProps = {
   /**
    * Render function to render an item.
    */
-  renderItem: PropTypes.func,
+  renderItem: (item: ListItem) => React.ReactNode;
 
   /**
-   * Callback invoked when an item is added via the dropdown menu.
+   * The list of items to render.
    */
-  onAddItem: PropTypes.func,
+  value: ListItem[];
+
+  onBlur: InputField['props']['onBlur'];
+  onChange: InputField['props']['onChange'];
 
   /**
-   * Callback invoked when an item is interacted with.
+   * Configuration for the add item dropdown.
    */
-  onEditItem: PropTypes.func,
+  addDropdown: DropdownProps;
 
   /**
-   * Callback invoked when an item is removed.
+   * Disables all controls in the rich list.
    */
-  onRemoveItem: PropTypes.func,
+  disabled: boolean;
 
   /**
    * Properties for the confirm remove dialog. If missing, the item will be
    * removed immediately.
    */
-  removeConfirm: PropTypes.object,
-};
-
-function getDefinedProps(propTypes, props) {
-  return pickBy(props, (_prop, key) => key in propTypes);
-}
+  removeConfirm?: ConfirmProps;
 
-class RichList extends React.PureComponent {
-  static propTypes = {
-    ...RichListProps,
-
-    /**
-     * Disables all controls in the rich list.
-     */
-    disabled: PropTypes.bool,
-
-    /**
-     * The list of items to render.
-     */
-    value: PropTypes.array.isRequired,
-  };
+  /**
+   * Callback invoked when an item is interacted with.
+   *
+   * The callback is expected to call `editItem(item)`
+   */
+  onEditItem?: RichListCallback;
+} & DefaultProps;
 
-  static defaultProps = {
-    addButtonText: t('Add Item'),
-    renderItem: item => item,
-    onAddItem: (item, addItem) => addItem(item),
-    onRemoveItem: (item, removeItem) => removeItem(item),
-  };
+class RichList extends React.PureComponent<RichListProps> {
+  static defaultProps = defaultProps;
 
-  triggerChange = items => {
+  triggerChange = (items: UpdatedItem[]) => {
     if (!this.props.disabled) {
-      this.props.onChange(items, {});
-      this.props.onBlur(items, {});
+      this.props.onChange?.(items, {});
+      this.props.onBlur?.(items, {});
     }
   };
 
-  addItem = data => {
+  addItem = (data: UpdatedItem) => {
     const items = [...this.props.value, data];
     this.triggerChange(items);
   };
 
-  updateItem = (data, index) => {
-    const items = [...this.props.value];
+  updateItem = (data: UpdatedItem, index: number) => {
+    const items = [...this.props.value] as UpdatedItem[];
     items.splice(index, 1, data);
     this.triggerChange(items);
   };
 
-  removeItem = index => {
+  removeItem = (index: number) => {
     const items = [...this.props.value];
     items.splice(index, 1);
     this.triggerChange(items);
   };
 
-  onSelectDropdownItem = item => {
-    if (!this.props.disabled) {
+  onSelectDropdownItem = (item: ListItem) => {
+    if (!this.props.disabled && this.props.onAddItem) {
       this.props.onAddItem(item, this.addItem);
     }
   };
 
-  onEditItem = (item, index) => {
-    if (!this.props.disabled) {
+  onEditItem = (item: ListItem, index: number) => {
+    if (!this.props.disabled && this.props.onEditItem) {
       this.props.onEditItem(item, data => this.updateItem(data, index));
     }
   };
 
-  onRemoveItem = (item, index) => {
+  onRemoveItem = (item: ListItem, index: number) => {
     if (!this.props.disabled) {
       this.props.onRemoveItem(item, () => this.removeItem(index));
     }
   };
 
-  renderItem = (item, index) => {
+  renderItem = (item: ListItem, index: number) => {
     const {disabled} = this.props;
 
-    const removeIcon = (onClick = null) => (
+    const removeIcon = (onClick?: () => void) => (
       <ItemButton
         onClick={onClick}
         disabled={disabled}
@@ -194,31 +215,31 @@ class RichList extends React.PureComponent {
   }
 }
 
-export default class RichListField extends React.PureComponent {
-  static propTypes = {
-    // TODO(ts)
-    // ...InputField.propTypes,
-    ...RichListProps,
-  };
-
-  renderRichList = fieldProps => {
-    const richListProps = getDefinedProps(RichListProps, this.props);
-    const {value, ...props} = fieldProps;
-
-    // We must not render this field until `setValue` has been applied by the
-    // model, which is done after the field is mounted for the first time. To
-    // check this, we cannot use Array.isArray because the value passed in by
-    // the model might actually be an ObservableArray.
-    if (typeof value === 'string' || value.length === undefined) {
-      return null;
-    }
-
-    return <RichList {...props} value={[...value]} {...richListProps} />;
-  };
-
-  render() {
-    return <InputField {...this.props} field={this.renderRichList} />;
-  }
+/**
+ * A 'rich' dropdown that provides action hooks for when item
+ * are selected/created/removed.
+ *
+ * An example usage is the debug image selector where each 'source' option
+ * requires additional configuration data.
+ */
+export default function RichListField(props: RichListProps & InputField['props']) {
+  return (
+    <InputField
+      {...props}
+      field={(fieldProps: RichListProps) => {
+        const {value, ...otherProps} = fieldProps;
+
+        // We must not render this field until `setValue` has been applied by the
+        // model, which is done after the field is mounted for the first time. To
+        // check this, we cannot use Array.isArray because the value passed in by
+        // the model might actually be an ObservableArray.
+        if (typeof value === 'string' || value?.length === undefined) {
+          return null;
+        }
+        return <RichList {...otherProps} value={[...value]} />;
+      }}
+    />
+  );
 }
 
 const ItemList = styled('ul')`
@@ -228,7 +249,7 @@ const ItemList = styled('ul')`
   padding: 0;
 `;
 
-const Item = styled('li')`
+const Item = styled('li')<{disabled?: boolean}>`
   display: flex;
   align-items: center;
   background-color: ${p => p.theme.button.default.background};

+ 21 - 1
src/sentry/static/sentry/app/views/settings/components/forms/type.tsx

@@ -4,6 +4,7 @@ import {createFilter} from 'react-select';
 import Alert from 'app/components/alert';
 import {AvatarProject, Project} from 'app/types';
 import RangeSlider from 'app/views/settings/components/forms/controls/rangeSlider';
+import {RichListProps} from 'app/views/settings/components/forms/richListField';
 
 export const FieldType = [
   'array',
@@ -167,16 +168,35 @@ export type SentryProjectSelectorType = {
   avatarSize?: number;
 };
 
+/**
+ * Json field configuration makes using generics hard.
+ * This isn't the ideal type to use, but it will cover
+ * general usage.
+ */
+export type RichListType = {
+  type: 'rich_list';
+} & Pick<
+  RichListProps,
+  | 'renderItem'
+  | 'addButtonText'
+  | 'onAddItem'
+  | 'onEditItem'
+  | 'onRemoveItem'
+  | 'addDropdown'
+  | 'removeConfirm'
+>;
+
 export type Field = (
   | CustomType
   | SelectControlType
   | InputType
   | TextareaType
   | RangeType
-  | {type: typeof FieldType[number]}
   | TableType
   | ProjectMapperType
   | SentryProjectSelectorType
+  | RichListType
+  | {type: typeof FieldType[number]}
 ) &
   BaseField;