Skip to content

Commit 9bc3722

Browse files
fix(DynamicPage): fix memory leaks when unmounted during resize (#1838)
fixes #1835
1 parent f75a5fa commit 9bc3722

File tree

3 files changed

+27
-8
lines changed

3 files changed

+27
-8
lines changed

packages/base/src/hooks/useIsRTL.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getRTL } from '@ui5/webcomponents-base/dist/config/RTL';
22
import { useIsomorphicLayoutEffect } from '@ui5/webcomponents-react-base/dist/hooks';
3-
import { RefObject, useState } from 'react';
3+
import { RefObject, useRef, useState } from 'react';
44

55
const GLOBAL_DIR_CSS_VAR = '--_ui5_dir';
66

@@ -32,13 +32,17 @@ const detectRTL = (elementRef: RefObject<HTMLElement>) => {
3232

3333
const useIsRTL = (elementRef: RefObject<HTMLElement>): boolean => {
3434
const [isRTL, setRTL] = useState<boolean>(getRTL()); // use config RTL as best guess
35+
const isMounted = useRef(false);
3536
useIsomorphicLayoutEffect(() => {
37+
isMounted.current = true;
3638
setRTL(detectRTL(elementRef)); // update immediately while rendering
3739
const targets = [document.documentElement, document.body, elementRef.current].filter(Boolean);
3840
const observer = new MutationObserver((mutations) => {
3941
mutations.forEach((mutation) => {
4042
if (mutation.attributeName === 'dir') {
41-
setRTL(detectRTL(elementRef));
43+
if (isMounted.current) {
44+
setRTL(detectRTL(elementRef));
45+
}
4246
}
4347
});
4448
});
@@ -48,9 +52,10 @@ const useIsRTL = (elementRef: RefObject<HTMLElement>): boolean => {
4852
});
4953

5054
return () => {
55+
isMounted.current = false;
5156
observer.disconnect();
5257
};
53-
}, []);
58+
}, [isMounted]);
5459

5560
return isRTL;
5661
};

packages/main/src/components/DynamicPageTitle/index.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import React, {
2222
Ref,
2323
useCallback,
2424
useEffect,
25+
useRef,
2526
useState
2627
} from 'react';
2728
import { createUseStyles } from 'react-jss';
@@ -106,6 +107,14 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
106107
const dynamicPageTitleRef = useConsolidatedRef<HTMLDivElement>(ref);
107108
const [showNavigationInTopArea, setShowNavigationInTopArea] = useState(undefined);
108109
const isRtl = useIsRTL(dynamicPageTitleRef);
110+
const isMounted = useRef(false);
111+
112+
useEffect(() => {
113+
isMounted.current = true;
114+
return () => {
115+
isMounted.current = false;
116+
};
117+
}, [isMounted]);
109118

110119
if (isIE()) {
111120
containerClasses.put(classes.iEClass);
@@ -134,9 +143,9 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
134143
: titleContainer.borderBoxSize;
135144
// Safari doesn't implement `borderBoxSize`
136145
const titleContainerWidth = borderBoxSize?.inlineSize ?? titleContainer.target.getBoundingClientRect().width;
137-
if (titleContainerWidth < 1280 && !showNavigationInTopArea === false) {
146+
if (titleContainerWidth < 1280 && !showNavigationInTopArea === false && isMounted.current) {
138147
setShowNavigationInTopArea(false);
139-
} else if (titleContainerWidth >= 1280 && !showNavigationInTopArea === true) {
148+
} else if (titleContainerWidth >= 1280 && !showNavigationInTopArea === true && isMounted.current) {
140149
setShowNavigationInTopArea(true);
141150
}
142151
}, 300)
@@ -147,7 +156,7 @@ const DynamicPageTitle: FC<DynamicPageTitleProps> = forwardRef((props: InternalP
147156
return () => {
148157
observer.disconnect();
149158
};
150-
}, [dynamicPageTitleRef.current, showNavigationInTopArea]);
159+
}, [dynamicPageTitleRef.current, showNavigationInTopArea, isMounted]);
151160

152161
const paddingLeftRtl = isRtl ? 'paddingRight' : 'paddingLeft';
153162

packages/main/src/internal/useResponsiveContentPadding.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,29 @@ const useStyles = createUseStyles(
1515
export const useResponsiveContentPadding = (element) => {
1616
const [currentRange, setCurrentRange] = useState(getCurrentRange().name);
1717
const resizeTimeout = useRef(null);
18+
const isMounted = useRef(false);
1819
const classes = useStyles();
1920

2021
useEffect(() => {
22+
isMounted.current = true;
2123
const observer = new ResizeObserver(([el]) => {
2224
if (resizeTimeout.current) {
2325
clearTimeout(resizeTimeout.current);
2426
}
2527
resizeTimeout.current = setTimeout(() => {
26-
setCurrentRange(() => getCurrentRange(el.contentRect.width)?.name);
28+
if (isMounted.current) {
29+
setCurrentRange(() => getCurrentRange(el.contentRect.width)?.name);
30+
}
2731
}, 150);
2832
});
2933
if (element) {
3034
observer.observe(element);
3135
}
3236
return () => {
37+
isMounted.current = false;
3338
observer.disconnect();
3439
};
35-
}, [element]);
40+
}, [element, isMounted]);
3641

3742
return classes[currentRange];
3843
};

0 commit comments

Comments
 (0)