Fix unsafe ref usage

packages/core/src/common/refs.ts

@@ -22,53 +22,51 @@ export interface IRefObject<T extends HTMLElement = HTMLElement> {
 }
 
 export function isRefObject<T extends HTMLElement>(value: IRef<T> | undefined | null): value is IRefObject<T> {
-    return value != null && typeof (value as IRefObject<T>).current !== "undefined";
+    return value != null && typeof value !== "function";
 }
 
-export type IRefCallback<T = HTMLElement> = (ref: T | null) => any;
+export type IRefCallback<T extends HTMLElement = HTMLElement> = (ref: T | null) => any;
 
 export function isRefCallback<T extends HTMLElement>(value: IRef<T> | undefined | null): value is IRefCallback<T> {
     return typeof value === "function";
 }
 
+/**
+ * Assign the given ref to a target, either a React ref object or a callback which takes the ref as its first argument.
+ */
+export function setRef<T extends HTMLElement>(refTarget: IRef<T> | undefined | null, ref: T | null): void {
+    if (isRefObject<T>(refTarget)) {
+        refTarget.current = ref;
+    } else if (isRefCallback(refTarget)) {
+        refTarget(ref);
+    }
+}
+
+/** @deprecated use mergeRefs() instead */
 export function combineRefs<T extends HTMLElement>(ref1: IRefCallback<T>, ref2: IRefCallback<T>) {
     return mergeRefs(ref1, ref2);
 }
 
+/**
+ * Utility for merging refs into one singular callback ref.
+ * If using in a functional component, would recomend using `useMemo` to preserve function identity.
+ */
 export function mergeRefs<T extends HTMLElement>(...refs: Array<IRef<T> | null>): IRefCallback<T> {
     return value => {
         refs.forEach(ref => {
-            if (isRefCallback(ref)) {
-                ref(value);
-            } else if (isRefObject(ref)) {
-                ref.current = value;
-            }
+            setRef(ref, value);
         });
     };
 }
 
-export function getRef<T extends HTMLElement>(ref: T | IRefObject<T> | null) {
+export function getRef<T extends HTMLElement>(ref: T | IRefObject<T> | null): T | null {
     if (ref === null) {
         return null;
     }
 
     return (ref as IRefObject<T>).current ?? (ref as T);
 }
 
-/**
- * Assign the given ref to a target, either a React ref object or a callback which takes the ref as its first argument.
- */
-export function setRef<T extends HTMLElement>(refTarget: IRef<T> | undefined, ref: T | null) {
-    if (refTarget === undefined) {
-        return;
-    }
-    if (isRefObject<T>(refTarget)) {
-        refTarget.current = ref;
-    } else if (isRefCallback(refTarget)) {
-        refTarget(ref);
-    }
-}
-
 /**
  * Creates a ref handler which assigns the ref returned by React for a mounted component to a field on the target object.
  * The target object is usually a component class.
@@ -78,25 +76,10 @@ export function setRef<T extends HTMLElement>(refTarget: IRef<T> | undefined, re
 export function refHandler<T extends HTMLElement, K extends string>(
     refTargetParent: { [k in K]: T | null },
     refTargetKey: K,
-): IRefCallback<T>;
-export function refHandler<T extends HTMLElement, K extends string>(
-    refTargetParent: { [k in K]: T | IRefObject<T> | null },
-    refTargetKey: K,
-    refProp: IRef<T> | undefined | null,
-): IRef<T>;
-export function refHandler<T extends HTMLElement, K extends string>(
-    refTargetParent: { [k in K]: T | IRefObject<T> | null },
-    refTargetKey: K,
     refProp?: IRef<T> | undefined | null,
-) {
-    if (isRefObject<T>(refProp)) {
-        refTargetParent[refTargetKey] = refProp;
-        return refProp;
-    }
+): IRefCallback<T> {
     return (ref: T | null) => {
         refTargetParent[refTargetKey] = ref;
-        if (isRefCallback(refProp)) {
-            refProp(ref);
-        }
+        setRef(refProp, ref);
     };
 }

packages/core/src/components/button/abstractButton.tsx

@@ -21,10 +21,8 @@ import {
     AbstractPureComponent2,
     Alignment,
     Classes,
-    getRef,
     IActionProps,
     IElementRefProps,
-    IRefObject,
     Keys,
     MaybeElement,
     Utils,
@@ -105,7 +103,7 @@ export abstract class AbstractButton<E extends HTMLButtonElement | HTMLAnchorEle
         isActive: false,
     };
 
-    protected abstract buttonRef: HTMLElement | IRefObject<HTMLElement> | null;
+    protected abstract buttonRef: HTMLElement | null;
 
     private currentKeyDown?: number;
 
@@ -165,7 +163,7 @@ export abstract class AbstractButton<E extends HTMLButtonElement | HTMLAnchorEle
         /* eslint-disable deprecation/deprecation */
         if (Keys.isKeyboardClick(e.which)) {
             this.setState({ isActive: false });
-            getRef(this.buttonRef)?.click();
+            this.buttonRef?.click();
         }
         this.currentKeyDown = undefined;
         this.props.onKeyUp?.(e);

packages/core/src/components/button/buttons.tsx

@@ -20,7 +20,7 @@
 import * as React from "react";
 
 import { DISPLAYNAME_PREFIX, removeNonHTMLProps } from "../../common/props";
-import { IRef, IRefObject, refHandler } from "../../common/refs";
+import { IRef, refHandler, setRef } from "../../common/refs";
 import { AbstractButton, IButtonProps, IAnchorButtonProps } from "./abstractButton";
 
 export { IAnchorButtonProps, IButtonProps };
@@ -29,7 +29,7 @@ export class Button extends AbstractButton<HTMLButtonElement> {
     public static displayName = `${DISPLAYNAME_PREFIX}.Button`;
 
     // need to keep this ref so that we can access it in AbstractButton#handleKeyUp
-    public buttonRef: HTMLButtonElement | IRefObject<HTMLButtonElement> | null = null;
+    public buttonRef: HTMLButtonElement | null = null;
 
     protected handleRef: IRef<HTMLButtonElement> = refHandler(this, "buttonRef", this.props.elementRef);
 
@@ -45,13 +45,21 @@ export class Button extends AbstractButton<HTMLButtonElement> {
             </button>
         );
     }
+
+    public componentDidUpdate(prevProps: IButtonProps) {
+        if (prevProps.elementRef !== this.props.elementRef) {
+            setRef(prevProps.elementRef, null);
+            this.handleRef = refHandler(this, "buttonRef", this.props.elementRef);
+            setRef(this.props.elementRef, this.buttonRef);
+        }
+    }
 }
 
 export class AnchorButton extends AbstractButton<HTMLAnchorElement> {
     public static displayName = `${DISPLAYNAME_PREFIX}.AnchorButton`;
 
     // need to keep this ref so that we can access it in AbstractButton#handleKeyUp
-    public buttonRef: HTMLAnchorElement | IRefObject<HTMLAnchorElement> | null = null;
+    public buttonRef: HTMLAnchorElement | null = null;
 
     protected handleRef: IRef<HTMLAnchorElement> = refHandler(this, "buttonRef", this.props.elementRef);
 
@@ -72,4 +80,12 @@ export class AnchorButton extends AbstractButton<HTMLAnchorElement> {
             </a>
         );
     }
+
+    public componentDidUpdate(prevProps: IAnchorButtonProps) {
+        if (prevProps.elementRef !== this.props.elementRef) {
+            setRef(prevProps.elementRef, null);
+            this.handleRef = refHandler(this, "buttonRef", this.props.elementRef);
+            setRef(this.props.elementRef, this.buttonRef);
+        }
+    }
 }

packages/core/src/components/forms/controls.tsx

@@ -22,7 +22,7 @@ import classNames from "classnames";
 import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 
-import { AbstractPureComponent2, Alignment, Classes, IRef, refHandler } from "../../common";
+import { AbstractPureComponent2, Alignment, Classes, IRef, refHandler, setRef } from "../../common";
 import { DISPLAYNAME_PREFIX, HTMLInputProps, IProps } from "../../common/props";
 
 export interface IControlProps extends IProps, HTMLInputProps {
@@ -266,8 +266,13 @@ export class Checkbox extends AbstractPureComponent2<ICheckboxProps, ICheckboxSt
         this.updateIndeterminate();
     }
 
-    public componentDidUpdate() {
+    public componentDidUpdate(prevProps: ICheckboxProps) {
         this.updateIndeterminate();
+        if (prevProps.inputRef !== this.props.inputRef) {
+            setRef(prevProps.inputRef, null);
+            this.handleInputRef = refHandler(this, "input", this.props.inputRef);
+            setRef(this.props.inputRef, this.input);
+        }
     }
 
     private updateIndeterminate() {

packages/core/src/components/forms/numericInput.tsx

@@ -34,6 +34,7 @@ import {
     Position,
     refHandler,
     removeNonHTMLProps,
+    setRef,
     Utils,
 } from "../../common";
 import * as Errors from "../../common/errors";
@@ -343,6 +344,12 @@ export class NumericInput extends AbstractPureComponent2<HTMLInputProps & INumer
     public componentDidUpdate(prevProps: INumericInputProps, prevState: INumericInputState) {
         super.componentDidUpdate(prevProps, prevState);
 
+        if (prevProps.inputRef !== this.props.inputRef) {
+            setRef(prevProps.inputRef, null);
+            this.inputRef = refHandler(this, "inputElement", this.props.inputRef);
+            setRef(this.props.inputRef, this.inputElement);
+        }
+
         if (this.state.shouldSelectAfterUpdate) {
             this.inputElement?.setSelectionRange(0, this.state.value.length);
         }

packages/core/src/components/forms/textArea.tsx

@@ -18,16 +18,7 @@ import classNames from "classnames";
 import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 
-import {
-    AbstractPureComponent2,
-    Classes,
-    getRef,
-    IRef,
-    IRefObject,
-    isRefCallback,
-    isRefObject,
-    refHandler,
-} from "../../common";
+import { AbstractPureComponent2, Classes, IRef, IRefCallback, refHandler, setRef } from "../../common";
 import { DISPLAYNAME_PREFIX, IIntentProps, IProps } from "../../common/props";
 
 export interface ITextAreaProps extends IIntentProps, IProps, React.TextareaHTMLAttributes<HTMLTextAreaElement> {
@@ -70,29 +61,25 @@ export class TextArea extends AbstractPureComponent2<ITextAreaProps, ITextAreaSt
     public state: ITextAreaState = {};
 
     // used to measure and set the height of the component on first mount
-    public textareaElement: HTMLTextAreaElement | IRefObject<HTMLTextAreaElement> | null = null;
+    public textareaElement: HTMLTextAreaElement | null = null;
 
-    private handleRef: IRef<HTMLTextAreaElement> = refHandler(this, "textareaElement", this.props.inputRef);
+    private handleRef: IRefCallback<HTMLTextAreaElement> = refHandler(this, "textareaElement", this.props.inputRef);
 
     public componentDidMount() {
         if (this.props.growVertically && this.textareaElement !== null) {
             // HACKHACK: this should probably be done in getSnapshotBeforeUpdate
             /* eslint-disable-next-line react/no-did-mount-set-state */
             this.setState({
-                height: getRef(this.textareaElement)!.scrollHeight,
+                height: this.textareaElement?.scrollHeight,
             });
         }
     }
 
     public componentDidUpdate(prevProps: ITextAreaProps) {
-        const { inputRef } = this.props;
-        if (prevProps.inputRef !== inputRef) {
-            if (isRefObject<HTMLTextAreaElement>(inputRef)) {
-                inputRef.current = (this.textareaElement as IRefObject<HTMLTextAreaElement>).current;
-                this.textareaElement = inputRef;
-            } else if (isRefCallback<HTMLTextAreaElement>(inputRef)) {
-                inputRef(this.textareaElement as HTMLTextAreaElement | null);
-            }
+        if (prevProps.inputRef !== this.props.inputRef) {
+            setRef(prevProps.inputRef, null);
+            this.handleRef = refHandler(this, "textareaElement", this.props.inputRef);
+            setRef(this.props.inputRef, this.textareaElement);
         }
     }
 

packages/core/src/components/popover/popover.tsx

@@ -20,7 +20,7 @@ import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 import { Manager, Popper, PopperChildrenProps, Reference, ReferenceChildrenProps } from "react-popper";
 
-import { AbstractPureComponent2, Classes, IRef, refHandler } from "../../common";
+import { AbstractPureComponent2, Classes, IRef, refHandler, setRef } from "../../common";
 import * as Errors from "../../common/errors";
 import { DISPLAYNAME_PREFIX, HTMLDivProps } from "../../common/props";
 import * as Utils from "../../common/utils";
@@ -221,8 +221,15 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
         this.updateDarkParent();
     }
 
-    public componentDidUpdate(props: IPopoverProps, state: IPopoverState) {
-        super.componentDidUpdate(props, state);
+    public componentDidUpdate(prevProps: IPopoverProps, prevState: IPopoverState) {
+        super.componentDidUpdate(prevProps, prevState);
+
+        if (prevProps.popoverRef !== this.props.popoverRef) {
+            setRef(prevProps.popoverRef, null);
+            this.handlePopoverRef = refHandler(this, "popoverElement", this.props.popoverRef);
+            setRef(this.props.popoverRef, this.popoverElement);
+        }
+
         this.updateDarkParent();
 
         const nextIsOpen = this.getIsOpen(this.props);
@@ -570,7 +577,7 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
     }
 
     private isElementInPopover(element: Element) {
-        return this.popoverElement != null && this.popoverElement.contains(element);
+        return this.popoverElement?.contains(element);
     }
 
     private isHoverInteractionKind() {

packages/core/src/components/tag-input/tagInput.tsx

@@ -18,7 +18,7 @@ import classNames from "classnames";
 import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 
-import { AbstractPureComponent2, Classes, getRef, IRef, IRefObject, Keys, refHandler, Utils } from "../../common";
+import { AbstractPureComponent2, Classes, IRef, Keys, refHandler, setRef, Utils } from "../../common";
 import { DISPLAYNAME_PREFIX, HTMLInputProps, IIntentProps, IProps, MaybeElement } from "../../common/props";
 import { Icon, IconName } from "../icon/icon";
 import { ITagProps, Tag } from "../tag/tag";
@@ -222,7 +222,7 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
         isInputFocused: false,
     };
 
-    public inputElement: HTMLInputElement | IRefObject<HTMLInputElement> | null = null;
+    public inputElement: HTMLInputElement | null = null;
 
     private handleRef: IRef<HTMLInputElement> = refHandler(this, "inputElement", this.props.inputRef);
 
@@ -276,6 +276,14 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
         );
     }
 
+    public componentDidUpdate(prevProps: ITagInputProps) {
+        if (prevProps.inputRef !== this.props.inputRef) {
+            setRef(prevProps.inputRef, null);
+            this.handleRef = refHandler(this, "inputElement", this.props.inputRef);
+            setRef(this.props.inputRef, this.inputElement);
+        }
+    }
+
     private addTags = (value: string, method: TagInputAddMethod = "default") => {
         const { inputValue, onAdd, onChange, values } = this.props;
         const newValues = this.getValues(value);
@@ -347,7 +355,7 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
     }
 
     private handleContainerClick = () => {
-        getRef(this.inputElement)?.focus();
+        this.inputElement?.focus();
     };
 
     private handleContainerBlur = ({ currentTarget }: React.FocusEvent<HTMLDivElement>) => {