[core] fix(Popover): work around react-popper bug, fix React 18 compat

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

@@ -19,7 +19,15 @@ import classNames from "classnames";
 import * as React from "react";
 import { Manager, Modifier, Popper, PopperChildrenProps, Reference, ReferenceChildrenProps } from "react-popper";
 
-import { AbstractPureComponent, Classes, DISPLAYNAME_PREFIX, HTMLDivProps, refHandler, Utils } from "../../common";
+import {
+    AbstractPureComponent,
+    Classes,
+    DISPLAYNAME_PREFIX,
+    HTMLDivProps,
+    mergeRefs,
+    refHandler,
+    Utils,
+} from "../../common";
 import * as Errors from "../../common/errors";
 import { Overlay } from "../overlay/overlay";
 import { ResizeSensor } from "../resize-sensor/resizeSensor";
@@ -174,12 +182,16 @@ export class Popover<
     /** Popover ref handler */
     private popoverRef: React.Ref<HTMLDivElement> = refHandler(this, "popoverElement", this.props.popoverRef);
 
+    /** DOM element that contains the target. */
+    public targetElement: HTMLElement | null = null;
+
     /**
-     * Target DOM element ref.
+     * Target ref handler.
      *
-     * @public for testing
+     * N.B. we use a ref callback instead of an object here due to a `react-popper` bug where `<Reference innerRef>`
+     * does not work correctly in React 18 strict mode. See https://github.com/floating-ui/react-popper/pull/459
      */
-    public targetRef = React.createRef<HTMLElement>();
+    private targetRef: React.Ref<HTMLElement> = el => (this.targetElement = el);
 
     private cancelOpenTimeout?: () => void;
 
@@ -238,7 +250,7 @@ export class Popover<
 
         return (
             <Manager>
-                <Reference innerRef={this.targetRef}>{this.renderTarget}</Reference>
+                <Reference>{this.renderTarget}</Reference>
                 <Popper
                     innerRef={this.popoverRef}
                     placement={placement ?? positionToPlacement(position)}
@@ -314,7 +326,7 @@ export class Popover<
      */
     public reposition = () => this.popperScheduleUpdate?.();
 
-    private renderTarget = ({ ref }: ReferenceChildrenProps) => {
+    private renderTarget = ({ ref: popperChildRef }: ReferenceChildrenProps) => {
         const { children, className, fill, openOnTargetFocus, renderTarget } = this.props;
         const { isOpen } = this.state;
         const isControlled = this.isControlled();
@@ -325,6 +337,10 @@ export class Popover<
             targetTagName = "div";
         }
 
+        // react-popper has a wide type for this ref, but we can narrow it based on the source
+        // see https://github.com/floating-ui/react-popper/blob/beac280d61082852c4efc302be902911ce2d424c/src/Reference.js#L17
+        const ref = mergeRefs(popperChildRef as React.RefCallback<HTMLElement | null>, this.targetRef);
+
         const targetEventHandlers: PopoverHoverTargetHandlers<T> | PopoverClickTargetHandlers<T> =
             isHoverInteractionKind
                 ? {
@@ -407,7 +423,7 @@ export class Popover<
         // N.B. we must attach the ref ('wrapped' with react-popper functionality) to the DOM element here and
         // let ResizeSensor know about it
         return (
-            <ResizeSensor targetRef={this.targetRef} onResize={this.reposition}>
+            <ResizeSensor targetRef={ref} onResize={this.reposition}>
                 {target}
             </ResizeSensor>
         );
@@ -665,14 +681,14 @@ export class Popover<
     };
 
     private handleOverlayClose = (e?: React.SyntheticEvent<HTMLElement>) => {
-        if (this.targetRef.current == null || e === undefined) {
+        if (this.targetElement == null || e === undefined) {
             return;
         }
 
         const event = (e.nativeEvent ?? e) as Event;
         const eventTarget = (event.composed ? event.composedPath()[0] : event.target) as HTMLElement;
         // if click was in target, target event listener will handle things, so don't close
-        if (!Utils.elementIsOrContains(this.targetRef.current, eventTarget) || e.nativeEvent instanceof KeyboardEvent) {
+        if (!Utils.elementIsOrContains(this.targetElement, eventTarget) || e.nativeEvent instanceof KeyboardEvent) {
             this.setOpenState(false, e);
         }
     };
@@ -729,8 +745,7 @@ export class Popover<
 
     private updateDarkParent() {
         if (this.props.usePortal && this.state.isOpen) {
-            const hasDarkParent =
-                this.targetRef.current != null && this.targetRef.current.closest(`.${Classes.DARK}`) != null;
+            const hasDarkParent = this.targetElement != null && this.targetElement.closest(`.${Classes.DARK}`) != null;
             this.setState({ hasDarkParent });
         }
     }

packages/core/src/components/resize-sensor/resizeSensor.tsx

@@ -16,7 +16,7 @@
 
 import * as React from "react";
 
-import { AbstractPureComponent, DISPLAYNAME_PREFIX } from "../../common";
+import { AbstractPureComponent, DISPLAYNAME_PREFIX, mergeRefs } from "../../common";
 
 // backwards-compatible with @blueprintjs/core v4.x
 export type ResizeEntry = ResizeObserverEntry;
@@ -57,7 +57,7 @@ export interface ResizeSensorProps {
      * If you attach a `ref` to the child yourself when rendering it, you must pass the
      * same value here (otherwise, ResizeSensor won't be able to attach its own).
      */
-    targetRef?: React.RefObject<HTMLElement>;
+    targetRef?: React.Ref<HTMLElement>;
 }
 
 /**
@@ -70,7 +70,13 @@ export interface ResizeSensorProps {
 export class ResizeSensor extends AbstractPureComponent<ResizeSensorProps> {
     public static displayName = `${DISPLAYNAME_PREFIX}.ResizeSensor`;
 
-    private targetRef = this.props.targetRef ?? React.createRef<HTMLElement>();
+    private ownTargetRef = React.createRef<HTMLElement>();
+
+    private get targetRef() {
+        return this.props.targetRef === undefined
+            ? this.ownTargetRef
+            : mergeRefs(this.props.targetRef, this.ownTargetRef);
+    }
 
     private prevElement: HTMLElement | undefined = undefined;
 
@@ -79,12 +85,6 @@ export class ResizeSensor extends AbstractPureComponent<ResizeSensorProps> {
 
     public render(): React.ReactNode {
         const onlyChild = React.Children.only(this.props.children);
-
-        // if we're provided a ref to the child already, we don't need to attach one ourselves
-        if (this.props.targetRef !== undefined) {
-            return onlyChild;
-        }
-
         return React.cloneElement(onlyChild, { ref: this.targetRef });
     }
 
@@ -111,27 +111,27 @@ export class ResizeSensor extends AbstractPureComponent<ResizeSensorProps> {
             return;
         }
 
-        if (!(this.targetRef.current instanceof Element)) {
+        if (!(this.ownTargetRef.current instanceof Element)) {
             // stop everything if not defined
             this.observer.disconnect();
             return;
         }
 
-        if (this.targetRef.current === this.prevElement && !force) {
+        if (this.ownTargetRef.current === this.prevElement && !force) {
             // quit if given same element -- nothing to update (unless forced)
             return;
         } else {
             // clear observer list if new element
             this.observer.disconnect();
             // remember element reference for next time
-            this.prevElement = this.targetRef.current;
+            this.prevElement = this.ownTargetRef.current;
         }
 
         // observer callback is invoked immediately when observing new elements
-        this.observer.observe(this.targetRef.current);
+        this.observer.observe(this.ownTargetRef.current);
 
         if (this.props.observeParents) {
-            let parent = this.targetRef.current.parentElement;
+            let parent = this.ownTargetRef.current.parentElement;
             while (parent != null) {
                 this.observer.observe(parent);
                 parent = parent.parentElement;

packages/core/test/popover/popoverTests.tsx

@@ -855,7 +855,7 @@ describe("<Popover>", () => {
 
         const instance = wrapper.instance() as Popover<React.HTMLProps<HTMLButtonElement>>;
         wrapper.popoverElement = instance.popoverElement!;
-        wrapper.targetElement = instance.targetRef.current!;
+        wrapper.targetElement = instance.targetElement!;
         wrapper.assertFindClass = (className: string, expected = true, msg = className) => {
             const actual = wrapper!.findClass(className);
             if (expected) {

packages/core/test/resize-sensor/resizeSensorTests.tsx

@@ -32,11 +32,12 @@ describe("<ResizeSensor>", () => {
         wrapper?.unmount();
         wrapper?.detach();
     });
+
     after(() => testsContainerElement.remove());
 
     it("onResize is called when size changes", async () => {
         const onResize = spy();
-        mountResizeSensor(onResize);
+        mountResizeSensor({ onResize });
         await resize({ width: 200 });
         await resize({ height: 100 });
         await resize({ width: 55 });
@@ -46,7 +47,7 @@ describe("<ResizeSensor>", () => {
 
     it("onResize is NOT called redundantly when size is unchanged", async () => {
         const onResize = spy();
-        mountResizeSensor(onResize);
+        mountResizeSensor({ onResize });
         await resize({ width: 200 });
         await resize({ width: 200 }); // this one should be ignored
         assert.equal(onResize.callCount, 1);
@@ -55,7 +56,7 @@ describe("<ResizeSensor>", () => {
 
     it("onResize is called when element changes", async () => {
         const onResize = spy();
-        mountResizeSensor(onResize);
+        mountResizeSensor({ onResize });
         await resize({ width: 200, id: 1 });
         await resize({ width: 200, id: 2 }); // not ignored bc element recreated
         await resize({ width: 55, id: 3 });
@@ -64,7 +65,7 @@ describe("<ResizeSensor>", () => {
 
     it("onResize can be changed", async () => {
         const onResize1 = spy();
-        mountResizeSensor(onResize1);
+        mountResizeSensor({ onResize: onResize1 });
         await resize({ width: 200, id: 1 });
 
         const onResize2 = spy();
@@ -76,16 +77,29 @@ describe("<ResizeSensor>", () => {
         assert.equal(onResize2.callCount, 2, "second callback should have been called exactly twice");
     });
 
-    function mountResizeSensor(onResize: ResizeSensorProps["onResize"]) {
+    it("still works when user sets their own targetRef", async () => {
+        const onResize = spy();
+        const targetRef = React.createRef<HTMLElement>();
+        const RESIZE_WIDTH = 200;
+        mountResizeSensor({ onResize, targetRef });
+        await resize({ width: RESIZE_WIDTH });
+        assert.equal(onResize.callCount, 1);
+        assertResizeArgs(onResize, [`${RESIZE_WIDTH}x0`]);
+        assert.isNotNull(targetRef.current, "user-provided targetRef should be set");
+        assert.strictEqual(targetRef.current?.clientWidth, RESIZE_WIDTH, "user-provided targetRef.current.clientWidth");
+    });
+
+    function mountResizeSensor(props: Omit<ResizeSensorProps, "children">) {
         return (wrapper = mount<ResizeTesterProps>(
-            <ResizeTester id={0} onResize={onResize} />,
+            <ResizeTester id={0} {...props} />,
             // must be in the DOM for measurement
             { attachTo: testsContainerElement },
         ));
     }
 
     function resize(size: SizeProps) {
         wrapper!.setProps(size);
+        wrapper!.update();
         return new Promise(resolve => setTimeout(resolve, 30));
     }
 

packages/table/src/deprecatedAliases.ts

@@ -32,12 +32,12 @@ export {
     /** @deprecated import JSONFormat instead */
     JSONFormat as JSONFormat2,
     /** @deprecated import JSONFormatProps instead */
-    JSONFormatProps as JSONFormat2Props,
+    type JSONFormatProps as JSONFormat2Props,
 } from "./cell/formats/jsonFormat";
 
 export {
     /** @deprecated import TruncatedFormat instead */
     TruncatedFormat as TruncatedFormat2,
     /** @deprecated import TruncatedFormatProps instead */
-    TruncatedFormatProps as TruncatedFormat2Props,
+    type TruncatedFormatProps as TruncatedFormat2Props,
 } from "./cell/formats/truncatedFormat";