From c4006cef7bf1a70ead1290b09500d1092ed5f0a5 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 21 May 2022 12:50:27 -0700 Subject: [PATCH] perf: Remove markdown serialize from editor render path (#3567) * perf: Remove markdown serialize from editor render path * fix: Simplify heading equality check * perf: Add cache for slugified headings * tsc --- app/components/DocumentMetaWithViews.tsx | 4 +- app/components/DocumentTasks.tsx | 3 +- app/editor/components/EmojiMenu.tsx | 2 +- app/editor/index.tsx | 34 +++----------- app/models/Document.ts | 7 +++ app/scenes/Document/components/Document.tsx | 42 +++++++++++------- .../Document/components/MultiplayerEditor.tsx | 3 -- app/scenes/KeyboardShortcuts.tsx | 2 +- shared/editor/lib/getHeadings.ts | 20 +++++++-- shared/editor/lib/getTasks.ts | 44 +++++++++++++++++++ shared/editor/lib/headingToSlug.ts | 11 ++++- 11 files changed, 115 insertions(+), 57 deletions(-) create mode 100644 shared/editor/lib/getTasks.ts diff --git a/app/components/DocumentMetaWithViews.tsx b/app/components/DocumentMetaWithViews.tsx index c147b06dd..2499e17e0 100644 --- a/app/components/DocumentMetaWithViews.tsx +++ b/app/components/DocumentMetaWithViews.tsx @@ -1,4 +1,4 @@ -import { useObserver } from "mobx-react"; +import { observer, useObserver } from "mobx-react"; import * as React from "react"; import { useTranslation } from "react-i18next"; import { usePopoverState, PopoverDisclosure } from "reakit/Popover"; @@ -83,4 +83,4 @@ const Meta = styled(DocumentMeta)<{ rtl?: boolean }>` } `; -export default DocumentMetaWithViews; +export default observer(DocumentMetaWithViews); diff --git a/app/components/DocumentTasks.tsx b/app/components/DocumentTasks.tsx index 5cc1e234d..d924aa138 100644 --- a/app/components/DocumentTasks.tsx +++ b/app/components/DocumentTasks.tsx @@ -1,3 +1,4 @@ +import { observer } from "mobx-react"; import { DoneIcon } from "outline-icons"; import * as React from "react"; import { useTranslation, TFunction } from "react-i18next"; @@ -60,4 +61,4 @@ const Done = styled(DoneIcon)<{ $animated: boolean }>` transform-origin: center center; `; -export default DocumentTasks; +export default observer(DocumentTasks); diff --git a/app/editor/components/EmojiMenu.tsx b/app/editor/components/EmojiMenu.tsx index 9e14c561e..28e9be245 100644 --- a/app/editor/components/EmojiMenu.tsx +++ b/app/editor/components/EmojiMenu.tsx @@ -21,7 +21,7 @@ const searcher = new FuzzySearch<{ sort: true, }); -class EmojiMenu extends React.Component< +class EmojiMenu extends React.PureComponent< Omit< Props, | "renderMenuItem" diff --git a/app/editor/index.tsx b/app/editor/index.tsx index aa9f23e1f..a17c6d8e1 100644 --- a/app/editor/index.tsx +++ b/app/editor/index.tsx @@ -18,7 +18,8 @@ import * as React from "react"; import { DefaultTheme, ThemeProps } from "styled-components"; import Extension, { CommandFactory } from "@shared/editor/lib/Extension"; import ExtensionManager from "@shared/editor/lib/ExtensionManager"; -import headingToSlug from "@shared/editor/lib/headingToSlug"; +import getHeadings from "@shared/editor/lib/getHeadings"; +import getTasks from "@shared/editor/lib/getTasks"; import { MarkdownSerializer } from "@shared/editor/lib/markdown/serializer"; import Mark from "@shared/editor/marks/Mark"; import Node from "@shared/editor/nodes/Node"; @@ -575,34 +576,11 @@ export class Editor extends React.PureComponent< }; public getHeadings = () => { - const headings: { title: string; level: number; id: string }[] = []; - const previouslySeen = {}; + return getHeadings(this.view.state.doc); + }; - this.view.state.doc.forEach((node) => { - if (node.type.name === "heading") { - // calculate the optimal slug - const slug = headingToSlug(node); - let id = slug; - - // check if we've already used it, and if so how many times? - // Make the new id based on that number ensuring that we have - // unique ID's even when headings are identical - if (previouslySeen[slug] > 0) { - id = headingToSlug(node, previouslySeen[slug]); - } - - // record that we've seen this slug for the next loop - previouslySeen[slug] = - previouslySeen[slug] !== undefined ? previouslySeen[slug] + 1 : 1; - - headings.push({ - title: node.textContent, - level: node.attrs.level, - id, - }); - } - }); - return headings; + public getTasks = () => { + return getTasks(this.view.state.doc); }; public render() { diff --git a/app/models/Document.ts b/app/models/Document.ts index bbcd84121..6b3b9370e 100644 --- a/app/models/Document.ts +++ b/app/models/Document.ts @@ -219,6 +219,13 @@ export default class Document extends ParanoidModel { return floor((this.tasks.completed / this.tasks.total) * 100); } + @action + updateTasks(total: number, completed: number) { + if (total !== this.tasks.total || completed !== this.tasks.completed) { + this.tasks = { total, completed }; + } + } + @action share = async () => { return this.store.rootStore.shares.create({ diff --git a/app/scenes/Document/components/Document.tsx b/app/scenes/Document/components/Document.tsx index d7af65b59..6a9d12653 100644 --- a/app/scenes/Document/components/Document.tsx +++ b/app/scenes/Document/components/Document.tsx @@ -14,6 +14,7 @@ import { } from "react-router"; import styled from "styled-components"; import breakpoint from "styled-components-breakpoint"; +import { Heading } from "@shared/editor/lib/getHeadings"; import getTasks from "@shared/utils/getTasks"; import RootStore from "~/stores/RootStore"; import Document from "~/models/Document"; @@ -29,6 +30,7 @@ import PageTitle from "~/components/PageTitle"; import PlaceholderDocument from "~/components/PlaceholderDocument"; import RegisterKeyDown from "~/components/RegisterKeyDown"; import withStores from "~/components/withStores"; +import type { Editor as TEditor } from "~/editor"; import { NavigationNode } from "~/types"; import { client } from "~/utils/ApiClient"; import { isCustomDomain } from "~/utils/domains"; @@ -73,7 +75,7 @@ type Props = WithTranslation & @observer class DocumentScene extends React.Component { @observable - editor = React.createRef(); + editor = React.createRef(); @observable isUploading = false; @@ -96,6 +98,9 @@ class DocumentScene extends React.Component { @observable title: string = this.props.document.title; + @observable + headings: Heading[] = []; + getEditorText: () => string = () => this.props.document.text; componentDidMount() { @@ -158,7 +163,6 @@ class DocumentScene extends React.Component { return; } - // @ts-expect-error ts-migrate(2339) FIXME: Property 'view' does not exist on type 'unknown'. const { view, parser } = editorRef; view.dispatch( view.state.tr @@ -375,13 +379,24 @@ class DocumentScene extends React.Component { const { document, auth } = this.props; this.getEditorText = getEditorText; - // If the multiplayer editor is enabled then we still want to keep the local - // text value in sync as it is used as a cache. + // Keep headings in sync for table of contents + const headings = this.editor.current?.getHeadings() ?? []; + if ( + headings.map((h) => h.level + h.title).join("") !== + this.headings.map((h) => h.level + h.title).join("") + ) { + this.headings = headings; + } + + // Keep derived task list in sync + const tasks = this.editor.current?.getTasks(); + const total = tasks?.length ?? 0; + const completed = tasks?.filter((t) => t.completed).length ?? 0; + document.updateTasks(total, completed); + + // If the multiplayer editor is enabled we're done here as changes are saved + // through the persistence protocol. The rest of this method is legacy. if (auth.team?.collaborativeEditing) { - action(() => { - document.text = this.getEditorText(); - document.tasks = getTasks(document.text); - })(); return; } @@ -429,12 +444,7 @@ class DocumentScene extends React.Component { const embedsDisabled = (team && team.documentEmbeds === false) || document.embedsDisabled; - const headings = this.editor.current - ? // @ts-expect-error ts-migrate(2571) FIXME: Object is of type 'unknown'. - this.editor.current.getHeadings() - : []; - - const hasHeadings = headings.length > 0; + const hasHeadings = this.headings.length > 0; const showContents = ui.tocVisible && ((readOnly && hasHeadings) || team?.collaborativeEditing); @@ -549,7 +559,7 @@ class DocumentScene extends React.Component { sharedTree={this.props.sharedTree} onSelectTemplate={this.replaceDocument} onSave={this.onSave} - headings={headings} + headings={this.headings} /> { {showContents && ( )} diff --git a/app/scenes/Document/components/MultiplayerEditor.tsx b/app/scenes/Document/components/MultiplayerEditor.tsx index 130f41b6c..38af2864e 100644 --- a/app/scenes/Document/components/MultiplayerEditor.tsx +++ b/app/scenes/Document/components/MultiplayerEditor.tsx @@ -139,9 +139,6 @@ function MultiplayerEditor({ onSynced, ...props }: Props, ref: any) { }); if (debug) { - provider.on("status", (ev: ConnectionStatusEvent) => - Logger.debug("collaboration", "status", ev) - ); provider.on("message", (ev: MessageEvent) => Logger.debug("collaboration", "incoming", { message: ev.message, diff --git a/app/scenes/KeyboardShortcuts.tsx b/app/scenes/KeyboardShortcuts.tsx index 8ffee9364..db9219b3e 100644 --- a/app/scenes/KeyboardShortcuts.tsx +++ b/app/scenes/KeyboardShortcuts.tsx @@ -445,4 +445,4 @@ const Label = styled.dd` color: ${(props) => props.theme.textSecondary}; `; -export default KeyboardShortcuts; +export default React.memo(KeyboardShortcuts); diff --git a/shared/editor/lib/getHeadings.ts b/shared/editor/lib/getHeadings.ts index 96734b714..9d17504f8 100644 --- a/shared/editor/lib/getHeadings.ts +++ b/shared/editor/lib/getHeadings.ts @@ -1,11 +1,23 @@ -import { EditorView } from "prosemirror-view"; +import { Node } from "prosemirror-model"; import headingToSlug from "./headingToSlug"; -export default function getHeadings(view: EditorView) { - const headings: { title: string; level: number; id: string }[] = []; +export type Heading = { + title: string; + level: number; + id: string; +}; + +/** + * Iterates through the document to find all of the headings and their level. + * + * @param doc Prosemirror document node + * @returns Array + */ +export default function getHeadings(doc: Node) { + const headings: Heading[] = []; const previouslySeen = {}; - view.state.doc.forEach((node) => { + doc.forEach((node) => { if (node.type.name === "heading") { // calculate the optimal id const id = headingToSlug(node); diff --git a/shared/editor/lib/getTasks.ts b/shared/editor/lib/getTasks.ts new file mode 100644 index 000000000..b1ad71c0a --- /dev/null +++ b/shared/editor/lib/getTasks.ts @@ -0,0 +1,44 @@ +import { Node } from "prosemirror-model"; + +export type Task = { + text: string; + completed: boolean; +}; + +/** + * Iterates through the document to find all of the tasks and their completion + * state. + * + * @param doc Prosemirror document node + * @returns Array + */ +export default function getTasks(doc: Node): Task[] { + const tasks: Task[] = []; + + doc.descendants((node) => { + if (!node.isBlock) { + return false; + } + + if (node.type.name === "checkbox_list") { + node.content.forEach((listItem) => { + let text = ""; + + listItem.forEach((contentNode) => { + if (contentNode.type.name === "paragraph") { + text += contentNode.textContent; + } + }); + + tasks.push({ + text, + completed: listItem.attrs.checked, + }); + }); + } + + return true; + }); + + return tasks; +} diff --git a/shared/editor/lib/headingToSlug.ts b/shared/editor/lib/headingToSlug.ts index 71fd3b75b..9cfc42fdf 100644 --- a/shared/editor/lib/headingToSlug.ts +++ b/shared/editor/lib/headingToSlug.ts @@ -2,16 +2,25 @@ import { escape } from "lodash"; import { Node } from "prosemirror-model"; import slugify from "slugify"; +const cache = new Map(); + // Slugify, escape, and remove periods from headings so that they are // compatible with both url hashes AND dom ID's (querySelector does not like // ID's that begin with a number or a period, for example). function safeSlugify(text: string) { - return `h-${escape( + if (cache.has(text)) { + return cache.get(text) as string; + } + + const slug = `h-${escape( slugify(text, { remove: /[!"#$%&'\.()*+,\/:;<=>?@\[\]\\^_`{|}~]/g, lower: true, }) )}`; + + cache.set(text, slug); + return slug; } // calculates a unique slug for this heading based on it's text and position