From 5b8b9458121e56bb99dd6549a2adfd286ce8827a Mon Sep 17 00:00:00 2001 From: Mathew Kleppin Date: Fri, 27 Oct 2023 19:48:27 -0400 Subject: [PATCH 1/3] [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. --- packages/slate-react/src/components/editable.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx index b2cae4d98a..26f43501de 100644 --- a/packages/slate-react/src/components/editable.tsx +++ b/packages/slate-react/src/components/editable.tsx @@ -4,6 +4,7 @@ import throttle from 'lodash/throttle' import React, { useCallback, useEffect, + useLayoutEffect, useMemo, useReducer, 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 // needs to be manually focused. useEffect(() => { From 944365db667e534450573b68ef32f0398be43ad1 Mon Sep 17 00:00:00 2001 From: Mathew Kleppin Date: Fri, 27 Oct 2023 20:09:32 -0400 Subject: [PATCH 2/3] Update editable.tsx --- packages/slate-react/src/components/editable.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/slate-react/src/components/editable.tsx b/packages/slate-react/src/components/editable.tsx index 26f43501de..da574f7117 100644 --- a/packages/slate-react/src/components/editable.tsx +++ b/packages/slate-react/src/components/editable.tsx @@ -184,10 +184,10 @@ export const Editable = (props: EditableProps) => { } // Avoid leaking DOM nodes when this component is unmounted. if (state.latestElement != null) { - state.latestElement.remove(); + state.latestElement.remove() } if (state.latestElement != null) { - state.latestElement = null; + state.latestElement = null } } }, []) From 40aa9d41ac81289569c3f1a283617ec5d51bc444 Mon Sep 17 00:00:00 2001 From: Dylan Schiemann Date: Tue, 31 Oct 2023 09:17:08 -0700 Subject: [PATCH 3/3] Add changeset --- .changeset/empty-shirts-mate.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/empty-shirts-mate.md diff --git a/.changeset/empty-shirts-mate.md b/.changeset/empty-shirts-mate.md new file mode 100644 index 0000000000..e1bb4400d2 --- /dev/null +++ b/.changeset/empty-shirts-mate.md @@ -0,0 +1,5 @@ +--- +'slate-react': patch +--- + +Fix Memory Leak when switching between focused editables