fix(Popper): prevent race conditions by properly cleaning up animation frames

Add cleanup for requestAnimationFrame to prevent race conditions when popper
visibility changes rapidly or component unmounts during RAF execution.

Signed-off-by: Mohamed Fall <ps.hackmaster@gmail.com>

Author: fallmo

packages/react-core/src/helpers/Popper/Popper.tsx

@@ -3,7 +3,7 @@ import * as ReactDOM from 'react-dom';
 import { usePopper } from './thirdparty/react-popper/usePopper';
 import { Options as OffsetOptions } from './thirdparty/popper-core/modifiers/offset';
 import { Placement, Modifier } from './thirdparty/popper-core';
-import { clearTimeouts } from '../util';
+import { clearAnimationFrames, clearTimeouts } from '../util';
 import { css } from '@patternfly/react-styles';
 import '@patternfly/react-styles/css/components/Popper/Popper.css';
 import { getLanguageDirection } from '../util';
@@ -234,6 +234,7 @@ export const Popper: React.FunctionComponent<PopperProps> = ({
   const transitionTimerRef = useRef(null);
   const showTimerRef = useRef(null);
   const hideTimerRef = useRef(null);
+  const rafRef = useRef<number>(null);
   const prevExitDelayRef = useRef<number>(undefined);
 
   const refOrTrigger = refElement || triggerElement;
@@ -275,6 +276,7 @@ export const Popper: React.FunctionComponent<PopperProps> = ({
   useEffect(
     () => () => {
       clearTimeouts([transitionTimerRef, hideTimerRef, showTimerRef]);
+      clearAnimationFrames([rafRef]);
     },
     []
   );
@@ -469,6 +471,7 @@ export const Popper: React.FunctionComponent<PopperProps> = ({
   useEffect(() => {
     if (prevExitDelayRef.current < exitDelay) {
       clearTimeouts([transitionTimerRef, hideTimerRef]);
+      clearAnimationFrames([rafRef]);
       hideTimerRef.current = setTimeout(() => {
         transitionTimerRef.current = setTimeout(() => {
           setInternalIsVisible(false);
@@ -481,14 +484,16 @@ export const Popper: React.FunctionComponent<PopperProps> = ({
   const show = () => {
     onShow();
     clearTimeouts([transitionTimerRef, hideTimerRef]);
+    clearAnimationFrames([rafRef]);
     showTimerRef.current = setTimeout(() => {
       setInternalIsVisible(true);
       // Ensures React has committed the DOM changes and the popper element is rendered
-      requestAnimationFrame(() => {
+      rafRef.current = requestAnimationFrame(() => {
         // Ensures Popper.js has calculated and applied the position transform before making element visible
-        requestAnimationFrame(() => {
+        rafRef.current = requestAnimationFrame(() => {
           setOpacity(1);
           onShown();
+          rafRef.current = null;
         });
       });
     }, entryDelay);
@@ -497,6 +502,7 @@ export const Popper: React.FunctionComponent<PopperProps> = ({
   const hide = () => {
     onHide();
     clearTimeouts([showTimerRef]);
+    clearAnimationFrames([rafRef]);
     hideTimerRef.current = setTimeout(() => {
       setOpacity(0);
       transitionTimerRef.current = setTimeout(() => {

packages/react-core/src/helpers/__mocks__/util.ts

@@ -2,4 +2,6 @@ export const getUniqueId = () => 'unique_id_mock';
 
 export const clearTimeouts = () => {};
 
+export const clearAnimationFrames = () => {};
+
 export const getLanguageDirection = () => 'ltr';

packages/react-core/src/helpers/util.ts

@@ -524,6 +524,17 @@ export const clearTimeouts = (timeoutRefs: React.RefObject<any>[]) => {
   });
 };
 
+/**
+ * @param {React.RefObject<any>[]} animationFrameRefs - Animation frame refs to clear
+ */
+export const clearAnimationFrames = (animationFrameRefs: React.RefObject<any>[]) => {
+  animationFrameRefs.forEach((ref) => {
+    if (ref.current) {
+      cancelAnimationFrame(ref.current);
+    }
+  });
+};
+
 /**
  * Helper function to get the language direction of a given element, useful for figuring out if left-to-right
  * or right-to-left specific logic should be applied.