Make ButtonProps.onClick more type-able. (#4853)

* Make Button onClick prop more typeable Swaps the order of the arguments so that the optional arg is second. * Review fixes. * removes some implicit any's * NoticeTable to FC might as well, at this point. we're trying to do these as we can as they present themselves * fixes margin for collapse icon (broken before this PR) and removes an unnecessary type parameter Co-authored-by: Dimitri Mitropoulos <dimitrimitropoulos@gmail.com>
2026-05-24 16:51:06 -04:00 · 2022-06-13 15:20:05 +00:00
parent dcdc429b9b
commit fc36be710f
11 changed files with 157 additions and 147 deletions
--- a/packages/insomnia-components/src/button/async-button.tsx
+++ b/packages/insomnia-components/src/button/async-button.tsx
@@ -1,4 +1,4 @@
-import React, { ReactNode, useCallback, useState } from 'react';
+import React, { MouseEvent, ReactNode, useCallback, useState } from 'react';

 import type { ButtonProps } from './button';
 import { Button } from './button';
@@ -26,7 +26,7 @@ export const AsyncButton = <T extends unknown>({
  ...props
 }: AsyncButtonProps<T>) => {
  const [loading, setLoading] = useState(false);
-  const asyncHandler = useCallback(async event => {
+  const asyncHandler = useCallback(async (event: MouseEvent<HTMLButtonElement>) => {
    const result = onClick(event);

    if (isPromise(result)) {
--- a/packages/insomnia-components/src/notice-table.tsx
+++ b/packages/insomnia-components/src/notice-table.tsx
@@ -1,4 +1,4 @@
-import React, { PureComponent } from 'react';
+import React, { PropsWithChildren, useCallback, useState } from 'react';
 import styled from 'styled-components';

 import { Button } from './button';
@@ -13,16 +13,12 @@ export interface Notice {

 export interface NoticeTableProps<T extends Notice> {
  notices: T[];
-  onClick?: (notice: T, event: React.SyntheticEvent<HTMLElement>) => any;
+  onClick: (notice: T) => void;
  onVisibilityToggle?: (expanded: boolean) => any;
  compact?: boolean;
  className?: string;
 }

-interface State {
-  collapsed: boolean;
-}
-
 const Wrapper = styled.div`
  width: 100%;

@@ -94,104 +90,109 @@ const Header = styled.header`
  padding-left: var(--padding-md);
 `;

-export class NoticeTable<T extends Notice> extends PureComponent<NoticeTableProps<T>, State> {
-  state: State = {
-    collapsed: false,
-  };
+const NoticeRow = <T extends Notice>({
+  notice,
+  onClick: propsOnClick,
+}: PropsWithChildren<{
+  notice: T;
+  onClick:(notice: T) => void;
+}>) => {
+  const onClick = useCallback(() => {
+    propsOnClick?.(notice);
+  }, [notice, propsOnClick]);

-  collapse() {
-    const { onVisibilityToggle } = this.props;
-    this.setState(
-      state => ({
-        collapsed: !state.collapsed,
-      }),
-      () => {
-        if (onVisibilityToggle) {
-          onVisibilityToggle(!this.state.collapsed);
-        }
-      },
-    );
-  }
+  return (
+    <TableRow key={`${notice.line}:${notice.type}:${notice.message}`}>
+      <TableData align="center">
+        <SvgIcon icon={notice.type} />
+      </TableData>
+      <TableData align="center">
+        {notice.line}
+        <JumpButton onClick={onClick}>
+          <SvgIcon icon={IconEnum.arrowRight} />
+        </JumpButton>
+      </TableData>
+      <TableData align="left">{notice.message}</TableData>
+    </TableRow>
+  );
+};

-  onClick(notice: T, event: React.SyntheticEvent<HTMLButtonElement>) {
-    const { onClick } = this.props;
-    onClick?.(notice, event);
-  }
+export const NoticeTable = <T extends Notice>({
+  notices,
+  compact,
+  onClick,
+  onVisibilityToggle,
+}: PropsWithChildren<NoticeTableProps<T>>) => {
+  const [collapsed, setCollapsed] = useState(false);

-  render() {
-    const { notices, compact } = this.props;
-    const { collapsed } = this.state;
-    const caret = collapsed ? (
-      <SvgIcon icon={IconEnum.chevronUp} />
-    ) : (
-      <SvgIcon icon={IconEnum.chevronDown} />
-    );
-    const errors = notices.filter(n => n.type === 'error');
-    const warnings = notices.filter(n => n.type === 'warning');
-    return (
-      <Wrapper>
-        <Header>
-          <div>
-            {errors.length > 0 && (
-              <ErrorCount>
-                <SvgIcon icon={IconEnum.error} label={errors.length} />
-              </ErrorCount>
-            )}
-            {warnings.length > 0 && (
-              <ErrorCount>
-                <SvgIcon icon={IconEnum.warning} label={warnings.length} />
-              </ErrorCount>
-            )}
-          </div>
-          {/* @ts-expect-error TSCONVERSION */}
-          <Button onClick={this.collapse.bind(this)} noOutline>
-            {collapsed ? 'Show' : 'Hide'} Details{caret}
-          </Button>
-        </Header>
-        {!collapsed && (
-          <ScrollWrapperStyled>
-            <Table striped compact={compact}>
-              <TableHead>
-                <TableRow>
-                  <TableHeader align="center">Type</TableHeader>
-                  <TableHeader
-                    style={{
-                      minWidth: '3em',
-                    }}
-                    align="center"
-                  >
-                    Line
-                  </TableHeader>
-                  <TableHeader
-                    style={{
-                      width: '100%',
-                    }}
-                    align="left"
-                  >
-                    Message
-                  </TableHeader>
-                </TableRow>
-              </TableHead>
-              <TableBody>
-                {notices.map(notice => (
-                  <TableRow key={`${notice.line}:${notice.type}:${notice.message}`}>
-                    <TableData align="center">
-                      <SvgIcon icon={notice.type} />
-                    </TableData>
-                    <TableData align="center">
-                      {notice.line}
-                      <JumpButton onClick={this.onClick.bind(this, notice)}>
-                        <SvgIcon icon={IconEnum.arrowRight} />
-                      </JumpButton>
-                    </TableData>
-                    <TableData align="left">{notice.message}</TableData>
-                  </TableRow>
-                ))}
-              </TableBody>
-            </Table>
-          </ScrollWrapperStyled>
-        )}
-      </Wrapper>
-    );
-  }
-}
+  const onCollapse = useCallback(() => {
+    setCollapsed(!collapsed);
+    if (onVisibilityToggle) {
+      onVisibilityToggle(!collapsed);
+    }
+  }, [onVisibilityToggle, collapsed]);
+
+  const errors = notices.filter(notice => notice.type === 'error');
+  const warnings = notices.filter(notice => notice.type === 'warning');
+  return (
+    <Wrapper>
+      <Header>
+        <div>
+          {errors.length > 0 && (
+            <ErrorCount>
+              <SvgIcon icon={IconEnum.error} label={errors.length} />
+            </ErrorCount>
+          )}
+          {warnings.length > 0 && (
+            <ErrorCount>
+              <SvgIcon icon={IconEnum.warning} label={warnings.length} />
+            </ErrorCount>
+          )}
+        </div>
+        <Button onClick={onCollapse}>
+          {collapsed ? 'Show' : 'Hide'} Details
+          <SvgIcon
+            style={{ marginLeft: 'var(--padding-xs)' }}
+            icon={collapsed ? IconEnum.chevronUp : IconEnum.chevronDown}
+          />
+        </Button>
+      </Header>
+      {!collapsed && (
+        <ScrollWrapperStyled>
+          <Table striped compact={compact}>
+            <TableHead>
+              <TableRow>
+                <TableHeader align="center">Type</TableHeader>
+                <TableHeader
+                  style={{
+                    minWidth: '3em',
+                  }}
+                  align="center"
+                >
+                  Line
+                </TableHeader>
+                <TableHeader
+                  style={{
+                    width: '100%',
+                  }}
+                  align="left"
+                >
+                  Message
+                </TableHeader>
+              </TableRow>
+            </TableHead>
+            <TableBody>
+              {notices.map(notice => (
+                <NoticeRow
+                  key={`${notice.line}${notice.message}`}
+                  notice={notice}
+                  onClick={onClick}
+                />
+              ))}
+            </TableBody>
+          </Table>
+        </ScrollWrapperStyled>
+      )}
+    </Wrapper>
+  );
+};
--- a/packages/insomnia/src/ui/components/base/button.tsx
+++ b/packages/insomnia/src/ui/components/base/button.tsx
@@ -7,8 +7,8 @@ export interface ButtonProps<T> {
  children: ReactNode;
  value?: T;
  className?: string;
-  onDisabledClick?: React.MouseEventHandler<HTMLButtonElement> | ((value: T | undefined, event: React.MouseEvent<HTMLButtonElement>) => void);
-  onClick?: React.MouseEventHandler<HTMLButtonElement> | ((value: T | undefined, event: React.MouseEvent<HTMLButtonElement>) => void);
+  onDisabledClick?: (event: React.MouseEvent<HTMLButtonElement>, value?: T) => void;
+  onClick?: (event: React.MouseEvent<HTMLButtonElement>, value?: T) => void;
  disabled?: boolean;
  tabIndex?: number;
  type?: ButtonHTMLAttributes<HTMLButtonElement>['type'];
@@ -24,10 +24,8 @@ export class Button<T> extends PureComponent<ButtonProps<T>> {
    const fn = disabled ? onDisabledClick : onClick;

    if (this.props.hasOwnProperty('value')) {
-      // @ts-expect-error -- TSCONVERSION we really need to make the `value` argument come second
-      fn?.(value, event);
+      fn?.(event, value);
    } else {
-      // @ts-expect-error -- TSCONVERSION we really need to make the `value` argument come second
      fn?.(event);
    }
  }
--- a/packages/insomnia/src/ui/components/base/copy-button.tsx
+++ b/packages/insomnia/src/ui/components/base/copy-button.tsx
@@ -24,7 +24,7 @@ export class CopyButton extends PureComponent<Props, State> {

  _triggerTimeout: NodeJS.Timeout | null = null;

-  async _handleClick(event) {
+  async _handleClick(event: React.MouseEvent<HTMLButtonElement>) {
    event.preventDefault();
    event.stopPropagation();
    const content =
--- a/packages/insomnia/src/ui/components/base/prompt-button.tsx
+++ b/packages/insomnia/src/ui/components/base/prompt-button.tsx
@@ -13,14 +13,14 @@ type States =
 const STATE_DEFAULT = 'default';
 const STATE_ASK = 'ask';
 const STATE_DONE = 'done';
-interface Props {
-  onClick?: React.MouseEventHandler<HTMLButtonElement> | ((value: any, event: React.MouseEvent<HTMLButtonElement>) => void);
+interface Props<T> {
+  onClick?: (event: React.MouseEvent<HTMLButtonElement>, value?: T) => void;
  addIcon?: boolean;
  children?: ReactNode;
  disabled?: boolean;
  confirmMessage?: string;
  doneMessage?: string;
-  value?: any;
+  value?: T;
  tabIndex?: number;
  className?: string;
  // TODO(TSCONVERSION) this is linked to Button not using title
@@ -32,22 +32,21 @@ interface State {
 }

@autoBindMethodsForReact(AUTOBIND_CFG)
-export class PromptButton extends PureComponent<Props> {
+export class PromptButton<T> extends PureComponent<Props<T>> {
  _doneTimeout: NodeJS.Timeout | null = null;
  _triggerTimeout: NodeJS.Timeout | null = null;
  state: State = {
    state: STATE_DEFAULT,
  };

-  _confirm(...args) {
+  _confirm(event: React.MouseEvent<HTMLButtonElement>, value?: T) {
    if (this._triggerTimeout !== null) {
      // Clear existing timeouts
      clearTimeout(this._triggerTimeout);
    }

    // Fire the click handler
-    // @ts-expect-error -- TSCONVERSION
-    this.props.onClick?.(...args);
+    this.props.onClick?.(event, value);
    // Set the state to done (but delay a bit to not alarm user)
    // using global.setTimeout to force use of the Node timeout rather than DOM timeout
    this._doneTimeout = global.setTimeout(() => {
@@ -64,8 +63,7 @@ export class PromptButton extends PureComponent<Props> {
    }, 2000);
  }

-  _ask(...args) {
-    const event = args[args.length - 1];
+  _ask(event: React.MouseEvent<HTMLButtonElement>) {
    // Prevent events (ex. won't close dropdown if it's in one)
    event.preventDefault();
    event.stopPropagation();
@@ -82,13 +80,13 @@ export class PromptButton extends PureComponent<Props> {
    }, 2000);
  }

-  _handleClick(...args) {
+  _handleClick(event: React.MouseEvent<HTMLButtonElement>, value?: T) {
    const { state } = this.state;

    if (state === STATE_ASK) {
-      this._confirm(...args);
+      this._confirm(event, value);
    } else if (state === STATE_DEFAULT) {
-      this._ask(...args);
+      this._ask(event);
    } else {
      // Do nothing
    }
--- a/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx
+++ b/packages/insomnia/src/ui/components/editors/auth/components/auth-input-row.tsx
@@ -24,6 +24,9 @@ export const AuthInputRow: FC<Props> = ({ label, getAutocompleteConstants, prope
  const canBeMasked = !showPasswords && mask;
  const isMasked = canBeMasked && masked;

+  // this handler is needed to ignore the parameters sent by button into onClick...
+  const onClick = useCallback(() => toggleMask(), [toggleMask]);
+
  const onChange = useCallback((value: string) => patchAuth({ [property]: value }), [patchAuth, property]);

  const id = toKebabCase(label);
@@ -42,8 +45,7 @@ export const AuthInputRow: FC<Props> = ({ label, getAutocompleteConstants, prope
      {canBeMasked ? (
        <Button
          className="btn btn--super-duper-compact pointer"
-          // inline function needed to ignore the parameters sent by button into onClick...
-          onClick={() => toggleMask()}
+          onClick={onClick}
          value={isMasked}
        >
          {isMasked ? <i className="fa fa-eye" data-testid="reveal-password-icon" /> : <i className="fa fa-eye-slash" data-testid="mask-password-icon" />}
--- a/packages/insomnia/src/ui/components/editors/auth/components/auth-toggle-row.tsx
+++ b/packages/insomnia/src/ui/components/editors/auth/components/auth-toggle-row.tsx
@@ -27,7 +27,10 @@ export const AuthToggleRow: FC<Props> = ({
  const { activeRequest: { authentication }, patchAuth } = useActiveRequest();

  const databaseValue = Boolean(authentication[property]);
-  const toggle = useCallback((value: boolean) => patchAuth({ [property]: value }), [patchAuth, property]);
+  const toggle = useCallback(
+    (_event: React.MouseEvent, value: boolean) =>
+      patchAuth({ [property]: value }), [patchAuth, property]
+  );

  const isActuallyOn = invert ? !databaseValue : databaseValue;

--- a/packages/insomnia/src/ui/components/key-value-editor/row.tsx
+++ b/packages/insomnia/src/ui/components/key-value-editor/row.tsx
@@ -188,7 +188,7 @@ class KeyValueEditorRowInternal extends PureComponent<Props, State> {
    });
  }

-  _handleDisableChange(disabled: boolean) {
+  _handleDisableChange(_event: React.MouseEvent, disabled: boolean) {
    this._sendChange({
      disabled,
    });
--- a/packages/insomnia/src/ui/components/modals/prompt-modal.tsx
+++ b/packages/insomnia/src/ui/components/modals/prompt-modal.tsx
@@ -100,11 +100,11 @@ export class PromptModal extends PureComponent<{}, State> {
    this.modal = modal;
  }

-  _handleSelectHint(hint: string) {
+  _handleSelectHint(_event: React.MouseEvent, hint: string) {
    this._done(hint);
  }

-  _handleDeleteHint(hint: string) {
+  _handleDeleteHint(_event: React.MouseEvent, hint: string) {
    const { onDeleteHint } = this.state;
    onDeleteHint?.(hint);
    const hints = this.state.hints.filter(h => h !== hint);
--- a/packages/insomnia/src/ui/components/modals/request-switcher-modal.tsx
+++ b/packages/insomnia/src/ui/components/modals/request-switcher-modal.tsx
@@ -3,7 +3,7 @@ import classnames from 'classnames';
 import { buildQueryStringFromParams, joinUrlAndQueryString } from 'insomnia-url';
 import React, { Fragment, PureComponent } from 'react';
 import { connect } from 'react-redux';
-import { bindActionCreators } from 'redux';
+import { AnyAction, bindActionCreators, Dispatch } from 'redux';

 import { AUTOBIND_CFG, METHOD_GRPC } from '../../../common/constants';
 import { hotKeyRefs } from '../../../common/hotkeys';
@@ -39,7 +39,7 @@ const mapStateToProps = (state: RootState) => ({
  workspaceRequestsAndRequestGroups: selectWorkspaceRequestsAndRequestGroups(state),
 });

-const mapDispatchToProps = dispatch => {
+const mapDispatchToProps = (dispatch: Dispatch<AnyAction>) => {
  const bound = bindActionCreators({ activateWorkspace }, dispatch);
  return {
    handleActivateWorkspace: bound.activateWorkspace,
@@ -159,13 +159,17 @@ class RequestSwitcherModal extends PureComponent<Props, State> {
    this._activateRequest(request);
  }

-  async _activateWorkspace(workspace: Workspace) {
+  async _activateWorkspace(workspace?: Workspace) {
+    if (!workspace) {
+      return;
+    }
+
    await this.props.handleActivateWorkspace({ workspace });

    this.modal?.hide();
  }

-  _activateRequest(request: Request | GrpcRequest) {
+  _activateRequest(request?: Request | GrpcRequest) {
    if (!request) {
      return;
    }
@@ -238,7 +242,7 @@ class RequestSwitcherModal extends PureComponent<Props, State> {
  _handleChangeValue(searchString: string) {
    const { workspace, workspaceRequestsAndRequestGroups, workspacesForActiveProject, requestMetas, grpcRequestMetas, activeRequest } = this.props;
    const { maxRequests, maxWorkspaces, hideNeverActiveRequests } = this.state;
-    const lastActiveMap = {};
+    const lastActiveMap: Record<string, number> = {};

    for (const meta of requestMetas) {
      lastActiveMap[meta.parentId] = meta.lastActive;
@@ -442,7 +446,7 @@ class RequestSwitcherModal extends PureComponent<Props, State> {
              </div>
            )}
            <ul>
-              {matchedRequests.map((r, i) => {
+              {matchedRequests.map((r: Request | GrpcRequest, i) => {
                const requestGroup = requestGroups.find(rg => rg._id === r.parentId);
                const buttonClasses = classnames(
                  'btn btn--expandable-small wide text-left pad-bottom',
@@ -452,7 +456,7 @@ class RequestSwitcherModal extends PureComponent<Props, State> {
                );
                return (
                  <li key={r._id}>
-                    <Button onClick={this._activateRequest} value={r} className={buttonClasses}>
+                    <Button onClick={(_e, r) => this._activateRequest(r)} value={r} className={buttonClasses}>
                      <div>
                        {requestGroup ? (
                          <div className="pull-right faint italic">
@@ -485,7 +489,7 @@ class RequestSwitcherModal extends PureComponent<Props, State> {
                });
                return (
                  <li key={w._id}>
-                    <Button onClick={this._activateWorkspace} value={w} className={buttonClasses}>
+                    <Button onClick={(_e, value) => this._activateWorkspace(value)} value={w} className={buttonClasses}>
                      <i className="fa fa-random" />
                      &nbsp;&nbsp;&nbsp; Switch to <strong>{w.name}</strong>
                    </Button>
--- a/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx
+++ b/packages/insomnia/src/ui/components/modals/workspace-environments-edit-modal.tsx
@@ -53,7 +53,7 @@ const SidebarListItem = SortableElement<SidebarListItemProps>(({
  handleActivateEnvironment,
  selectedEnvironment,
  showEnvironment,
-}) => {
+}: SidebarListItemProps) => {
  const classes = classnames({
    'env-modal__sidebar-item': true,
    'env-modal__sidebar-item--active': selectedEnvironment === environment,
@@ -113,7 +113,7 @@ const SidebarList = SortableContainer<SidebarListProps>(
    handleActivateEnvironment,
    selectedEnvironment,
    showEnvironment,
-  }) => (
+  }: SidebarListProps) => (
    <ul>
      {environments.map((environment, index) => (
        <SidebarListItem
@@ -220,7 +220,7 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent<Props, State>
    await this._load(workspace, environment);
  }

-  _handleShowEnvironment(environment: Environment) {
+  _handleShowEnvironment(_event: React.MouseEvent, environment?: Environment) {
    // Don't allow switching if the current one has errors
    if (this.environmentEditorRef && !this.environmentEditorRef.isValid()) {
      return;
@@ -234,13 +234,13 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent<Props, State>
    this._load(workspace, environment);
  }

-  async _handleDuplicateEnvironment(environment: Environment) {
+  async _handleDuplicateEnvironment(_event: React.MouseEvent, environment: Environment) {
    const { workspace } = this.state;
    const newEnvironment = await models.environment.duplicate(environment);
    await this._load(workspace, newEnvironment);
  }

-  async _handleDeleteEnvironment(environment: Environment) {
+  async _handleDeleteEnvironment(_event: React.MouseEvent, environment: Environment) {
    const { handleChangeEnvironment, activeEnvironmentId } = this.props;
    const { rootEnvironment, workspace } = this.state;

@@ -377,12 +377,12 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent<Props, State>
      return;
    }

-    let patch;
+    let patch: Partial<Environment>;

    try {
      const data = this.environmentEditorRef.getValue();
      patch = {
-        data: data && data.object,
+        data: data?.object,
        dataPropertyOrder: data && data.propertyOrder,
      };
    } catch (err) {
@@ -410,15 +410,19 @@ export class WorkspaceEnvironmentsEditModal extends PureComponent<Props, State>
    this._handleChangeEnvironmentColor(environment, null);
  }

-  _handleActivateEnvironment: ButtonProps<Environment>['onClick'] = (environment: Environment) => {
+  _handleActivateEnvironment: ButtonProps<Environment>['onClick'] = (event, environment) => {
    const { handleChangeEnvironment, activeEnvironmentId } = this.props;

+    if (!environment) {
+      return;
+    }
+
    if (environment._id === activeEnvironmentId) {
      return;
    }

    handleChangeEnvironment(environment._id);
-    this._handleShowEnvironment(environment);
+    this._handleShowEnvironment(event, environment);
  };

  render() {