Compare commits

..

1 Commits

Author SHA1 Message Date
agent_coder e9e4c1028d perf(editor): cut per-keystroke work on the typing hot path (#343)
The editor lagged while typing (worse with doc size, and under collaboration the
same cost is paid for every REMOTE keystroke). ProseMirror itself was fine — the
overhead was the surrounding work done on every transaction. Behavior is 1:1;
only WHEN work runs changed.

- getJSON() off the keystroke path: `onUpdate` no longer serializes the whole doc
  synchronously — the serialization now runs inside a 3s debounce (new hook
  use-page-content-cache.ts), flushed on unmount so the last snapshot isn't lost.
- footnote numbering: merged 3 per-docChanged O(n) doc walks into one, and
  short-circuit the whole-doc renumber when the doc has no footnotes and the
  transaction didn't insert one (step-slice scan — covers typing/paste/collab).
- toolbar: replaced per-keystroke `editor.can().undo()/.redo()` dry-runs with
  cheap history-depth reads (Yjs undoManager stack length / pm-history depth).
- render side-effect bug: `remote.attach()` moved out of the render body into a
  useEffect.
- debounced the TOC all-headings rescan and memoized the slash-command suggestion
  build (was rebuilt twice per keystroke).
- node menus (image/video/audio/pdf/callout/subpages): the per-transaction
  selectors early-return a cheap isActive check instead of running getAttributes +
  multiple alignment probes while their node type is inactive (shouldShow still
  controls display — appears exactly when it did).
- code blocks: the global selectionUpdate listener is now added only for mermaid
  blocks (the only consumer of the selected state), eliminating N listeners +
  N setStates per caret move for normal code blocks.

Deferred (documented, collab hot-path risk): full conditional menu MOUNTING
(menu-less-frame risk on same-tx context switch) and code-block re-tokenization
debounce / language-persist (self-dispatching meta tx + node-attr writes interact
with collab/undo). The route split from #342 already keeps lowlight off startup.

Gate: editor-ext build + 252/252 tests, client editor tests pass, tsc --noEmit 0,
client build ok. New tests: footnote no-footnote-doc → 0 traversals + numbering
unchanged; page-content-cache onUpdate-no-sync-getJSON + flush-on-unmount.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 22:49:48 +03:00
21 changed files with 548 additions and 170 deletions
+11 -42
View File
@@ -18,48 +18,12 @@ env:
IMAGE: ghcr.io/vvzvlad/gitmost
jobs:
# Run the reusable test suite. Together with the e2e jobs below it gates the
# publish job (the image push), not the build itself — build runs in parallel.
# Run the reusable test suite first so a failing test blocks the image build.
test:
uses: ./.github/workflows/test.yml
# Runs in parallel with the test/e2e jobs and only warms the buildx cache
# (GHA cache, scope develop-amd64). No push happens here — the publish job
# below is the only one that pushes the image.
build:
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
- name: Resolve version
id: version
run: echo "value=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
- name: Build develop image (warm cache, no push)
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/amd64
build-args: |
APP_VERSION=${{ steps.version.outputs.value }}
AI_AGENT_ROLES_CATALOG_URL=https://raw.githubusercontent.com/vvzvlad/gitmost/develop/agent-roles-catalog
push: false
cache-from: type=gha,scope=develop-amd64
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
# The gate: rebuilds from the cache the build job just wrote (near-instant on
# a cache hit; worst case — cache eviction — a full rebuild, which matches the
# old sequential timing) and pushes :develop only when unit tests AND both
# e2e suites AND the build are green.
publish:
needs: [test, e2e-server, e2e-mcp, build]
needs: test
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
@@ -93,10 +57,13 @@ jobs:
push: true
tags: ${{ env.IMAGE }}:develop
cache-from: type=gha,scope=develop-amd64
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
# e2e jobs gate the publish (image push), not the build: the :develop image
# is pushed only when unit tests AND both e2e suites pass (publish.needs
# lists them all).
# e2e jobs run on every develop push but DO NOT gate the build/publish above:
# `build` stays `needs: test` only, so the :develop image still ships even if
# e2e fails. A failing e2e job turns the run red and triggers GitHub's email
# to the pusher — that red run + email is the intended notification, not a
# deploy block.
e2e-server:
runs-on: ubuntu-latest
# Hard cap: the full-AppModule e2e leaks open handles and hung jest to the 6h max.
@@ -157,7 +124,9 @@ jobs:
- name: Run server e2e
run: pnpm --filter ./apps/server test:e2e
# Gates the publish too — see the comment above e2e-server.
# Same rationale as e2e-server: this job is intentionally NOT in
# `build.needs`. Deploy of the :develop image must not be blocked by e2e;
# a red run plus GitHub's email to the pusher is the notification mechanism.
e2e-mcp:
runs-on: ubuntu-latest
timeout-minutes: 20
-43
View File
@@ -13,49 +13,6 @@ permissions:
contents: read
jobs:
# Guard against a long-lived branch adding a migration whose timestamped
# filename sorts BEFORE migrations already applied on the target branch (and
# thus in prod). The Kysely startup migrator rejects that as "corrupted
# migrations" and crash-loops the app on boot (incident #361). This gate fails
# the PR so the migration is renamed to a current timestamp before merge. Only
# runs for pull_request events (needs a base branch to diff against).
migration-order:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Checkout (full history for the base-branch diff)
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Added migrations must sort after the newest on the base branch
env:
TARGET_BRANCH: ${{ github.base_ref }}
run: |
set -euo pipefail
MIG_DIR="apps/server/src/database/migrations"
# checkout above already did fetch-depth:0 (full history). Fetch the base
# WITHOUT --depth (a shallow graft would truncate the base history and
# break the merge-base when the base has moved ahead of the PR merge —
# exactly the long-branch-vs-moving-base case this gate guards, #361).
git fetch --no-tags origin "$TARGET_BRANCH"
newest_on_target=$(git ls-tree -r --name-only "origin/${TARGET_BRANCH}" "$MIG_DIR" | sort | tail -1)
# NO `|| true`: a diff failure (e.g. an unresolved merge-base) must fail
# the job CLOSED — a gate whose job is to BLOCK must never pass on error.
# `set -e` above already aborts on a non-zero diff exit.
added=$(git diff --diff-filter=A --name-only "origin/${TARGET_BRANCH}...HEAD" -- "$MIG_DIR")
bad=0
for f in $added; do
if [[ "$f" < "$newest_on_target" || "$f" == "$newest_on_target" ]]; then
echo "::error::Migration $f sorts at or before the newest on ${TARGET_BRANCH} ($newest_on_target) — rename it with a CURRENT timestamp before merge (do not change its contents). See incident #361."
bad=1
fi
done
if [ "$bad" -eq 0 ]; then
echo "Migration order OK (added migrations all sort after $newest_on_target)."
fi
exit $bad
test:
runs-on: ubuntu-latest
timeout-minutes: 20
+1 -4
View File
@@ -250,10 +250,7 @@ pnpm --filter server migration:codegen # regenerate src/databa
```
Migration files live in `apps/server/src/database/migrations/` and are named `YYYYMMDDThhmmss-description.ts`. Fork-specific migrations only **add** tables (`page_embeddings`, `ai_chats`, `ai_chat_messages`, `ai_provider_credentials`, `ai_mcp_servers`, `page_template_references`) and columns (e.g. `pages.is_template`, a `NOT NULL DEFAULT false` boolean) — never drop/rewrite Docmost data.
**Migration ordering — always check when merging branches/features.** Kysely runs migrations in **alphabetical (= timestamp) order**. A *new* migration that sorts **before** one already applied to the DB is a "back-dated" migration, which branches developed in parallel routinely produce: a feature branch adds `…T130000-…`, `develop` meanwhile ships and deploys `…T150000-…`, and after the merge the older-timestamped file has been skipped. Two layers guard this (both added for incident #361, where a back-dated migration crash-looped prod for ~11 min):
- **CI gate (primary):** the `migration-order` job in `.github/workflows/test.yml` fails a PR whose added migration sorts at/before the newest on the base branch. **So the fix is to rename your migration to a timestamp after the latest one already in the target** (`/bin/ls -1 apps/server/src/database/migrations | sort | tail`; content unchanged — the filename is the ordering key), then rebuild so the compiled `dist/database/migrations/` picks up the new name.
- **Runtime safety net:** both Migrators (`migration.service.ts` startup auto-migrate + `migrate.ts` CLI) set `allowUnorderedMigrations: true`, so the app does **not** refuse to start on an out-of-order migration — it applies the skipped older one instead of crash-looping. Kysely's `#ensureNoMissingMigrations` guard is still on (a *removed* applied migration is still an error). Because apply order can then differ from lexicographic across instances, migrations must stay **independent** (each creates its own objects) — the CI gate remains the primary line; this net only covers a gate bypass (manual push / hotfix branch).
**Migration ordering — always check when merging branches/features.** Kysely runs migrations in **alphabetical (= timestamp) order** and refuses to start if a *new* migration sorts **before** one already applied to the DB (`corrupted migrations: ... must always have a name that comes alphabetically after the last executed migration`). When you merge a branch or land a feature, verify your migration's timestamp still sorts **after every migration that may already be applied on the target** (`/bin/ls -1 apps/server/src/database/migrations | sort | tail`). Branches developed in parallel routinely break this: a feature branch adds `…T130000-…`, `main` meanwhile ships and deploys `…T150000-…`, and after the merge the older-timestamped file is rejected at boot. **Fix = rename your migration to a timestamp after the latest one already in the target** (content unchanged — the filename is the ordering key), then rebuild so the compiled `dist/database/migrations/` picks up the new name.
## Architecture — the big picture
@@ -46,6 +46,13 @@ export function AudioMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes unless an audio node is active. The menu
// only shows for an active audio node (shouldShow), so the null state while
// inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("audio")) {
return null;
}
const audioAttrs = ctx.editor.getAttributes("audio");
return {
@@ -43,8 +43,15 @@ export function CalloutMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip the per-type isActive() probes unless a callout is
// active. The menu only shows for an active callout (shouldShow), so the
// null state while inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("callout")) {
return null;
}
return {
isCallout: ctx.editor.isActive("callout"),
isCallout: true,
isInfo: ctx.editor.isActive("callout", { type: "info" }),
isNote: ctx.editor.isActive("callout", { type: "note" }),
isSuccess: ctx.editor.isActive("callout", { type: "success" }),
@@ -22,6 +22,12 @@ export default function CodeBlockView(props: NodeViewProps) {
const [isSelected, setIsSelected] = useState(false);
useEffect(() => {
// #343 PART 6: `isSelected` only drives the mermaid source's visibility (the
// `hidden` prop below). For every non-mermaid code block it is never read,
// so skip the per-block `selectionUpdate` listener entirely — otherwise N
// code blocks each add a global listener + a setState on every caret move.
if (language !== "mermaid") return;
const updateSelection = () => {
const { state } = editor;
const { from, to } = state.selection;
@@ -32,11 +38,14 @@ export default function CodeBlockView(props: NodeViewProps) {
setIsSelected(isNodeSelected);
};
// Initialize on attach so switching a block's language to "mermaid" reflects
// the current selection immediately (the listener was not running before).
updateSelection();
editor.on("selectionUpdate", updateSelection);
return () => {
editor.off("selectionUpdate", updateSelection);
};
}, [editor, getPos(), node.nodeSize]);
}, [editor, getPos(), node.nodeSize, language]);
function changeLanguage(language: string) {
setLanguageValue(language);
@@ -1,5 +1,7 @@
import type { Editor } from "@tiptap/react";
import { useEditorState } from "@tiptap/react";
import { undoDepth, redoDepth } from "@tiptap/pm/history";
import { yUndoPluginKey } from "@tiptap/y-tiptap";
export interface ToolbarState {
isBold: boolean;
@@ -16,14 +18,45 @@ export interface ToolbarState {
canRedo: boolean;
}
// Undo/redo come from either StarterKit's history or the Yjs collaboration
// history extension. During the brief moment a page is rendered with the
// static editor (mainExtensions only, undoRedo disabled), neither is loaded
// and editor.can().undo/redo is undefined.
function safeCan(editor: Editor, command: "undo" | "redo"): boolean {
const can = editor.can() as Record<string, unknown>;
const fn = can[command];
return typeof fn === "function" ? (fn as () => boolean)() : false;
// Undo/redo availability, computed WITHOUT `editor.can().undo()/.redo()`.
//
// `editor.can()` runs the command as a dry-run (building a throwaway state +
// transaction) — the most expensive work in this selector, and it ran on every
// keystroke (and every REMOTE keystroke under collaboration). Instead we read
// the history stack depth directly, which is a cheap plugin-state lookup and
// mirrors exactly what the undo/redo commands themselves check:
//
// - Collaboration (Yjs): the yjs UndoManager's undo/redo stack lengths — the
// same `undoStack.length === 0` / `redoStack.length === 0` guard the
// Collaboration extension's undo/redo commands use.
// - Plain history (templates / non-collab): prosemirror-history's undoDepth /
// redoDepth, which back the UndoRedo extension.
//
// When neither history backend is installed (the pre-sync static editor —
// mainExtensions only, undoRedo disabled), both fall through to 0 -> false,
// matching the previous `safeCan` behavior.
function historyAvailability(editor: Editor): {
canUndo: boolean;
canRedo: boolean;
} {
const state = editor.state;
// Collaboration history (Yjs) takes precedence when present.
const yState = yUndoPluginKey.getState(state) as
| { undoManager?: { undoStack: unknown[]; redoStack: unknown[] } }
| undefined;
if (yState?.undoManager) {
return {
canUndo: yState.undoManager.undoStack.length > 0,
canRedo: yState.undoManager.redoStack.length > 0,
};
}
// Plain prosemirror-history (returns 0 when the history plugin is absent).
return {
canUndo: undoDepth(state) > 0,
canRedo: redoDepth(state) > 0,
};
}
export function useToolbarState(editor: Editor | null): ToolbarState | null {
@@ -31,6 +64,7 @@ export function useToolbarState(editor: Editor | null): ToolbarState | null {
editor,
selector: (ctx) => {
if (!ctx.editor) return null;
const { canUndo, canRedo } = historyAvailability(ctx.editor);
return {
isBold: ctx.editor.isActive("bold"),
isItalic: ctx.editor.isActive("italic"),
@@ -42,8 +76,8 @@ export function useToolbarState(editor: Editor | null): ToolbarState | null {
isBulletList: ctx.editor.isActive("bulletList"),
isOrderedList: ctx.editor.isActive("orderedList"),
isTaskList: ctx.editor.isActive("taskList"),
canUndo: safeCan(ctx.editor, "undo"),
canRedo: safeCan(ctx.editor, "redo"),
canUndo,
canRedo,
};
},
});
@@ -38,6 +38,14 @@ export function ImageMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip the expensive per-keystroke work (getAttributes + the
// alignment isActive() probes) unless an image is actually active. The
// menu is only shown when an image is active (see shouldShow), so a null
// state while inactive is never rendered — behavior is unchanged.
if (!ctx.editor.isActive("image")) {
return null;
}
const imageAttrs = ctx.editor.getAttributes("image");
return {
@@ -25,6 +25,13 @@ export function PdfMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes unless a pdf node is active. The menu
// only shows for an active pdf node (shouldShow), so the null state while
// inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("pdf")) {
return null;
}
const pdfAttrs = ctx.editor.getAttributes("pdf");
return {
@@ -70,7 +70,14 @@ export const SubpagesMenu = React.memo(
// toggle without re-rendering on every keystroke.
const isRecursive = useEditorState({
editor,
selector: (ctx) => ctx.editor?.getAttributes("subpages")?.recursive ?? false,
// #343 PART 1: skip getAttributes unless a subpages node is active. The
// menu only shows for an active subpages node (shouldShow), so the value
// is only read then; getAttributes on an inactive node returns the default
// (recursive === false) anyway, so this is behavior-preserving.
selector: (ctx) =>
ctx.editor?.isActive("subpages")
? (ctx.editor.getAttributes("subpages")?.recursive ?? false)
: false,
});
return (
@@ -4,6 +4,7 @@ import React, { FC, useEffect, useRef, useState } from "react";
import classes from "./table-of-contents.module.css";
import clsx from "clsx";
import { Box, Text, Title } from "@mantine/core";
import { useDebouncedCallback } from "@mantine/hooks";
import { useTranslation } from "react-i18next";
type TableOfContentsProps = {
@@ -79,13 +80,21 @@ export const TableOfContents: FC<TableOfContentsProps> = (props) => {
setHeadingDOMNodes(result.nodes);
};
// Debounce the update-driven rescan: `$nodes("heading")` scans every heading
// in the document, and it previously ran on EVERY keystroke while the TOC
// panel was open. The panel is derived UI, so recomputing ~300ms after typing
// settles keeps it correct without doing an all-headings scan per keystroke
// (#343, PART 7). `useDebouncedCallback` returns a stable reference and always
// invokes the latest `handleUpdate`.
const debouncedHandleUpdate = useDebouncedCallback(handleUpdate, 300);
useEffect(() => {
props.editor?.on("update", handleUpdate);
props.editor?.on("update", debouncedHandleUpdate);
return () => {
props.editor?.off("update", handleUpdate);
props.editor?.off("update", debouncedHandleUpdate);
};
}, [props.editor]);
}, [props.editor, debouncedHandleUpdate]);
useEffect(
() => {
@@ -31,6 +31,13 @@ export function VideoMenu({ editor }: EditorMenuProps) {
return null;
}
// #343 PART 1: skip getAttributes + alignment isActive() probes unless a
// video is active. The menu only shows for an active video (shouldShow),
// so the null state while inactive is never rendered — behavior unchanged.
if (!ctx.editor.isActive("video")) {
return null;
}
const videoAttrs = ctx.editor.getAttributes("video");
return {
@@ -6,6 +6,23 @@ import getSuggestionItems from '@/features/editor/components/slash-menu/menu-ite
export const slashMenuPluginKey = new PluginKey('slash-command');
// getSuggestionItems fuzzy-matches EVERY command against the query (plus its
// wrong-keyboard-layout remaps) and, while the slash menu is open, is invoked
// TWICE per keystroke: once by the synchronous `allow` gate below and once by
// the popup's `items` builder. A synchronous gating predicate can't be
// debounced without breaking the suggestion decoration/activation, so instead we
// memoize the LAST query's result: the two same-query calls in one keystroke
// build the list only once, and the cache invalidates the moment the query
// changes — so there is no stale-state risk (#343, PART 7).
let lastQuery: string | null = null;
let lastResult: ReturnType<typeof getSuggestionItems> | null = null;
function suggestionItemsForQuery(query: string) {
if (query === lastQuery && lastResult) return lastResult;
lastQuery = query;
lastResult = getSuggestionItems({ query });
return lastResult;
}
// @ts-ignore
const Command = Extension.create({
name: 'slash-command',
@@ -38,7 +55,7 @@ const Command = Extension.create({
// non-matching queries while keeping multi-word matches (e.g.
// "/Heading 1") working.
const query = state.doc.textBetween(range.from + 1, range.to);
const groups = getSuggestionItems({ query });
const groups = suggestionItemsForQuery(query);
const hasMatches = Object.values(groups).some(
(items) => items.length > 0,
);
@@ -61,7 +78,9 @@ const Command = Extension.create({
const SlashCommand = Command.configure({
suggestion: {
items: getSuggestionItems,
// Share the per-query memo with `allow` so the pair of same-query calls in a
// single keystroke rebuilds the list once (#343, PART 7).
items: ({ query }: { query: string }) => suggestionItemsForQuery(query),
render: renderItems,
},
});
@@ -0,0 +1,100 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { renderHook, act } from "@testing-library/react";
import type { MutableRefObject } from "react";
import type { Editor } from "@tiptap/react";
// Mock the app entry so importing the hook doesn't boot the whole app; the hook
// only needs queryClient's cache read/write, which we stub here. Declared via
// vi.hoisted so the spies exist before the hoisted vi.mock factory runs.
const { getQueryData, setQueryData } = vi.hoisted(() => ({
getQueryData: vi.fn(() => undefined as unknown),
setQueryData: vi.fn(),
}));
vi.mock("@/main.tsx", () => ({
queryClient: { getQueryData, setQueryData },
}));
import { usePageContentCache } from "./use-page-content-cache";
const SNAPSHOT = { type: "doc", content: [] };
function makeFakeEditor(overrides: Partial<Editor> = {}): Editor {
return {
isEmpty: false,
isDestroyed: false,
getJSON: vi.fn(() => SNAPSHOT),
...overrides,
} as unknown as Editor;
}
describe("usePageContentCache (#343 PART 3) — getJSON off the keystroke path", () => {
beforeEach(() => {
vi.useFakeTimers();
vi.clearAllMocks();
// A cached page exists so the write path runs.
getQueryData.mockReturnValue({ id: "p1", content: {} });
});
afterEach(() => {
vi.useRealTimers();
});
it("onUpdate (calling the debounced fn) does NOT call getJSON synchronously", () => {
const editor = makeFakeEditor();
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
// Simulate a keystroke's onUpdate -> only schedules the debounce.
act(() => {
result.current();
result.current();
result.current();
});
// The whole-doc serialization must NOT have happened yet.
expect(editor.getJSON).not.toHaveBeenCalled();
expect(setQueryData).not.toHaveBeenCalled();
// Once the debounce window elapses, getJSON runs exactly once (not per call).
act(() => vi.advanceTimersByTime(3000));
expect(editor.getJSON).toHaveBeenCalledTimes(1);
expect(setQueryData).toHaveBeenCalledWith(["pages", "slug-1"], {
id: "p1",
content: SNAPSHOT,
});
});
it("flushes the pending snapshot on unmount so the last edit isn't lost", () => {
const editor = makeFakeEditor();
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result, unmount } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
act(() => result.current());
expect(editor.getJSON).not.toHaveBeenCalled();
// Navigation/unmount must flush (not drop) the pending write.
act(() => unmount());
expect(editor.getJSON).toHaveBeenCalledTimes(1);
expect(setQueryData).toHaveBeenCalledTimes(1);
});
it("skips the write when the editor is destroyed (flush racing teardown)", () => {
const editor = makeFakeEditor({ isDestroyed: true });
const editorRef = { current: editor } as MutableRefObject<Editor | null>;
const { result } = renderHook(() =>
usePageContentCache(editorRef, "slug-1", 3000),
);
act(() => result.current());
act(() => vi.advanceTimersByTime(3000));
expect(editor.getJSON).not.toHaveBeenCalled();
expect(setQueryData).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,50 @@
import type { MutableRefObject } from "react";
import { useDebouncedCallback } from "@mantine/hooks";
import type { Editor } from "@tiptap/react";
import { queryClient } from "@/main.tsx";
import { IPage } from "@/features/page/types/page.types.ts";
/**
* Off-keystroke local page-cache updater (issue #343, PART 3).
*
* The editor's `onUpdate` fires on every keystroke — and, under collaboration,
* on every REMOTE keystroke too. Serializing the WHOLE document with
* `editor.getJSON()` on that hot path is expensive, and the previous 3s debounce
* only guarded the cache WRITE, not the serialization: `getJSON()` still ran per
* keystroke.
*
* This hook moves the serialization INSIDE the debounced callback, so the
* full-doc traversal happens at most once per `delay`, not per keystroke. Call
* the returned function from `onUpdate` (it only schedules the debounce); the
* `getJSON()` snapshot is taken when the debounce fires.
*
* On unmount/navigation the pending snapshot is FLUSHED (via `flushOnUnmount`)
* so the last edits within the debounce window aren't lost from the local cache.
* The source of truth is collab/Yjs, but the cache must not go stale.
*
* IMPORTANT: call this hook BEFORE `useEditor`. React runs effect cleanups in
* declaration order on unmount, so the debounce's flush cleanup must be declared
* before `useEditor`'s teardown to run while the editor is still alive; the
* `isDestroyed` guard keeps a flush that still races teardown safe (it skips).
*/
export function usePageContentCache(
editorRef: MutableRefObject<Editor | null>,
slugId: string | undefined,
delay = 3000,
) {
return useDebouncedCallback(
() => {
const e = editorRef.current;
if (!e || e.isDestroyed || e.isEmpty) return;
const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
if (pageData) {
// getJSON() (full-doc serialization) runs HERE, off the keystroke path.
queryClient.setQueryData(["pages", slugId], {
...pageData,
content: e.getJSON(),
});
}
},
{ delay, flushOnUnmount: true },
);
}
+20 -19
View File
@@ -62,7 +62,7 @@ import ExcalidrawMenu from "./components/excalidraw/excalidraw-menu-lazy";
import DrawioMenu from "./components/drawio/drawio-menu";
import { useCollabToken } from "@/features/auth/queries/auth-query.tsx";
import SearchAndReplaceDialog from "@/features/editor/components/search-and-replace/search-and-replace-dialog.tsx";
import { useDebouncedCallback, useDocumentVisibility } from "@mantine/hooks";
import { useDocumentVisibility } from "@mantine/hooks";
import { useIdle } from "@/hooks/use-idle.ts";
import { queryClient } from "@/main.tsx";
import { IPage } from "@/features/page/types/page.types.ts";
@@ -79,6 +79,7 @@ import { PageEditMode } from "@/features/user/types/user.types.ts";
import { jwtDecode } from "jwt-decode";
import { searchSpotlight } from "@/features/search/constants.ts";
import { useEditorScroll } from "./hooks/use-editor-scroll";
import { usePageContentCache } from "./hooks/use-page-content-cache";
import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
import { useSwapHeightReservation } from "./hooks/use-swap-height-reservation";
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
@@ -267,8 +268,13 @@ export default function PageEditor({
}
}, [isIdle, documentState, providersReady, resetIdle]);
// Attach here, to make sure the connection gets properly established
providersRef.current?.remote.attach();
// Attach the remote provider once it's ready (and again after a pageId swap
// recreates it) to make sure the connection gets properly established. This
// used to run in the render body — a side effect during render (#343, PART 7).
// `attach()` is idempotent, so re-running it on these deps is safe.
useEffect(() => {
providersRef.current?.remote.attach();
}, [providersReady, pageId]);
const extensions = useMemo(() => {
if (!providersReady || !providersRef.current || !currentUser?.user) {
@@ -283,6 +289,12 @@ export default function PageEditor({
];
}, [providersReady, currentUser?.user]);
// getJSON() serialization + cache write live in the hook, off the keystroke
// path, and flush on unmount so the last snapshot survives navigation (#343).
// MUST be declared before useEditor: React runs effect cleanups in declaration
// order on unmount, so the flush must run before the editor is torn down.
const debouncedUpdateContent = usePageContentCache(editorRef, slugId);
const editor = useEditor(
{
extensions,
@@ -353,11 +365,11 @@ export default function PageEditor({
editorRef.current = editor;
}
},
onUpdate({ editor }) {
if (editor.isEmpty) return;
const editorJson = editor.getJSON();
//update local page cache to reduce flickers
debouncedUpdateContent(editorJson);
onUpdate() {
// Only schedule the debounce here — the whole-doc getJSON() serialization
// happens INSIDE the debounced callback (see usePageContentCache), so it
// no longer runs synchronously on every (local or remote) keystroke.
debouncedUpdateContent();
},
},
[pageId, editable, extensions],
@@ -403,17 +415,6 @@ export default function PageEditor({
};
}, [editor, pageId, editorIsEditable]);
const debouncedUpdateContent = useDebouncedCallback((newContent: any) => {
const pageData = queryClient.getQueryData<IPage>(["pages", slugId]);
if (pageData) {
queryClient.setQueryData(["pages", slugId], {
...pageData,
content: newContent,
});
}
}, 3000);
const handleActiveCommentEvent = (event) => {
const { commentId, resolved } = event.detail;
-4
View File
@@ -24,10 +24,6 @@ const migrator = new Migrator({
path,
migrationFolder,
}),
// Match the startup auto-migrator (migration.service.ts): a back-dated
// migration from a long-lived branch must be applied, not rejected as
// "corrupted migrations" (incident #361). See that file for the full rationale.
allowUnorderedMigrations: true,
});
run(db, migrator, migrationFolder);
@@ -19,16 +19,6 @@ export class MigrationService {
path,
migrationFolder: path.join(__dirname, '..', 'migrations'),
}),
// A long-lived branch can add a migration whose timestamped filename sorts
// BEFORE migrations already applied in prod (e.g. #234's 20260627 landing
// after 20260704 was live). With the default (ordered) setting the startup
// migrator then sees "corrupted migrations" — the applied set is no longer a
// prefix of the sorted list — throws, and the app crash-loops on boot
// (incident #361: 502s for ~11 min). allowUnorderedMigrations runs any
// not-yet-applied migration regardless of filename order, so a back-dated
// migration is applied instead of bricking startup. A CI order-gate still
// discourages back-dating; this is the runtime safety net.
allowUnorderedMigrations: true,
});
const { error, results } = await migrator.migrateToLatest();
@@ -0,0 +1,134 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { getSchema } from '@tiptap/core';
import { Document } from '@tiptap/extension-document';
import { Paragraph } from '@tiptap/extension-paragraph';
import { Text } from '@tiptap/extension-text';
import { EditorState } from '@tiptap/pm/state';
import { Node as PMNode } from '@tiptap/pm/model';
import { FootnoteReference } from './footnote-reference';
import { FootnotesList } from './footnotes-list';
import { FootnoteDefinition } from './footnote-definition';
import {
footnoteNumberingPlugin,
footnoteNumberingPluginKey,
getFootnoteNumber,
} from './footnote-numbering';
import {
FOOTNOTE_REFERENCE_NAME,
FOOTNOTES_LIST_NAME,
FOOTNOTE_DEFINITION_NAME,
} from './footnote-util';
const extensions = [
Document,
Paragraph,
Text,
FootnoteReference,
FootnotesList,
FootnoteDefinition,
];
const schema = getSchema(extensions);
function makeState(docJson: any): EditorState {
return EditorState.create({
doc: PMNode.fromJSON(schema, docJson),
plugins: [footnoteNumberingPlugin()],
});
}
const withTwoFootnotes = {
type: 'doc',
content: [
{
type: 'paragraph',
content: [
{ type: 'text', text: 'a' },
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'x' } },
{ type: 'text', text: 'b' },
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'y' } },
],
},
{
type: FOOTNOTES_LIST_NAME,
content: [
{
type: FOOTNOTE_DEFINITION_NAME,
attrs: { id: 'x' },
content: [{ type: 'paragraph' }],
},
{
type: FOOTNOTE_DEFINITION_NAME,
attrs: { id: 'y' },
content: [{ type: 'paragraph' }],
},
],
},
],
};
describe('footnote numbering plugin — short-circuit (#343 PART 5)', () => {
afterEach(() => vi.restoreAllMocks());
it('does ZERO document traversals on a docChanged transaction when the doc has no footnotes', () => {
const state = makeState({
type: 'doc',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hi' }] }],
});
// Only count traversals caused by the transaction, not the initial build.
const descendantsSpy = vi.spyOn(PMNode.prototype, 'descendants');
const before = footnoteNumberingPluginKey.getState(state);
// A real content edit (docChanged) that introduces no footnote node.
const next = state.apply(state.tr.insertText('!', 3));
const after = footnoteNumberingPluginKey.getState(next);
// The plugin never walked the document...
expect(descendantsSpy).not.toHaveBeenCalled();
// ...and reused the exact same (empty) state object — proof it short-circuited.
expect(after).toBe(before);
expect(after?.hasFootnotes).toBe(false);
});
it('rebuilds (numbering appears) the first time a footnote is inserted into a footnote-free doc', () => {
const state = makeState({
type: 'doc',
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'hi' }] }],
});
expect(footnoteNumberingPluginKey.getState(state)?.hasFootnotes).toBe(false);
const ref = schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: 'x' });
const next = state.apply(state.tr.insert(3, ref));
const after = footnoteNumberingPluginKey.getState(next);
expect(after?.hasFootnotes).toBe(true);
expect(getFootnoteNumber(next, 'x')).toBe(1);
});
});
describe('footnote numbering plugin — numbering unchanged with footnotes (#343 PART 5)', () => {
it('numbers references in document order via the single merged walk', () => {
const state = makeState(withTwoFootnotes);
expect(getFootnoteNumber(state, 'x')).toBe(1);
expect(getFootnoteNumber(state, 'y')).toBe(2);
});
it('produces a decoration for every reference and matching definition', () => {
const state = makeState(withTwoFootnotes);
const decos = footnoteNumberingPluginKey.getState(state)?.decorations;
// 2 references + 2 definitions = 4 number decorations.
expect(decos?.find().length).toBe(4);
});
it('keeps numbering current after an edit while footnotes exist', () => {
const state = makeState(withTwoFootnotes);
// Insert a NEW reference (id "z") before the others: it must become #1 and
// shift x -> #2, y -> #3 (deterministic document-order numbering).
const ref = schema.nodes[FOOTNOTE_REFERENCE_NAME].create({ id: 'z' });
const next = state.apply(state.tr.insert(1, ref));
expect(getFootnoteNumber(next, 'z')).toBe(1);
expect(getFootnoteNumber(next, 'x')).toBe(2);
expect(getFootnoteNumber(next, 'y')).toBe(3);
});
});
@@ -1,11 +1,9 @@
import { EditorState, Plugin, PluginKey } from '@tiptap/pm/state';
import { EditorState, Plugin, PluginKey, Transaction } from '@tiptap/pm/state';
import { Decoration, DecorationSet } from '@tiptap/pm/view';
import { Node as ProseMirrorNode } from '@tiptap/pm/model';
import { Node as ProseMirrorNode, Slice } from '@tiptap/pm/model';
import {
FOOTNOTE_DEFINITION_NAME,
FOOTNOTE_REFERENCE_NAME,
computeFootnoteNumbers,
computeFootnoteRefCounts,
} from './footnote-util';
export const footnoteNumberingPluginKey = new PluginKey<FootnoteNumberingState>(
@@ -27,8 +25,22 @@ interface FootnoteNumberingState {
refCounts: Map<string, number>;
/** Decorations rendering those numbers (refs + definitions). */
decorations: DecorationSet;
/** Whether the document contains ANY footnote reference/definition node.
* Cached so `apply` can skip the whole-doc walk on every keystroke in the
* common case (documents with no footnotes), recomputing only once a
* transaction actually inserts a footnote node (#343, PART 5). */
hasFootnotes: boolean;
}
/** Reusable empty state for footnote-free documents — avoids reallocating an
* empty map/decoration set on every keystroke while there are no footnotes. */
const EMPTY_STATE: FootnoteNumberingState = {
numbers: new Map(),
refCounts: new Map(),
decorations: DecorationSet.empty,
hasFootnotes: false,
};
/**
* Build the decoration set for footnote numbers. Pure function of the document:
* walk references in document order, assign 1-based numbers, then attach a
@@ -41,50 +53,101 @@ export function buildFootnoteDecorations(doc: ProseMirrorNode): DecorationSet {
return buildFootnoteNumberingState(doc).decorations;
}
function numberDecoration(pos: number, nodeSize: number, num: number): Decoration {
return Decoration.node(pos, pos + nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
});
}
/**
* Compute both the number map AND the decorations for `doc` in a single walk.
* The plugin caches the result so NodeViews can read numbers without
* recomputing.
* Compute the number map, reference counts AND the decorations for `doc` in a
* SINGLE document walk (previously three separate O(n) traversals per
* docChanged — computeFootnoteNumbers + computeFootnoteRefCounts + a decoration
* pass, #343 PART 5). The plugin caches the result so NodeViews can read numbers
* without recomputing.
*
* References are numbered and decorated as they are encountered (document
* order). Definition positions are collected during the same walk and decorated
* afterwards from the completed number map — so a definition that appears before
* its reference in document order still resolves to the correct number, and the
* output is identical to the previous three-pass implementation. (Decoration
* insertion order does not matter: DecorationSet.create indexes by position.)
*/
function buildFootnoteNumberingState(
doc: ProseMirrorNode,
): FootnoteNumberingState {
const numbers = computeFootnoteNumbers(doc);
const refCounts = computeFootnoteRefCounts(doc);
const numbers = new Map<string, number>();
const refCounts = new Map<string, number>();
const decorations: Decoration[] = [];
const definitions: { id: string; pos: number; nodeSize: number }[] = [];
let n = 0;
let hasFootnotes = false;
doc.descendants((node, pos) => {
if (node.type.name === FOOTNOTE_REFERENCE_NAME) {
const num = numbers.get(node.attrs.id);
if (num != null) {
decorations.push(
Decoration.node(pos, pos + node.nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
}),
);
}
}
if (node.type.name === FOOTNOTE_DEFINITION_NAME) {
const num = numbers.get(node.attrs.id);
if (num != null) {
decorations.push(
Decoration.node(pos, pos + node.nodeSize, {
'data-footnote-number': String(num),
style: `--footnote-number: "${num}";`,
}),
);
const typeName = node.type.name;
if (typeName === FOOTNOTE_REFERENCE_NAME) {
hasFootnotes = true;
const id = node.attrs.id;
if (id) {
if (!numbers.has(id)) numbers.set(id, ++n);
refCounts.set(id, (refCounts.get(id) ?? 0) + 1);
decorations.push(numberDecoration(pos, node.nodeSize, numbers.get(id)!));
}
} else if (typeName === FOOTNOTE_DEFINITION_NAME) {
hasFootnotes = true;
const id = node.attrs.id;
if (id != null) definitions.push({ id, pos, nodeSize: node.nodeSize });
}
});
if (!hasFootnotes) return EMPTY_STATE;
for (const def of definitions) {
const num = numbers.get(def.id);
if (num != null) {
decorations.push(numberDecoration(def.pos, def.nodeSize, num));
}
}
return {
numbers,
refCounts,
decorations: DecorationSet.create(doc, decorations),
hasFootnotes: true,
};
}
/**
* Cheap check: does any of a transaction's inserted content contain a footnote
* reference/definition node? Footnote nodes can only ENTER the document through
* replace steps (ReplaceStep / ReplaceAroundStep both expose a `.slice`), so
* scanning only the inserted slices — O(change size), not O(doc) — is sufficient
* to detect a newly-added footnote. Mark/attr steps never introduce nodes.
* Lets `apply` keep skipping the whole-doc walk until a footnote first appears.
*/
function transactionInsertsFootnote(tr: Transaction): boolean {
for (const step of tr.steps) {
const slice = (step as unknown as { slice?: Slice }).slice;
if (!slice || slice.content.size === 0) continue;
let found = false;
slice.content.descendants((node) => {
if (found) return false;
const typeName = node.type.name;
if (
typeName === FOOTNOTE_REFERENCE_NAME ||
typeName === FOOTNOTE_DEFINITION_NAME
) {
found = true;
return false;
}
return true;
});
if (found) return true;
}
return false;
}
/**
* Read the cached footnote number for `id` from the numbering plugin's state.
* This is the source NodeViews should use instead of calling
@@ -126,6 +189,13 @@ export function footnoteNumberingPlugin(): Plugin {
// the number map NodeViews read stays current on every edit while
// non-doc transactions (selection, etc.) reuse the cache for free.
if (!tr.docChanged) return old;
// Short-circuit the whole-doc walk while the document has no footnotes:
// if there were none and this transaction did not INSERT one, there is
// still nothing to number, so reuse the empty state (#343, PART 5). Once
// a footnote exists we always rebuild (covers renumbering/deletion).
if (!old.hasFootnotes && !transactionInsertsFootnote(tr)) {
return old;
}
return buildFootnoteNumberingState(tr.doc);
},
},
+1 -1
View File
@@ -450,7 +450,7 @@ async function main() {
// 8. get_page markdown round-trip sanity (table separator present)
const md = await client.getPage(pageId);
check("get_page md: table separator emitted", md.data.content.includes("| --- |"), "");
check("get_page md: callout exported as Obsidian '> [!info]'", md.data.content.includes("> [!info]"));
check("get_page md: callout exported as :::", md.data.content.includes(":::info"));
// 9. comments: create / list / reply / update / check_new / delete
const beforeComments = new Date(Date.now() - 1000).toISOString();