mirror of
https://github.com/ianstormtaylor/slate.git
synced 2025-08-25 08:11:53 +02:00
[editable] Fix Memory Leak when switching between focused editables (#5542)
* [editable] Fix Memory Leak when switching between focused editable containers I've found a memory leak in Slate React where-in when you'd switch between two pages, each with their own editor instance, auto-focusing on these elements causes a memory leak, where there would be a lot of Detached HTML Element's just floating around. At first I thought it a bunch of different bugs with chromium, but noticed that when I force removed the input elements from the dom before the component would unmount, we would still see small leak referencing "latestElement" (while simultaniously, I would still the element & children references in the `IN_FOCUSE` WeakMap) Looking at editable's code, I realized that we're storing state in a weird way, directly mutating it using `useMemo`, and React isn't removing all references (probably because its still stored in the WeakMap. The simple fix for this is what I've commited; using `useLayoutEffect`, we forcably remove the `latestElement` and references to it. * Update editable.tsx * Add changeset --------- Co-authored-by: Dylan Schiemann <dylan@dojotoolkit.org>
This commit is contained in:
5
.changeset/empty-shirts-mate.md
Normal file
5
.changeset/empty-shirts-mate.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
'slate-react': patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix Memory Leak when switching between focused editables
|
@@ -4,6 +4,7 @@ import throttle from 'lodash/throttle'
|
|||||||
import React, {
|
import React, {
|
||||||
useCallback,
|
useCallback,
|
||||||
useEffect,
|
useEffect,
|
||||||
|
useLayoutEffect,
|
||||||
useMemo,
|
useMemo,
|
||||||
useReducer,
|
useReducer,
|
||||||
useRef,
|
useRef,
|
||||||
@@ -176,6 +177,21 @@ export const Editable = (props: EditableProps) => {
|
|||||||
[]
|
[]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
return () => {
|
||||||
|
if (state == null) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
// Avoid leaking DOM nodes when this component is unmounted.
|
||||||
|
if (state.latestElement != null) {
|
||||||
|
state.latestElement.remove()
|
||||||
|
}
|
||||||
|
if (state.latestElement != null) {
|
||||||
|
state.latestElement = null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}, [])
|
||||||
|
|
||||||
// The autoFocus TextareaHTMLAttribute doesn't do anything on a div, so it
|
// The autoFocus TextareaHTMLAttribute doesn't do anything on a div, so it
|
||||||
// needs to be manually focused.
|
// needs to be manually focused.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
Reference in New Issue
Block a user