Browse Source

ref(timeouts): make sure we clear timeouts (#33029)

* ref(setTimeout): use window.setTimeout so we dont end up with NodeJS.timeout

* fix(timeouts): make sure we clear timeouts

* fix(timeouts): revert replace on compiled code

* fix(timeouts): call clearTimeout with ref

* fix(timeouts): revert to setTimeout

* fix(navigateTo): store timeout from return value and clear it

* fix(import): fix static/app/views/organizationIntegrations/sentryAppDetailedView.tsx import

* fix(timeouts): call clearTimeout with ref

* fix(setTimeout): reference window

* fix(genericQueryBatcher): reference the right timeout

* fix(groupdetails): it is actually an interval

* ref(setInterval): make sure we clear setInterval (#33038)

* revert(navigateTo): revert to master

* fix(): fix timeout leak

* style(lint): Auto commit lint changes

* style(lint): Auto commit lint changes

* style(lint): Auto commit lint changes

* style(lint): Auto commit lint changes

* fix(onboarding): fix react imports

* fix(indicatorStore): add back unsubscribe listeners

* fix(autocomplete): revert to using arrow fn

* fix(autocomplete): revert to binding

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Jonas 2 years ago
parent
commit
10780656ff

+ 1 - 1
static/app/actionCreators/navigation.tsx

@@ -33,7 +33,7 @@ export function navigateTo(
           }
           onFinish={path => {
             modalProps.closeModal();
-            setTimeout(() => router.push(path), 0);
+            return window.setTimeout(() => router.push(path), 0);
           }}
         />
       ),

+ 52 - 40
static/app/components/autoComplete.tsx

@@ -133,28 +133,34 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
     this.items.clear();
   }
 
+  componentWillUnmount() {
+    if (this.blurTimeout) {
+      window.clearTimeout(this.blurTimeout);
+    }
+  }
+
   items = new Map();
-  blurTimer: any;
+  blurTimeout: number | null = null;
   itemCount?: number;
 
-  isControlled = () => typeof this.props.isOpen !== 'undefined';
-
-  getOpenState = () => {
-    const {isOpen} = this.props;
+  isControlled() {
+    return typeof this.props.isOpen !== 'undefined';
+  }
 
-    return this.isControlled() ? isOpen : this.state.isOpen;
-  };
+  getOpenState() {
+    return this.isControlled() ? this.props.isOpen : this.state.isOpen;
+  }
 
   /**
    * Resets `this.items` and `this.state.highlightedIndex`.
    * Should be called whenever `inputValue` changes.
    */
-  resetHighlightState = () => {
+  resetHighlightState() {
     // reset items and expect `getInputProps` in child to give us a list of new items
     this.setState({
       highlightedIndex: this.props.defaultHighlightedIndex || 0,
     });
-  };
+  }
 
   handleInputChange =
     <E extends HTMLInputElement>({onChange}: Pick<GetInputArgs<E>, 'onChange'>) =>
@@ -188,22 +194,25 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
    * Clicks outside should close the dropdown immediately via <DropdownMenu />,
    * however blur via keyboard will have a 200ms delay
    */
-  handleInputBlur =
-    <E extends HTMLInputElement>({onBlur}: Pick<GetInputArgs<E>, 'onBlur'>) =>
-    (e: React.FocusEvent<E>) => {
-      this.blurTimer = window.setTimeout(() => {
+  handleInputBlur<E extends HTMLInputElement>({onBlur}: Pick<GetInputArgs<E>, 'onBlur'>) {
+    return (e: React.FocusEvent<E>) => {
+      if (this.blurTimeout) {
+        window.clearTimeout(this.blurTimeout);
+      }
+      this.blurTimeout = window.setTimeout(() => {
         this.closeMenu();
         onBlur?.(e);
       }, 200);
     };
+  }
 
   // Dropdown detected click outside, we should close
-  handleClickOutside = async () => {
+  async handleClickOutside() {
     // Otherwise, it's possible that this gets fired multiple times
     // e.g. click outside triggers closeMenu and at the same time input gets blurred, so
     // a timer is set to close the menu
-    if (this.blurTimer) {
-      clearTimeout(this.blurTimer);
+    if (this.blurTimeout) {
+      window.clearTimeout(this.blurTimeout);
     }
 
     // Wait until the current macrotask completes, in the case that the click
@@ -213,7 +222,7 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
     await new Promise(resolve => window.setTimeout(resolve));
 
     this.closeMenu();
-  };
+  }
 
   handleInputKeyDown =
     <E extends HTMLInputElement>({onKeyDown}: Pick<GetInputArgs<E>, 'onKeyDown'>) =>
@@ -250,20 +259,20 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
       onKeyDown?.(e);
     };
 
-  handleItemClick =
-    ({item, index}: GetItemArgs<T>) =>
-    (e: React.MouseEvent) => {
+  handleItemClick({item, index}: GetItemArgs<T>) {
+    return (e: React.MouseEvent) => {
       if (item.disabled) {
         return;
       }
 
-      if (this.blurTimer) {
-        clearTimeout(this.blurTimer);
+      if (this.blurTimeout) {
+        window.clearTimeout(this.blurTimeout);
       }
 
       this.setState({highlightedIndex: index});
       this.handleSelect(item, e);
     };
+  }
 
   handleItemMouseEnter =
     ({item, index}: GetItemArgs<T>) =>
@@ -274,19 +283,22 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
       this.setState({highlightedIndex: index});
     };
 
-  handleMenuMouseDown = () => {
+  handleMenuMouseDown() {
+    if (this.blurTimeout) {
+      window.clearTimeout(this.blurTimeout);
+    }
     // Cancel close menu from input blur (mouseDown event can occur before input blur :()
-    window.setTimeout(() => {
-      if (this.blurTimer) {
-        clearTimeout(this.blurTimer);
+    this.blurTimeout = window.setTimeout(() => {
+      if (this.blurTimeout) {
+        window.clearTimeout(this.blurTimeout);
       }
     });
-  };
+  }
 
   /**
    * When an item is selected via clicking or using the keyboard (e.g. pressing "Enter")
    */
-  handleSelect = (item: T, e: React.MouseEvent | React.KeyboardEvent) => {
+  handleSelect(item: T, e: React.MouseEvent | React.KeyboardEvent) {
     const {onSelect, itemToString, closeOnSelect} = this.props;
 
     onSelect?.(item, this.state, e);
@@ -302,7 +314,7 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
     }
 
     this.setState({selectedItem: item});
-  };
+  }
 
   moveHighlightedIndex(step: number) {
     let newIndex = this.state.highlightedIndex + step;
@@ -342,7 +354,7 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
    *
    * This is exposed to render function
    */
-  closeMenu = (...args: Array<any>) => {
+  closeMenu(...args: Array<any>) {
     const {onClose, resetInputOnClose} = this.props;
 
     onClose?.(...args);
@@ -355,11 +367,11 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
       isOpen: false,
       inputValue: resetInputOnClose ? '' : state.inputValue,
     }));
-  };
+  }
 
-  getInputProps = <E extends HTMLInputElement>(
+  getInputProps<E extends HTMLInputElement>(
     inputProps?: GetInputArgs<E>
-  ): GetInputOutput<E> => {
+  ): GetInputOutput<E> {
     const {onChange, onKeyDown, onFocus, onBlur, ...rest} = inputProps ?? {};
     return {
       ...rest,
@@ -369,7 +381,7 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
       onFocus: this.handleInputFocus<E>({onFocus}),
       onBlur: this.handleInputBlur<E>({onBlur}),
     };
-  };
+  }
 
   getItemProps = (itemProps: GetItemArgs<T>) => {
     const {item, index, ...props} = itemProps ?? {};
@@ -390,14 +402,14 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
     };
   };
 
-  getMenuProps = <E extends Element>(props?: GetMenuArgs<E>): GetMenuArgs<E> => {
+  getMenuProps<E extends Element>(props?: GetMenuArgs<E>): GetMenuArgs<E> {
     this.itemCount = props?.itemCount;
 
     return {
       ...(props ?? {}),
-      onMouseDown: this.handleMenuMouseDown,
+      onMouseDown: this.handleMenuMouseDown.bind(this),
     };
-  };
+  }
 
   render() {
     const {children, onMenuOpen, inputIsActor} = this.props;
@@ -407,7 +419,7 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
     return (
       <DropdownMenu
         isOpen={isOpen}
-        onClickOutside={this.handleClickOutside}
+        onClickOutside={this.handleClickOutside.bind(this)}
         onOpen={onMenuOpen}
       >
         {dropdownMenuProps =>
@@ -431,8 +443,8 @@ class AutoComplete<T extends Item> extends React.Component<Props<T>, State<T>> {
             selectedItem,
             highlightedIndex,
             actions: {
-              open: this.openMenu,
-              close: this.closeMenu,
+              open: this.openMenu.bind(this),
+              close: this.closeMenu.bind(this),
             },
           })
         }

+ 11 - 2
static/app/components/bases/pluginComponentBase.tsx

@@ -51,8 +51,17 @@ class PluginComponentBase<
 
   componentWillUnmount() {
     this.api.clear();
+    if (this.successMessageTimeout) {
+      window.clearTimeout(this.successMessageTimeout);
+    }
+    if (this.errorMessageTimeout) {
+      window.clearTimeout(this.errorMessageTimeout);
+    }
   }
 
+  successMessageTimeout: number | null = null;
+  errorMessageTimeout: number | null = null;
+
   api = new Client();
 
   fetchData() {
@@ -112,7 +121,7 @@ class PluginComponentBase<
       },
       () => callback && callback()
     );
-    window.setTimeout(() => {
+    this.successMessageTimeout = window.setTimeout(() => {
       addSuccessMessage(t('Success!'));
     }, 0);
   }
@@ -125,7 +134,7 @@ class PluginComponentBase<
       },
       () => callback && callback()
     );
-    window.setTimeout(() => {
+    this.errorMessageTimeout = window.setTimeout(() => {
       addErrorMessage(t('Unable to save changes. Please try again.'));
     }, 0);
   }

+ 8 - 1
static/app/components/charts/pieChart.tsx

@@ -29,9 +29,16 @@ class PieChart extends React.Component<Props> {
 
     // Timeout is because we need to wait for rendering animation to complete
     // And I haven't found a callback for this
-    window.setTimeout(() => this.highlight(0), 1000);
+    this.highlightTimeout = window.setTimeout(() => this.highlight(0), 1000);
   }
 
+  componentWillUnmount() {
+    if (this.highlightTimeout) {
+      window.clearTimeout(this.highlightTimeout);
+    }
+  }
+
+  highlightTimeout: number | null = null;
   isInitialSelected = true;
   selected = 0;
   chart = React.createRef<ReactEchartsRef>();

+ 27 - 13
static/app/components/contextPickerModal.tsx

@@ -43,7 +43,7 @@ type Props = ModalRenderProps & {
   /**
    * Finish callback
    */
-  onFinish: (path: string) => void;
+  onFinish: (path: string) => number | void;
   /**
    * Callback for when organization is selected
    */
@@ -108,6 +108,14 @@ class ContextPickerModal extends Component<Props> {
     }
   }
 
+  componentWillUnmount() {
+    if (this.onFinishTimeout) {
+      window.clearTimeout(this.onFinishTimeout);
+    }
+  }
+
+  onFinishTimeout: number | null = null;
+
   // TODO(ts) The various generics in react-select types make getting this
   // right hard.
   orgSelect: any | null = null;
@@ -136,13 +144,18 @@ class ContextPickerModal extends Component<Props> {
       return;
     }
 
+    if (this.onFinishTimeout) {
+      window.clearTimeout(this.onFinishTimeout);
+    }
+
     // If there is only one org and we don't need a project slug, then call finish callback
     if (!needProject) {
-      onFinish(
-        replaceRouterParams(nextPath, {
-          orgId: organizations[0].slug,
-        })
-      );
+      this.onFinishTimeout =
+        onFinish(
+          replaceRouterParams(nextPath, {
+            orgId: organizations[0].slug,
+          })
+        ) ?? null;
       return;
     }
 
@@ -152,13 +165,14 @@ class ContextPickerModal extends Component<Props> {
       org = organizations[0].slug;
     }
 
-    onFinish(
-      replaceRouterParams(nextPath, {
-        orgId: org,
-        projectId: projects[0].slug,
-        project: this.props.projects.find(p => p.slug === projects[0].slug)?.id,
-      })
-    );
+    this.onFinishTimeout =
+      onFinish(
+        replaceRouterParams(nextPath, {
+          orgId: org,
+          projectId: projects[0].slug,
+          project: this.props.projects.find(p => p.slug === projects[0].slug)?.id,
+        })
+      ) ?? null;
   };
 
   doFocus = (ref: any | null) => {

+ 10 - 0
static/app/components/dashboards/issueWidgetQueriesForm.tsx

@@ -44,6 +44,13 @@ class IssueWidgetQueriesForm extends React.Component<Props, State> {
       blurTimeout: undefined,
     };
   }
+
+  componentWillUnmount() {
+    if (this.state.blurTimeout) {
+      window.clearTimeout(this.state.blurTimeout);
+    }
+  }
+
   // Handle scalar field values changing.
   handleFieldChange = (field: string) => {
     const {query, onChange} = this.props;
@@ -83,6 +90,9 @@ class IssueWidgetQueriesForm extends React.Component<Props, State> {
                 // this, we set a timer in our onSearch handler to block our onBlur
                 // handler from firing if it is within 200ms, ie from clicking an
                 // autocomplete value.
+                if (this.state.blurTimeout) {
+                  window.clearTimeout(this.state.blurTimeout);
+                }
                 this.setState({
                   blurTimeout: window.setTimeout(() => {
                     this.setState({blurTimeout: undefined});

+ 12 - 0
static/app/components/dashboards/widgetQueriesForm.tsx

@@ -90,6 +90,12 @@ type Props = {
  * callback. This component's state should live in the parent.
  */
 class WidgetQueriesForm extends React.Component<Props> {
+  componentWillUnmount() {
+    if (this.blurTimeout) {
+      window.clearTimeout(this.blurTimeout);
+    }
+  }
+
   blurTimeout: number | null = null;
 
   // Handle scalar field values changing.
@@ -128,6 +134,9 @@ class WidgetQueriesForm extends React.Component<Props> {
           // this, we set a timer in our onSearch handler to block our onBlur
           // handler from firing if it is within 200ms, ie from clicking an
           // autocomplete value.
+          if (this.blurTimeout) {
+            window.clearTimeout(this.blurTimeout);
+          }
           this.blurTimeout = window.setTimeout(() => {
             this.blurTimeout = null;
           }, 200);
@@ -150,6 +159,9 @@ class WidgetQueriesForm extends React.Component<Props> {
           // this, we set a timer in our onSearch handler to block our onBlur
           // handler from firing if it is within 200ms, ie from clicking an
           // autocomplete value.
+          if (this.blurTimeout) {
+            window.clearTimeout(this.blurTimeout);
+          }
           this.blurTimeout = window.setTimeout(() => {
             this.blurTimeout = null;
           }, 200);

+ 23 - 18
static/app/components/dropdownMenu.tsx

@@ -119,15 +119,21 @@ class DropdownMenu extends React.Component<Props, State> {
     isOpen: false,
   };
 
-  componentWillUnmount() {
-    document.removeEventListener('click', this.checkClickOutside, true);
-  }
-
   dropdownMenu: Element | null = null;
   dropdownActor: Element | null = null;
 
-  mouseLeaveId: number | null = null;
-  mouseEnterId: number | null = null;
+  mouseLeaveTimeout: number | null = null;
+  mouseEnterTimeout: number | null = null;
+
+  componentWillUnmount() {
+    if (this.mouseLeaveTimeout) {
+      window.clearTimeout(this.mouseLeaveTimeout);
+    }
+    if (this.mouseEnterTimeout) {
+      window.clearTimeout(this.mouseEnterTimeout);
+    }
+    document.removeEventListener('click', this.checkClickOutside, true);
+  }
 
   // Gets open state from props or local state when appropriate
   isOpen = () => {
@@ -194,8 +200,8 @@ class DropdownMenu extends React.Component<Props, State> {
       });
     }
 
-    if (this.mouseLeaveId) {
-      window.clearTimeout(this.mouseLeaveId);
+    if (this.mouseLeaveTimeout) {
+      window.clearTimeout(this.mouseLeaveTimeout);
     }
 
     // If we always render menu (e.g. DropdownLink), then add the check click outside handlers when we open the menu
@@ -212,8 +218,7 @@ class DropdownMenu extends React.Component<Props, State> {
   // Decide whether dropdown should be closed when mouse leaves element
   // Only for nested dropdowns
   handleMouseLeave = (e: React.MouseEvent<Element>) => {
-    const {isNestedDropdown} = this.props;
-    if (!isNestedDropdown) {
+    if (!this.props.isNestedDropdown) {
       return;
     }
 
@@ -224,7 +229,7 @@ class DropdownMenu extends React.Component<Props, State> {
         this.dropdownMenu &&
         (!(toElement instanceof Element) || !this.dropdownMenu.contains(toElement))
       ) {
-        this.mouseLeaveId = window.setTimeout(() => {
+        this.mouseLeaveTimeout = window.setTimeout(() => {
           this.handleClose(e);
         }, MENU_CLOSE_DELAY);
       }
@@ -349,11 +354,11 @@ class DropdownMenu extends React.Component<Props, State> {
           return;
         }
 
-        if (this.mouseLeaveId) {
-          window.clearTimeout(this.mouseLeaveId);
+        if (this.mouseLeaveTimeout) {
+          window.clearTimeout(this.mouseLeaveTimeout);
         }
 
-        this.mouseEnterId = window.setTimeout(() => {
+        this.mouseEnterTimeout = window.setTimeout(() => {
           this.handleOpen(e);
         }, MENU_CLOSE_DELAY);
       },
@@ -363,8 +368,8 @@ class DropdownMenu extends React.Component<Props, State> {
           onMouseLeave(e);
         }
 
-        if (this.mouseEnterId) {
-          window.clearTimeout(this.mouseEnterId);
+        if (this.mouseEnterTimeout) {
+          window.clearTimeout(this.mouseEnterTimeout);
         }
         this.handleMouseLeave(e);
       },
@@ -406,8 +411,8 @@ class DropdownMenu extends React.Component<Props, State> {
         }
 
         // There is a delay before closing a menu on mouse leave, cancel this action if mouse enters menu again
-        if (this.mouseLeaveId) {
-          window.clearTimeout(this.mouseLeaveId);
+        if (this.mouseLeaveTimeout) {
+          window.clearTimeout(this.mouseLeaveTimeout);
         }
       },
       onMouseLeave: (e: React.MouseEvent<E>) => {

+ 13 - 4
static/app/components/hovercard.tsx

@@ -79,12 +79,21 @@ interface HovercardProps {
 function Hovercard(props: HovercardProps): React.ReactElement {
   const [visible, setVisible] = useState(false);
 
-  const inTimeout = useRef<number | null>(null);
   const scheduleUpdateRef = useRef<(() => void) | null>(null);
 
   const portalEl = useMemo(() => findOrCreatePortal(), []);
   const tooltipId = useMemo(() => domId('hovercard-'), []);
 
+  const showHoverCardTimeoutRef = useRef<number | null>(null);
+
+  useEffect(() => {
+    return () => {
+      if (showHoverCardTimeoutRef.current) {
+        window.clearTimeout(showHoverCardTimeoutRef.current);
+      }
+    };
+  }, []);
+
   useEffect(() => {
     // We had a problem with popper not recalculating position when body/header changed while hovercard still opened.
     // This can happen for example when showing a loading spinner in a hovercard and then changing it to the actual content once fetch finishes.
@@ -96,12 +105,12 @@ function Hovercard(props: HovercardProps): React.ReactElement {
   const toggleHovercard = useCallback(
     (value: boolean) => {
       // If a previous timeout is set, then clear it
-      if (typeof inTimeout.current === 'number') {
-        clearTimeout(inTimeout.current);
+      if (typeof showHoverCardTimeoutRef.current === 'number') {
+        clearTimeout(showHoverCardTimeoutRef.current);
       }
 
       // Else enqueue a new timeout
-      inTimeout.current = window.setTimeout(
+      showHoverCardTimeoutRef.current = window.setTimeout(
         () => setVisible(value),
         props.displayTimeout ?? 100
       );

+ 11 - 1
static/app/components/inputInline.tsx

@@ -65,6 +65,13 @@ class InputInline extends React.Component<Props, State> {
     isHovering: false,
   };
 
+  componentWillUnmount() {
+    if (this.onFocusSelectAllTimeout) {
+      window.clearTimeout(this.onFocusSelectAllTimeout);
+    }
+  }
+
+  onFocusSelectAllTimeout: number | null = null;
   private refInput = React.createRef<HTMLDivElement>();
 
   /**
@@ -98,7 +105,10 @@ class InputInline extends React.Component<Props, State> {
     this.setState({isFocused: true});
     callIfFunction(this.props.onFocus, InputInline.setValueOnEvent(event));
     // Wait for the next event loop so that the content region has focus.
-    window.setTimeout(() => document.execCommand('selectAll', false, undefined), 1);
+    this.onFocusSelectAllTimeout = window.setTimeout(
+      () => document.execCommand('selectAll', false, undefined),
+      1
+    );
   };
 
   /**

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