diff --git a/packages/editor/src/extensions/virtualization/__tests__/first-view.test.ts b/packages/editor/src/extensions/virtualization/__tests__/first-view.test.ts new file mode 100644 index 000000000..47afecc80 --- /dev/null +++ b/packages/editor/src/extensions/virtualization/__tests__/first-view.test.ts @@ -0,0 +1,112 @@ +/* +This file is part of the Notesnook project (https://notesnook.com/) + +Copyright (C) 2023 Streetwriters (Private) Limited + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +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 { describe, expect, test } from "vitest"; +import { Editor } from "@tiptap/core"; +import StarterKit from "@tiptap/starter-kit"; +import { Virtualization } from "../index.js"; +import { BlockId } from "../../block-id/block-id.js"; + +const BLOCKS = 400; + +/** + * Mirrors a saved Notesnook note: BlockId renders `data-block-id` into the + * stored HTML, and paging keys off it. + */ +function savedNoteHTML(n: number) { + let html = ""; + for (let i = 0; i < n; i++) + html += `

Paragraph number ${i} with filler.

`; + return html; +} + +/** A legacy/imported/pasted note, whose HTML has no block ids yet. */ +function unidentifiedHTML(n: number) { + let html = ""; + for (let i = 0; i < n; i++) + html += `

Paragraph number ${i} with filler.

`; + return html; +} + +function makeEditor(enabled: boolean, content = savedNoteHTML(BLOCKS)) { + return new Editor({ + element: document.createElement("div"), + content, + extensions: [ + StarterKit, + // Paging keys off blockId: a block without one always materializes + // (see viewport-plugin), so this is required, not incidental. + BlockId, + Virtualization.configure({ enabled, thresholdBlocks: 50 }) + ] + }); +} + +function placeholderCount(editor: Editor) { + return editor.view.dom.querySelectorAll("[data-virtual-placeholder]").length; +} + +describe("virtualization: the first view", () => { + test("the view built by the Editor constructor is already virtualized", () => { + const editor = makeEditor(true); + + // Nothing has run except the constructor - no React effect, no manual + // createView(). If this passes, the document was never mounted in full. + expect(editor.state.doc.childCount).toBe(BLOCKS); + expect(placeholderCount(editor)).toBeGreaterThan(0); + + editor.destroy(); + }); + + test("recreating the view keeps virtualization installed", () => { + const editor = makeEditor(true); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - mirrors what useEditor does on every deps change + editor.createView(); + + expect(editor.state.doc.childCount).toBe(BLOCKS); + expect(placeholderCount(editor)).toBeGreaterThan(0); + + editor.destroy(); + }); + + test("nothing is virtualized when the extension is disabled", () => { + const editor = makeEditor(false); + + expect(editor.state.doc.childCount).toBe(BLOCKS); + expect(placeholderCount(editor)).toBe(0); + + editor.destroy(); + }); + + // KNOWN GAP, not desired behaviour. BlockId assigns ids from an + // appendTransaction, which does not run during construction, so a note whose + // stored HTML has no data-block-id renders in full on its first view. The fix + // is to assign block ids at parse/load time - see + // docs/editor-performance/05-per-transaction-work.md section 4.1. When that + // lands, this expectation should flip to toBeGreaterThan(0). + test("a note with no block ids is not yet paged on its first view", () => { + const editor = makeEditor(true, unidentifiedHTML(BLOCKS)); + + expect(editor.state.doc.childCount).toBe(BLOCKS); + expect(placeholderCount(editor)).toBe(0); + + editor.destroy(); + }); +}); diff --git a/packages/editor/src/extensions/virtualization/index.ts b/packages/editor/src/extensions/virtualization/index.ts index 0468fc1ee..b353f7641 100644 --- a/packages/editor/src/extensions/virtualization/index.ts +++ b/packages/editor/src/extensions/virtualization/index.ts @@ -60,6 +60,14 @@ export const Virtualization = Extension.create({ }; }, + // Runs immediately before the Editor constructor creates its first view, so + // that view is already virtualized. Installing any later (e.g. from + // useEditor's effect) means the whole document gets mounted unvirtualized + // once, which is the exact cost virtualization exists to avoid. + onBeforeCreate() { + installVirtualization(this.editor); + }, + addProseMirrorPlugins() { if (!this.options.enabled) return []; return [virtualizationPlugin()]; @@ -67,10 +75,18 @@ export const Virtualization = Extension.create({ }); /** - * Wraps the editor's node views with the virtualization layer. Must run before - * the view is (re)created. A ProseMirror plugin cannot do this — prosemirror-view - * consults the view's own `nodeViews` prop before any plugin (buildNodeViews is - * first-wins) — so we decorate `extensionManager.nodeViews` at its source. + * Wraps the editor's node views with the virtualization layer. + * + * Called from the extension's own `onBeforeCreate`, which fires before the + * Editor constructor's `createView()` — so the very first view is virtualized. + * + * A ProseMirror plugin cannot do this: prosemirror-view consults the view's own + * `nodeViews` prop before any plugin (buildNodeViews is first-wins), and Tiptap + * overwrites `editorProps.nodeViews` with `extensionManager.nodeViews` via + * setProps right after construction. So we decorate the getter at its source. + * + * The patch lives on the extensionManager instance, which outlives individual + * views, so later `createView()` calls pick it up without reinstalling. */ export function installVirtualization(editor: Editor): void { const storage = editor.storage.virtualization as diff --git a/packages/editor/src/hooks/use-editor.ts b/packages/editor/src/hooks/use-editor.ts index e8f63d30f..e8d6b83b7 100644 --- a/packages/editor/src/hooks/use-editor.ts +++ b/packages/editor/src/hooks/use-editor.ts @@ -23,7 +23,6 @@ import { Editor } from "../types.js"; import { useToolbarStore } from "../toolbar/stores/toolbar-store.js"; import { EditorView } from "@tiptap/pm/view"; import { useEditorSearchStore } from "../toolbar/stores/search-store.js"; -import { installVirtualization } from "../extensions/virtualization/index.js"; function useForceUpdate() { const [, setValue] = useState(0); @@ -51,7 +50,6 @@ export const useEditor = ( const oldIsFocused = editor.isFocused; destroyView(editor.view); - installVirtualization(editor); // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore instead of creating a new editor, we just create // a new view. Due to some reason this is faster than resetting