Skip to content

Commit

Permalink
fix: eventual memory leaks on requestAnimationFrame
Browse files Browse the repository at this point in the history
Adding a reference, so animation can be properly cleaned when unmounting the Category and Formula widgets (with cancelAnimationFrame)
  • Loading branch information
VictorVelarde committed Feb 1, 2021
1 parent 1b044a7 commit cefa9af
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# CHANGELOG

## Not released

- Remove getUserDatasets method from api [#68](https://github.com/CartoDB/carto-react-lib/pull/68)
- Fix hover color in secondary buttons [#65](https://github.com/CartoDB/carto-react-lib/pull/65)
- 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)

Expand Down
16 changes: 8 additions & 8 deletions src/ui/utils/animations.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Animate one value from start to end
* Animate one value from start to end, storing the current request in requestRef hook
*/
export function animateValue({ start, end, duration, drawFrame }) {
export function animateValue({ start, end, duration, drawFrame, requestRef }) {
if (start === end) return;

const range = end - start;
Expand All @@ -11,18 +11,18 @@ export function animateValue({ start, end, duration, drawFrame }) {
current += step;
drawFrame(Math.floor(current));
if ((step > 0 && current < end) || (step < 0 && current > end)) {
requestAnimationFrame(animate);
requestRef.current = requestAnimationFrame(animate);
} else {
drawFrame(end);
}
};
requestAnimationFrame(animate);
requestRef.current = requestAnimationFrame(animate);
}

/**
* Animate a series of values from start to end
* Animate a series of values from start to end, storing the current request in requestRef hook
*/
export function animateValues({ start, end, duration, drawFrame }) {
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;
Expand Down Expand Up @@ -52,10 +52,10 @@ export function animateValues({ start, end, duration, drawFrame }) {
}));
drawFrame(currentValues);
currentFrame++;
requestAnimationFrame(animate);
requestRef.current = requestAnimationFrame(animate);
} else {
drawFrame(end);
}
};
requestAnimationFrame(animate);
requestRef.current = requestAnimationFrame(animate);
}
6 changes: 5 additions & 1 deletion src/ui/widgets/CategoryWidgetUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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();

Expand Down Expand Up @@ -327,8 +328,11 @@ function CategoryWidgetUI(props) {
start: prevAnimValues || [],
end: sortedData,
duration: 500,
drawFrame: (val) => setAnimValues(val)
drawFrame: (val) => setAnimValues(val),
requestRef
});

return () => cancelAnimationFrame(requestRef.current);
}, [sortedData]);

// Separated to simplify the widget layout but inside the main component to avoid passing all dependencies
Expand Down
10 changes: 8 additions & 2 deletions src/ui/widgets/FormulaWidgetUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function FormulaWidgetUI(props) {
const classes = useStyles();
const { data, formatter } = props;
const [value, setValue] = useState('-');
const requestRef = useRef();
const prevValue = usePrevious(value);

useEffect(() => {
Expand All @@ -41,7 +42,8 @@ function FormulaWidgetUI(props) {
start: prevValue || 0,
end: data,
duration: 500,
drawFrame: (val) => setValue(val)
drawFrame: (val) => setValue(val),
requestRef
});
} else if (
typeof data === 'object' &&
Expand All @@ -54,11 +56,15 @@ function FormulaWidgetUI(props) {
start: prevValue.value,
end: data.value,
duration: 1000,
drawFrame: (val) => setValue({ value: val, unit: data.prefix })
drawFrame: (val) => setValue({ value: val, unit: data.prefix }),
requestRef
});
} else {
setValue(data);
}

return () => cancelAnimationFrame(requestRef.current);

// eslint-disable-next-line
}, [data, setValue]);

Expand Down

0 comments on commit cefa9af

Please sign in to comment.