fix:memory leaks on requestAnimationFrame for Category & Formula widgets

Adding a ref hook, so animation can be properly cleaned when unmounting the Category and Formula widgets with cancelAnimationFrame

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Fix hover color in secondary buttons [#65](https://github.com/CartoDB/carto-react-lib/pull/65)
 - Fix widgets loading state when calculating client-side [#75](https://github.com/CartoDB/carto-react-lib/pull/75)
 - Fix min/max aggregated functions [#76](https://github.com/CartoDB/carto-react-lib/pull/76)
+- Fix eventual memory leaks on requestAnimationFrame, on Category and Formula widgets [#77](https://github.com/CartoDB/carto-react-lib/pull/77)
 
 ## 1.0.0-beta12 (2021-01-22)
 

src/ui/utils/animations.js

@@ -0,0 +1,61 @@
+/**
+ * Animate one value from start to end, storing the current request in requestRef hook
+ */
+export function animateValue({ start, end, duration, drawFrame, requestRef }) {
+  if (start === end) return;
+
+  const range = end - start;
+  let current = start;
+  const step = range / ((duration / 1000) * 60);
+  const animate = () => {
+    current += step;
+    drawFrame(Math.floor(current));
+    if ((step > 0 && current < end) || (step < 0 && current > end)) {
+      requestRef.current = requestAnimationFrame(animate);
+    } else {
+      drawFrame(end);
+    }
+  };
+  requestRef.current = requestAnimationFrame(animate);
+}
+
+/**
+ * Animate a series of values from start to end, storing the current request in requestRef hook
+ */
+export function animateValues({ start, end, duration, drawFrame, requestRef }) {
+  const isEqual =
+    start.length === end.length && start.every((val, i) => val.value === end[i].value);
+  if (isEqual) return;
+
+  let currentValues = end.map((elem, i) =>
+    start[i] && start[i].category === elem.category
+      ? { ...elem, value: start[i].value }
+      : elem
+  );
+  let currentFrame = 0;
+
+  const ranges = currentValues.map((elem, i) => end[i].value - elem.value);
+  const noChanges = ranges.every((val) => val === 0);
+  if (noChanges) {
+    drawFrame(end);
+    return;
+  }
+
+  const frames = (duration / 1000) * 60;
+  const steps = ranges.map((val) => val / frames);
+
+  const animate = () => {
+    if (currentFrame < frames) {
+      currentValues = currentValues.map((elem, i) => ({
+        ...elem,
+        value: elem.value + steps[i]
+      }));
+      drawFrame(currentValues);
+      currentFrame++;
+      requestRef.current = requestAnimationFrame(animate);
+    } else {
+      drawFrame(end);
+    }
+  };
+  requestRef.current = requestAnimationFrame(animate);
+}

src/ui/widgets/CategoryWidgetUI.js

@@ -14,6 +14,8 @@ import {
 } from '@material-ui/core';
 import { Alert, AlertTitle, Skeleton } from '@material-ui/lab';
 
+import { animateValues } from '../utils/animations';
+
 const useStyles = makeStyles((theme) => ({
   root: {
     ...theme.typography.body2
@@ -142,6 +144,7 @@ function CategoryWidgetUI(props) {
   const [blockedCategories, setBlockedCategories] = useState([]);
   const [tempBlockedCategories, setTempBlockedCategories] = useState(false);
   const [animValues, setAnimValues] = useState([]);
+  const requestRef = useRef();
   const prevAnimValues = usePrevious(animValues);
   const classes = useStyles();
 
@@ -329,47 +332,17 @@ function CategoryWidgetUI(props) {
   ]);
 
   useEffect(() => {
-    animateValues(prevAnimValues || [], sortedData, 500, (val) => setAnimValues(val));
+    animateValues({
+      start: prevAnimValues || [],
+      end: sortedData,
+      duration: 500,
+      drawFrame: (val) => setAnimValues(val),
+      requestRef
+    });
+
+    return () => cancelAnimationFrame(requestRef.current);
   }, [sortedData]);
 
-  const animateValues = (start, end, duration, drawFrame) => {
-    const isEqual =
-      start.length === end.length && start.every((val, i) => val.value === end[i].value);
-    if (isEqual) return;
-
-    let currentValues = end.map((elem, i) =>
-      start[i] && start[i].category === elem.category
-        ? { ...elem, value: start[i].value }
-        : elem
-    );
-    let currentFrame = 0;
-
-    const ranges = currentValues.map((elem, i) => end[i].value - elem.value);
-    const noChanges = ranges.every((val) => val === 0);
-    if (noChanges) {
-      drawFrame(end);
-      return;
-    }
-
-    const frames = (duration / 1000) * 60;
-    const steps = ranges.map((val) => val / frames);
-
-    const animate = () => {
-      if (currentFrame < frames) {
-        currentValues = currentValues.map((elem, i) => ({
-          ...elem,
-          value: elem.value + steps[i]
-        }));
-        drawFrame(currentValues);
-        currentFrame++;
-        requestAnimationFrame(animate);
-      } else {
-        drawFrame(end);
-      }
-    };
-    requestAnimationFrame(animate);
-  };
-
   // Separated to simplify the widget layout but inside the main component to avoid passing all dependencies
   const CategoryItem = (props) => {
     const { data, onCategoryClick } = props;

src/ui/widgets/FormulaWidgetUI.js

@@ -1,7 +1,7 @@
 import React, { useEffect, useRef, useState } from 'react';
 import PropTypes from 'prop-types';
 import { Box, makeStyles } from '@material-ui/core';
-import animateValue from '../utils/animateValue';
+import { animateValue } from '../utils/animations';
 
 const useStyles = makeStyles((theme) => ({
   root: {
@@ -33,24 +33,38 @@ function FormulaWidgetUI(props) {
   const classes = useStyles();
   const { data, formatter } = props;
   const [value, setValue] = useState('-');
+  const requestRef = useRef();
   const prevValue = usePrevious(value);
 
   useEffect(() => {
     if (typeof data === 'number') {
-      animateValue(prevValue || 0, data, 500, (val) => setValue(val));
+      animateValue({
+        start: prevValue || 0,
+        end: data,
+        duration: 500,
+        drawFrame: (val) => setValue(val),
+        requestRef
+      });
     } else if (
       typeof data === 'object' &&
       data &&
       prevValue &&
       data.value !== null &&
       data.value !== undefined
     ) {
-      animateValue(prevValue.value, data.value, 1000, (val) =>
-        setValue({ value: val, unit: data.prefix })
-      );
+      animateValue({
+        start: prevValue.value,
+        end: data.value,
+        duration: 1000,
+        drawFrame: (val) => setValue({ value: val, unit: data.prefix }),
+        requestRef
+      });
     } else {
       setValue(data);
     }
+
+    return () => cancelAnimationFrame(requestRef.current);
+
     // eslint-disable-next-line
   }, [data, setValue]);