[core] fix(PanelStack2): more stable controlled mode (#4808)

packages/core/src/components/panel-stack2/panel-stack2.md

@@ -103,4 +103,6 @@ const SettingsPanel: React.FC<PanelProps<SettingsPanelInfo>> = props => {
 
 PanelStack2 can be operated as a controlled or uncontrolled component.
 
+If controlled, panels should be added to and removed from the _end_ of the `stack` array.
+
 @interface PanelStack2Props

packages/core/src/components/panel-stack2/panelStack2.tsx

@@ -103,10 +103,6 @@ export const PanelStack2: PanelStack2Component = <T extends Panel<object>>(props
         stackLength.current = stack.length;
     }, [stack]);
 
-    if (stack.length === 0) {
-        return null;
-    }
-
     const handlePanelOpen = React.useCallback(
         (panel: T) => {
             props.onOpen?.(panel);
@@ -130,6 +126,11 @@ export const PanelStack2: PanelStack2Component = <T extends Panel<object>>(props
         [stack, props.onClose],
     );
 
+    // early return, after all hooks are called
+    if (stack.length === 0) {
+        return null;
+    }
+
     const panelsToRender = renderActivePanelOnly ? [stack[0]] : stack;
     const panels = panelsToRender
         .map((panel: T, index: number) => {

packages/core/src/components/panel-stack2/panelView2.tsx

@@ -68,6 +68,24 @@ export const PanelView2: PanelView2Component = <T extends Panel<object>>(props:
             />
         );
 
+    // `props.panel.renderPanel` is simply a function that returns a JSX.Element. It may be an FC which
+    // uses hooks. In order to avoid React errors due to inconsistent hook calls, we must encapsulate
+    // those hooks with their own lifecycle through a very simple wrapper component.
+
+    // N.B. A type cast is required because of error TS2345, where technically `panel.props` could be
+    // instantiated with a type unrelated to our generic constraint `T` here. We know
+    // we're sending the right values here though, and it makes the consumer API for this
+    // component type safe, so it's ok to do this...
+    const PanelWrapper: React.FunctionComponent = React.useMemo(
+        () => () =>
+            props.panel.renderPanel({
+                closePanel: handleClose,
+                openPanel: props.onOpen,
+                ...props.panel.props,
+            } as PanelProps<T>),
+        [props.panel.renderPanel, handleClose, props.onOpen, props.panel.props],
+    );
+
     return (
         <div className={Classes.PANEL_STACK2_VIEW}>
             {props.showHeader && (
@@ -80,17 +98,7 @@ export const PanelView2: PanelView2Component = <T extends Panel<object>>(props:
                     <span />
                 </div>
             )}
-            {/*
-             * Cast is required because of error TS2345, where technically `panel.props` could be
-             * instantiated with a type unrelated to our generic constraint `T` here. We know
-             * we're sending the right values here though, and it makes the consumer API for this
-             * component type safe, so it's ok to do this...
-             */}
-            {props.panel.renderPanel({
-                closePanel: handleClose,
-                openPanel: props.onOpen,
-                ...props.panel.props,
-            } as PanelProps<T>)}
+            <PanelWrapper />
         </div>
     );
 };