From a8db65b5af0af7fa37e5c06448c272ea4965c97e Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Wed, 1 May 2024 13:26:08 +0500 Subject: [PATCH] editor: fix performance issues due to using portals for react node views --- apps/web/src/components/editor/index.tsx | 2 +- apps/web/src/components/editor/tiptap.tsx | 101 ++++++++-------- .../editor-mobile/src/components/editor.tsx | 7 +- .../src/components/readonly-editor.tsx | 13 +-- .../src/extensions/react/react-node-view.tsx | 55 ++------- .../react/react-portal-provider.tsx | 108 +++--------------- .../editor/src/extensions/table/component.tsx | 3 - packages/editor/src/index.ts | 8 +- 8 files changed, 80 insertions(+), 217 deletions(-) diff --git a/apps/web/src/components/editor/index.tsx b/apps/web/src/components/editor/index.tsx index 5e4156d16..98ffd4cfe 100644 --- a/apps/web/src/components/editor/index.tsx +++ b/apps/web/src/components/editor/index.tsx @@ -87,6 +87,7 @@ function saveContent(noteId: string, ignoreEdit: boolean, content: string) { const deferredSave = debounceWithId(saveContent, 100); export default function TabsView() { + const sessions = useEditorStore((store) => store.sessions); const documentPreview = useEditorStore((store) => store.documentPreview); const activeSessionId = useEditorStore((store) => store.activeSessionId); const arePropertiesVisible = useEditorStore( @@ -95,7 +96,6 @@ export default function TabsView() { const isTOCVisible = useEditorStore((store) => store.isTOCVisible); const [dropRef, overlayRef] = useDragOverlay(); - const sessions = useEditorStore.getState().sessions; return ( <> {IS_DESKTOP_APP ? ( diff --git a/apps/web/src/components/editor/tiptap.tsx b/apps/web/src/components/editor/tiptap.tsx index 14fd063ab..d883c49c9 100644 --- a/apps/web/src/components/editor/tiptap.tsx +++ b/apps/web/src/components/editor/tiptap.tsx @@ -24,7 +24,6 @@ import "@notesnook/editor/styles/fonts.css"; import { Toolbar, useTiptap, - PortalProvider, Editor, AttachmentType, usePermissionHandler, @@ -41,7 +40,6 @@ import { import { Box, Flex } from "@theme-ui/components"; import { PropsWithChildren, - useCallback, useEffect, useLayoutEffect, useMemo, @@ -61,7 +59,6 @@ import { writeToClipboard } from "../../utils/clipboard"; import { useEditorStore } from "../../stores/editor-store"; import { parseInternalLink } from "@notesnook/core"; import Skeleton from "react-loading-skeleton"; -import { showToast } from "../../utils/toast"; export type OnChangeHandler = ( content: () => string, @@ -402,58 +399,56 @@ function TiptapWrapper( }, [theme]); return ( - - + { + props.onLoad?.(editor); + containerRef.current + ?.querySelector(".editor-loading-container") + ?.remove(); }} - > - { - props.onLoad?.(editor); - containerRef.current - ?.querySelector(".editor-loading-container") - ?.remove(); - }} - editorContainer={() => { - if (editorContainerRef.current) return editorContainerRef.current; - const editorContainer = document.createElement("div"); - editorContainer.classList.add("selectable"); - editorContainer.style.flex = "1"; - editorContainer.style.cursor = "text"; - editorContainer.style.color = - theme.scopes.editor?.primary?.paragraph || - theme.scopes.base.primary.paragraph; - editorContainer.style.fontSize = `${editorConfig.fontSize}px`; - editorContainer.style.fontFamily = - getFontById(editorConfig.fontFamily)?.font || "sans-serif"; - editorContainerRef.current = editorContainer; - return editorContainer; - }} - fontFamily={editorConfig.fontFamily} - fontSize={editorConfig.fontSize} + editorContainer={() => { + if (editorContainerRef.current) return editorContainerRef.current; + const editorContainer = document.createElement("div"); + editorContainer.classList.add("selectable"); + editorContainer.style.flex = "1"; + editorContainer.style.cursor = "text"; + editorContainer.style.color = + theme.scopes.editor?.primary?.paragraph || + theme.scopes.base.primary.paragraph; + editorContainer.style.fontSize = `${editorConfig.fontSize}px`; + editorContainer.style.fontFamily = + getFontById(editorConfig.fontFamily)?.font || "sans-serif"; + editorContainerRef.current = editorContainer; + return editorContainer; + }} + fontFamily={editorConfig.fontFamily} + fontSize={editorConfig.fontSize} + /> + {props.children} + + - {props.children} - - - - - - + + + ); } export default TiptapWrapper; diff --git a/packages/editor-mobile/src/components/editor.tsx b/packages/editor-mobile/src/components/editor.tsx index 8c40627de..de837d9c6 100644 --- a/packages/editor-mobile/src/components/editor.tsx +++ b/packages/editor-mobile/src/components/editor.tsx @@ -21,7 +21,6 @@ import { Editor, getFontById, getTableOfContents, - PortalProvider, TiptapOptions, Toolbar, usePermissionHandler, @@ -801,11 +800,7 @@ const TiptapProvider = (): JSX.Element => { } }, [settings.fontSize, settings.fontFamily]); - return ( - - - - ); + return ; }; export default TiptapProvider; diff --git a/packages/editor-mobile/src/components/readonly-editor.tsx b/packages/editor-mobile/src/components/readonly-editor.tsx index 49f97501f..b1573c8ae 100644 --- a/packages/editor-mobile/src/components/readonly-editor.tsx +++ b/packages/editor-mobile/src/components/readonly-editor.tsx @@ -16,12 +16,7 @@ GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see . */ -import { - PortalProvider, - TiptapOptions, - getFontById, - useTiptap -} from "@notesnook/editor"; +import { TiptapOptions, getFontById, useTiptap } from "@notesnook/editor"; import { useThemeColors } from "@notesnook/theme"; import { useCallback, @@ -71,11 +66,7 @@ export const ReadonlyEditorProvider = (): JSX.Element => { } }, [settings.fontSize, settings.fontFamily]); - return ( - - - - ); + return ; }; const Tiptap = ({ diff --git a/packages/editor/src/extensions/react/react-node-view.tsx b/packages/editor/src/extensions/react/react-node-view.tsx index 2aead5c8c..ecb70ca35 100644 --- a/packages/editor/src/extensions/react/react-node-view.tsx +++ b/packages/editor/src/extensions/react/react-node-view.tsx @@ -41,7 +41,7 @@ declare module "prosemirror-view" { slice: Slice ): { dom: HTMLElement; text: string }; } - +const portalProviderAPI = new PortalProviderAPI(); export class ReactNodeView

implements NodeView { private domRef!: HTMLElement; private contentDOMWrapper?: Node; @@ -53,30 +53,10 @@ export class ReactNodeView

implements NodeView { pos = -1; posEnd: number | undefined; - // in order to cleanly unmount a React Portal, we have to preserve the - // dom structure for the full node. However, Prosemirror/browser can, - // sometimes, detach the node before React is able to call "unmount". - // Unmounting a React child whose DOM counterpart is already removed - // results in a DOMException. - // To fix that, we observe and store all the detached subnodes and later add - // them back when we are ready to destroy our node view once & for all. - // This wouldn't be necessary if React allowed for a way to "sync" the DOM - // changes to its Virtual DOM. - detached: Set = new Set(); - detachObserver = new MutationObserver((mutations) => { - const filtered = mutations.filter( - (m) => m.target === this.domRef && m.removedNodes.length > 0 - ); - for (const mutation of filtered) { - for (const node of mutation.removedNodes) this.detached.add(node); - } - }); - constructor( node: PMNode, protected readonly editor: Editor, protected readonly getPos: GetPosNode, - protected readonly portalProviderAPI: PortalProviderAPI, protected readonly options: ReactNodeViewOptions

) { this.node = node; @@ -105,11 +85,6 @@ export class ReactNodeView

implements NodeView { init() { this.domRef = this.createDomRef(); this.domRef.ondragstart = (ev) => this.onDragStart(ev); - this.detachObserver.observe(this.domRef, { - childList: true, - subtree: true - }); - // this.setDomAttrs(this.node, this.domRef); const { dom: contentDOMWrapper, contentDOM } = this.getContentDOM() ?? {}; @@ -131,12 +106,12 @@ export class ReactNodeView

implements NodeView { private render() { if (process.env.NODE_ENV === "test") return; - if (!this.domRef || !this.portalProviderAPI) { + if (!this.domRef) { console.warn("Cannot render node view"); return; } - this.portalProviderAPI.render(this.Component, this.domRef); + portalProviderAPI.render(this.Component, this.domRef); } createDomRef(): HTMLElement { @@ -489,14 +464,7 @@ export class ReactNodeView

implements NodeView { } destroy() { - this.detachObserver.disconnect(); - // add back the detached nodes because React expects an untouched - // DOM representation (and there's no way to reconcile the DOM later). - for (const node of this.detached) { - this.domRef.appendChild(node); - } - this.portalProviderAPI.remove(this.domRef); - this.detached.clear(); + portalProviderAPI.remove(this.domRef); this.domRef.remove(); } } @@ -507,18 +475,11 @@ export function createNodeView( ) { return ({ node, getPos, editor }: NodeViewRendererProps) => { const _getPos = () => (typeof getPos === "boolean" ? -1 : getPos()); - if (!editor.storage.portalProviderAPI) return {}; - return new ReactNodeView( - node, - editor as Editor, - _getPos, - editor.storage.portalProviderAPI, - { - ...options, - component - } - ).init(); + return new ReactNodeView(node, editor as Editor, _getPos, { + ...options, + component + }).init(); }; } diff --git a/packages/editor/src/extensions/react/react-portal-provider.tsx b/packages/editor/src/extensions/react/react-portal-provider.tsx index 849b28e72..027cbc843 100644 --- a/packages/editor/src/extensions/react/react-portal-provider.tsx +++ b/packages/editor/src/extensions/react/react-portal-provider.tsx @@ -17,18 +17,10 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ -import React, { - FunctionComponent, - PropsWithChildren, - useContext, - useEffect, - useMemo, - useRef, - useState -} from "react"; -import { createPortal, flushSync } from "react-dom"; +import { FunctionComponent, PropsWithChildren } from "react"; +import { flushSync } from "react-dom"; import { EventDispatcher } from "./event-dispatcher"; -import { nanoid } from "nanoid"; +import { Root, createRoot } from "react-dom/client"; export type BasePortalProviderProps = PropsWithChildren; @@ -45,93 +37,31 @@ export type PortalRendererState = { export class PortalProviderAPI extends EventDispatcher { portals: Map = new Map(); + roots: Map = new Map(); constructor() { super(); } - /** - * Trigger an update in all subscribers. - */ - private update() { - this.emit("update", this.portals); - } - render(Component: FunctionComponent, container: HTMLElement) { - const portal = this.portals.get(container); - this.portals.set(container, { Component, key: portal?.key ?? nanoid() }); - this.update(); - } - - /** - * Force an update in all the portals by setting new keys for every portal. - * - * Delete all orphaned containers (deleted from the DOM). This is useful for - * Decoration where there is no destroy method. - */ - forceUpdate(): void { - for (const [container, { Component }] of this.portals) { - this.portals.set(container, { Component, key: nanoid() }); - } + const root = this.roots.get(container) || createRoot(container); + flushSync(() => root.render()); + this.roots.set(container, root); } remove(container: HTMLElement) { - // Remove the portal which was being wrapped in the provided container. - this.portals.delete(container); + // if container is already unmounted (maybe by prosemirror), + // no need to proceed + if (!container.parentNode) return; - // Trigger an update - this.update(); + const root = this.roots.get(container); + if (!root) return; + this.roots.delete(container); + + try { + root.unmount(); + } catch { + // ignore + } } } - -const PortalProviderContext = React.createContext< - PortalProviderAPI | undefined ->(undefined); -export function usePortalProvider() { - return useContext(PortalProviderContext); -} - -export function PortalProvider(props: PropsWithChildren) { - const portalProviderAPI = useMemo(() => new PortalProviderAPI(), []); - - return ( - - {props.children} - - - ); -} - -function PortalRenderer(props: { portalProviderAPI: PortalProviderAPI }) { - const { portalProviderAPI } = props; - const mounted = useRef(true); - const [portals, setPortals] = useState(() => - Array.from(portalProviderAPI.portals.entries()) - ); - - useEffect(() => { - mounted.current = true; - function onUpdate(portalMap: Portals) { - // we have to make sure the component is mounted otherwise React - // throws an error. - if (!mounted.current) return; - - // flushSync is necessary here, otherwise we get into a loop where - // ProseMirror destroys and recreates the node view over and over again. - flushSync(() => setPortals(Array.from(portalMap.entries()))); - } - portalProviderAPI.on("update", onUpdate); - return () => { - mounted.current = false; - portalProviderAPI.off("update", onUpdate); - }; - }, [portalProviderAPI]); - - return ( - <> - {portals.map(([container, { Component, key }]) => - createPortal(, container, key) - )} - - ); -} diff --git a/packages/editor/src/extensions/table/component.tsx b/packages/editor/src/extensions/table/component.tsx index 11f9492df..4c2084f16 100644 --- a/packages/editor/src/extensions/table/component.tsx +++ b/packages/editor/src/extensions/table/component.tsx @@ -82,8 +82,6 @@ export function TableComponent(props: ReactNodeViewProps) { } export function TableNodeView(editor: TiptapEditor) { - if (!editor.storage.portalProviderAPI) return; - const api = editor.storage.portalProviderAPI; class TableNode extends ReactNodeView> implements NodeView @@ -93,7 +91,6 @@ export function TableNodeView(editor: TiptapEditor) { node, editor, () => 0, // todo - api, { component: TableComponent, shouldUpdate: (prev, next) => { diff --git a/packages/editor/src/index.ts b/packages/editor/src/index.ts index 7b463d5a2..65b083cd5 100644 --- a/packages/editor/src/index.ts +++ b/packages/editor/src/index.ts @@ -61,7 +61,6 @@ import OrderedList from "./extensions/ordered-list"; import { OutlineList } from "./extensions/outline-list"; import { OutlineListItem } from "./extensions/outline-list-item"; import { Paragraph } from "./extensions/paragraph"; -import { PortalProviderAPI, usePortalProvider } from "./extensions/react"; import { SearchReplace } from "./extensions/search-replace"; import { Table } from "./extensions/table"; import TableCell from "./extensions/table-cell"; @@ -87,7 +86,6 @@ import { useEditorSearchStore } from "./toolbar/stores/search-store"; import { DiffHighlighter } from "./extensions/diff-highlighter"; interface TiptapStorage { - portalProviderAPI?: PortalProviderAPI; dateFormat?: DateTimeOptions["dateFormat"]; timeFormat?: DateTimeOptions["timeFormat"]; openLink?: (url: string) => void; @@ -124,7 +122,7 @@ export type TiptapOptions = EditorOptions & Omit & Omit & DateTimeOptions & - Omit & { + TiptapStorage & { downloadOptions?: DownloadOptions; isMobile?: boolean; doubleSpacedLines?: boolean; @@ -153,7 +151,6 @@ const useTiptap = ( ...restOptions } = options; - const PortalProviderAPI = usePortalProvider(); const setIsMobile = useToolbarStore((store) => store.setIsMobile); const closeAllPopups = useToolbarStore((store) => store.closeAllPopups); const setDownloadOptions = useToolbarStore( @@ -356,7 +353,6 @@ const useTiptap = ( }) ], onBeforeCreate: ({ editor }) => { - editor.storage.portalProviderAPI = PortalProviderAPI; editor.storage.dateFormat = dateFormat; editor.storage.timeFormat = timeFormat; @@ -378,7 +374,6 @@ const useTiptap = ( downloadAttachment, openAttachmentPicker, getAttachmentData, - PortalProviderAPI, onBeforeCreate, openLink, dateFormat, @@ -403,7 +398,6 @@ const useTiptap = ( export { type Fragment } from "prosemirror-model"; export { type Attachment, type AttachmentType } from "./extensions/attachment"; export { type ImageAttributes } from "./extensions/image"; -export * from "./extensions/react"; export * from "./toolbar"; export * from "./types"; export * from "./utils/word-counter";