[Page Actions] Fix render issues

Author: kyledurand

.changeset/twelve-doors-complain.md

@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+Fixed a re-render bug with Page Actions

polaris-react/playground/DetailsPage.tsx

@@ -463,10 +463,10 @@ export function DetailsPage() {
     </VisuallyHidden>
   );
 
-  // ---- Description ----
+  const [title, setTitle] = useState(
+    "The North Face Ventrix Active Trail Hybrid Hoodie - Men's",
+  );
   const [descriptionValue, setDescriptionValue] = useState(initialDescription);
-
-  // ---- Select ----
   const [selected, setSelected] = useState('today');
 
   const options = [
@@ -546,7 +546,7 @@ export function DetailsPage() {
     <Page
       fullWidth
       breadcrumbs={[{content: 'Products', url: '/products/31'}]}
-      title="The North Face Ventrix Active Trail Hybrid Hoodie - Men's"
+      title={title}
       titleMetadata={<Badge status="success">Success badge</Badge>}
       primaryAction={{
         content: 'Save this page',
@@ -597,8 +597,11 @@ export function DetailsPage() {
             <FormLayout>
               <TextField
                 label="Title"
-                value="M60-A"
-                onChange={() => setIsDirty(true)}
+                value={title}
+                onChange={(title) => {
+                  setTitle(title);
+                  setIsDirty(true);
+                }}
                 autoComplete="off"
               />
               <TextField

polaris-react/src/components/ActionMenu/components/Actions/Actions.tsx

@@ -1,4 +1,12 @@
-import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
+/* eslint-disable no-console */
+import React, {
+  useCallback,
+  useEffect,
+  useLayoutEffect,
+  useMemo,
+  useRef,
+  useState,
+} from 'react';
 
 import {debounce} from '../../../../utilities/debounce';
 import {useI18n} from '../../../../utilities/i18n';
@@ -9,10 +17,10 @@ import type {
   MenuGroupDescriptor,
 } from '../../../../types';
 import {ButtonGroup} from '../../../ButtonGroup';
-// eslint-disable-next-line import/no-deprecated
-import {EventListener} from '../../../EventListener';
 import {MenuGroup} from '../MenuGroup';
 import {SecondaryAction} from '../SecondaryAction';
+import {useEventListener} from '../../../../utilities/use-event-listener';
+import {useIsAfterInitialMount} from '../../../../utilities/use-is-after-initial-mount';
 
 import styles from './Actions.scss';
 
@@ -33,6 +41,12 @@ interface MeasuredActions {
 const ACTION_SPACING = 8;
 
 export function Actions({actions = [], groups = [], onActionRollup}: Props) {
+  const isMounted = useIsAfterInitialMount();
+  useEffect(() => {
+    isMounted
+      ? console.log('update, measured: ', timesMeasured.current)
+      : console.log('mount, measured: ', timesMeasured.current);
+  });
   const i18n = useI18n();
   const actionsLayoutRef = useRef<HTMLDivElement>(null);
   const menuGroupWidthRef = useRef<number>(0);
@@ -81,6 +95,10 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
       );
     }
 
+    console.log(
+      'updateActions => setMeasuredActions, measured: ',
+      timesMeasured.current,
+    );
     setMeasuredActions((currentMeasuredActions) => {
       const showable = actionsAndGroups.slice(
         0,
@@ -90,6 +108,16 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
         currentMeasuredActions.showable.length,
         actionsAndGroups.length,
       );
+
+      if (
+        showable === currentMeasuredActions.showable &&
+        rolledUp === currentMeasuredActions.rolledUp
+      ) {
+        // Bail out of setState if the values are the same
+        console.log('updateActions => bail out of setState');
+        return currentMeasuredActions;
+      }
+
       return {showable, rolledUp};
     });
   }, [actions, groups]);
@@ -105,6 +133,10 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
     const actionsAndGroups = [...actions, ...groups];
 
     if (actionsAndGroups.length === 1) {
+      console.log(
+        'measureActions => setMeasuredActions, measured: ',
+        timesMeasured.current,
+      );
       setMeasuredActions({showable: actionsAndGroups, rolledUp: []});
       return;
     }
@@ -143,10 +175,23 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
         rollupActiveRef.current = isRollupActive;
       }
     }
+    console.log(
+      'measureActions => setMeasuredActions, measured: ',
+      timesMeasured.current,
+    );
+    setMeasuredActions((currentMeasuredActions) => {
+      if (
+        currentMeasuredActions.showable === newShowableActions &&
+        currentMeasuredActions.rolledUp === newRolledUpActions
+      ) {
+        console.log('measureActions bail out of setState');
+        return currentMeasuredActions;
+      }
 
-    setMeasuredActions({
-      showable: newShowableActions,
-      rolledUp: newRolledUpActions,
+      return {
+        showable: newShowableActions,
+        rolledUp: newRolledUpActions,
+      };
     });
 
     timesMeasured.current += 1;
@@ -161,6 +206,10 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
           availableWidthRef.current = actionsLayoutRef.current.offsetWidth;
           // Set timesMeasured to 0 to allow re-measuring
           timesMeasured.current = 0;
+          console.log(
+            'handleResize => measureActions, measured: ',
+            timesMeasured.current,
+          );
           measureActions();
         },
         50,
@@ -169,7 +218,9 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
     [measureActions],
   );
 
-  useEffect(() => {
+  useEventListener('resize', handleResize);
+
+  useLayoutEffect(() => {
     if (!actionsLayoutRef.current) {
       return;
     }
@@ -178,12 +229,17 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
     if (
       // Allow measuring twice
       // This accounts for the initial paint and re-flow
-      timesMeasured.current >= 2 &&
+      timesMeasured.current >= 3 &&
       [...actions, ...groups].length === actionsAndGroupsLengthRef.current
     ) {
+      console.log(
+        'useEffect, updateActions, measured: ',
+        timesMeasured.current,
+      );
       updateActions();
       return;
     }
+    console.log('useEffect, measureActions, measured: ', timesMeasured.current);
     measureActions();
   }, [actions, groups, measureActions, updateActions]);
 
@@ -319,7 +375,6 @@ export function Actions({actions = [], groups = [], onActionRollup}: Props) {
   return (
     <div className={styles.ActionsLayout} ref={actionsLayoutRef}>
       {groupedActionsMarkup}
-      <EventListener event="resize" handler={handleResize} />
     </div>
   );
 }