perf+fix(footnotes): minimal-diff sync (no concurrent-edit loss); cache numbering
Release-cycle review found two hardening gaps: - The sync plugin deleted+rebuilt the WHOLE footnotesList on any reorder/orphan, replacing every definition's Yjs subtree -> a collaborator typing in a definition could lose in-flight characters on merge. Rework to targeted, minimal mutations: attr-only setNodeMarkup for collision re-ids, delete only genuine orphans, insert only genuinely-missing definitions (at the list end, not shifting existing subtrees), and consolidate multiple lists only in the abnormal paste/merge case. An unchanged (correct id, referenced) definition is left completely untouched. Numbering is decoration-only, so physical list order may drift after a reorder (accepted) while displayed numbers stay correct. Invariants preserved (reviewed + tested): one SYNC_META transaction, null when canonical (terminates), deterministic deriveFootnoteId, remote-skip -> no re-introduced freeze or divergence. - computeFootnoteNumbers ran per-NodeView-render (O(n^2)/keystroke in big docs). The numbering plugin now caches the number map in its state (computed once per docChanged); NodeViews read it O(1) via getFootnoteNumber. Tests: no-rebuild-on-reorder asserts unchanged definition node subtrees are identity-preserved; isRemoteTransaction skip; enableSync:false read-only; cache correctness. Browser re-smoke: insert (no freeze), number, persist across reload, cascade delete all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
@@ -1,6 +1,6 @@
|
||||
import { NodeViewContent, NodeViewProps, NodeViewWrapper } from "@tiptap/react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { computeFootnoteNumbers } from "@docmost/editor-ext";
|
||||
import { getFootnoteNumber } from "@docmost/editor-ext";
|
||||
import classes from "./footnote.module.css";
|
||||
|
||||
/**
|
||||
@@ -13,8 +13,9 @@ export default function FootnoteDefinitionView(props: NodeViewProps) {
|
||||
const { t } = useTranslation();
|
||||
const id = node.attrs.id as string;
|
||||
|
||||
const numbers = computeFootnoteNumbers(editor.state.doc);
|
||||
const number = numbers.get(id) ?? "?";
|
||||
// Read the cached number from the numbering plugin (computed once per doc
|
||||
// change) rather than recomputing the whole map on every render.
|
||||
const number = getFootnoteNumber(editor.state, id) ?? "?";
|
||||
|
||||
const handleBack = (e: React.MouseEvent) => {
|
||||
e.preventDefault();
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
} from "@floating-ui/dom";
|
||||
import {
|
||||
FOOTNOTE_DEFINITION_NAME,
|
||||
computeFootnoteNumbers,
|
||||
getFootnoteNumber,
|
||||
} from "@docmost/editor-ext";
|
||||
import { ActionIcon } from "@mantine/core";
|
||||
import { IconArrowDown } from "@tabler/icons-react";
|
||||
@@ -45,9 +45,10 @@ export default function FootnoteReferenceView(props: NodeViewProps) {
|
||||
const popoverRef = useRef<HTMLDivElement | null>(null);
|
||||
const [open, setOpen] = useState(false);
|
||||
|
||||
// Number is derived (not stored) — recompute from the current doc.
|
||||
const numbers = computeFootnoteNumbers(editor.state.doc);
|
||||
const number = numbers.get(id) ?? "?";
|
||||
// Number is derived (not stored). Read it from the numbering plugin's cached
|
||||
// map (computed once per doc change) instead of walking the whole document on
|
||||
// every render — recomputing per NodeView per render was O(n^2) per keystroke.
|
||||
const number = getFootnoteNumber(editor.state, id) ?? "?";
|
||||
const defText = open ? getDefinitionText(editor, id) : "";
|
||||
|
||||
const position = useCallback(() => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Plugin, PluginKey } from "@tiptap/pm/state";
|
||||
import { EditorState, Plugin, PluginKey } from "@tiptap/pm/state";
|
||||
import { Decoration, DecorationSet } from "@tiptap/pm/view";
|
||||
import { Node as ProseMirrorNode } from "@tiptap/pm/model";
|
||||
import {
|
||||
@@ -7,7 +7,23 @@ import {
|
||||
computeFootnoteNumbers,
|
||||
} from "./footnote-util";
|
||||
|
||||
export const footnoteNumberingPluginKey = new PluginKey("footnoteNumbering");
|
||||
export const footnoteNumberingPluginKey = new PluginKey<FootnoteNumberingState>(
|
||||
"footnoteNumbering",
|
||||
);
|
||||
|
||||
/**
|
||||
* Cached state of the numbering plugin. Both the displayed-number map and the
|
||||
* decoration set are computed ONCE per doc-changing transaction (in `apply`) and
|
||||
* cached here, so NodeViews can read a footnote's number by id without walking
|
||||
* the whole document on every React render (which was O(n^2) per keystroke in
|
||||
* large docs).
|
||||
*/
|
||||
interface FootnoteNumberingState {
|
||||
/** referenceId -> 1-based display number, for the current doc. */
|
||||
numbers: Map<string, number>;
|
||||
/** Decorations rendering those numbers (refs + definitions). */
|
||||
decorations: DecorationSet;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the decoration set for footnote numbers. Pure function of the document:
|
||||
@@ -18,6 +34,17 @@ export const footnoteNumberingPluginKey = new PluginKey("footnoteNumbering");
|
||||
* with no document mutation.
|
||||
*/
|
||||
export function buildFootnoteDecorations(doc: ProseMirrorNode): DecorationSet {
|
||||
return buildFootnoteNumberingState(doc).decorations;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
function buildFootnoteNumberingState(
|
||||
doc: ProseMirrorNode,
|
||||
): FootnoteNumberingState {
|
||||
const numbers = computeFootnoteNumbers(doc);
|
||||
const decorations: Decoration[] = [];
|
||||
|
||||
@@ -46,7 +73,21 @@ export function buildFootnoteDecorations(doc: ProseMirrorNode): DecorationSet {
|
||||
}
|
||||
});
|
||||
|
||||
return DecorationSet.create(doc, decorations);
|
||||
return { numbers, decorations: DecorationSet.create(doc, decorations) };
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the cached footnote number for `id` from the numbering plugin's state.
|
||||
* This is the source NodeViews should use instead of calling
|
||||
* computeFootnoteNumbers() on every render (that walked the whole doc per
|
||||
* NodeView per render = O(n^2) per keystroke). Returns undefined if the plugin
|
||||
* is not installed or the id has no number yet.
|
||||
*/
|
||||
export function getFootnoteNumber(
|
||||
state: EditorState,
|
||||
id: string,
|
||||
): number | undefined {
|
||||
return footnoteNumberingPluginKey.getState(state)?.numbers.get(id);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -59,16 +100,19 @@ export function footnoteNumberingPlugin(): Plugin {
|
||||
key: footnoteNumberingPluginKey,
|
||||
state: {
|
||||
init(_, { doc }) {
|
||||
return buildFootnoteDecorations(doc);
|
||||
return buildFootnoteNumberingState(doc);
|
||||
},
|
||||
apply(tr, old) {
|
||||
// Recompute (and re-cache) only when the document actually changed, so
|
||||
// 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;
|
||||
return buildFootnoteDecorations(tr.doc);
|
||||
return buildFootnoteNumberingState(tr.doc);
|
||||
},
|
||||
},
|
||||
props: {
|
||||
decorations(state) {
|
||||
return this.getState(state);
|
||||
return footnoteNumberingPluginKey.getState(state)?.decorations;
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
@@ -293,107 +293,237 @@ export function footnoteSyncPlugin(
|
||||
const plan = resolveCollisions(info);
|
||||
const referenceIds = plan.referenceIds;
|
||||
|
||||
// 1) Desired definitions: one per referenced id, in reference order,
|
||||
// reusing existing definition nodes (preserving their content) and
|
||||
// synthesizing empty ones for references that lack a definition.
|
||||
// Definitions whose id has no matching reference (true orphans) are
|
||||
// dropped per the existing orphan policy — but a collision is NEVER the
|
||||
// cause of a drop, because collisions were re-id'd above.
|
||||
const desiredDefs: ProseMirrorNode[] = referenceIds.map((id) => {
|
||||
const existing = plan.definitions.get(id);
|
||||
if (existing) {
|
||||
// A definition paired to a re-id'd reference keeps its CONTENT but
|
||||
// must carry the new id. Rewrite the id attr when it differs (cheap
|
||||
// no-op when it already matches).
|
||||
if (existing.attrs.id !== id) {
|
||||
return defType.create({ id }, existing.content);
|
||||
}
|
||||
return existing;
|
||||
}
|
||||
return defType.create({ id }, paragraphType.create());
|
||||
});
|
||||
// The set of ids that must have a definition, in reference order (after
|
||||
// collision re-id). De-duplicated already by resolveCollisions.
|
||||
const referenceIdSet = new Set(referenceIds);
|
||||
|
||||
// 2) Determine whether the document already matches the desired end-state.
|
||||
const hasRefs = desiredDefs.length > 0;
|
||||
// 1) For each definition occurrence, compute the id it should END UP with
|
||||
// (which differs from its current id only when collision resolution
|
||||
// re-id'd it). plan.definitions maps a FINAL id -> the chosen node, so
|
||||
// we invert it by node identity to recover each occurrence's target id.
|
||||
const finalIdByNode = new Map<ProseMirrorNode, string>();
|
||||
for (const [id, node] of plan.definitions) finalIdByNode.set(node, id);
|
||||
|
||||
// Is the existing single list already exactly the desired list, placed
|
||||
// after all meaningful content (nothing but empty paragraphs after it)?
|
||||
const isEmptyParagraph = (node: ProseMirrorNode) =>
|
||||
node.type === paragraphType && node.content.size === 0;
|
||||
|
||||
let alreadyCanonical = false;
|
||||
if (plan.changed) {
|
||||
// A collision was detected (duplicate ids among refs/defs). The doc must
|
||||
// be rewritten (re-id'd references + rebuilt list); it is never already
|
||||
// canonical in this case.
|
||||
alreadyCanonical = false;
|
||||
} else if (!hasRefs) {
|
||||
// Canonical when there is no footnotesList at all.
|
||||
alreadyCanonical = info.lists.length === 0;
|
||||
} else if (info.lists.length === 1) {
|
||||
const { pos, node } = info.lists[0];
|
||||
// Same definitions, same order, same identity (no rewrite needed)?
|
||||
const sameDefs =
|
||||
node.childCount === desiredDefs.length &&
|
||||
desiredDefs.every((d, i) => node.child(i) === d);
|
||||
// 2) Classify every existing definition occurrence:
|
||||
// - reId: keep the node in place, only change its id attr (collision).
|
||||
// - orphan: delete it (its final id has no matching reference).
|
||||
// A definition that already carries the right id and is referenced is
|
||||
// left COMPLETELY untouched (its Yjs subtree is preserved). This is the
|
||||
// core of the data-loss fix: a pure reference reorder produces NO
|
||||
// mutation of any definition subtree.
|
||||
interface DefReid {
|
||||
pos: number;
|
||||
node: ProseMirrorNode;
|
||||
newId: string;
|
||||
}
|
||||
const defReids: DefReid[] = [];
|
||||
const orphanDefs: DefOccurrence[] = [];
|
||||
// Track which referenced ids already have a surviving (non-orphan)
|
||||
// definition, so we can synthesize the genuinely missing ones.
|
||||
const satisfiedIds = new Set<string>();
|
||||
// Choose a "primary" list to receive inserts/migrated defs: the LAST list
|
||||
// whose placement is canonical (only empty paragraphs follow it), else the
|
||||
// last list, else none. New defs and consolidated defs land here.
|
||||
for (const occ of info.defOccurrences) {
|
||||
const finalId = finalIdByNode.get(occ.node) ?? occ.id;
|
||||
if (!referenceIdSet.has(finalId)) {
|
||||
orphanDefs.push(occ);
|
||||
continue;
|
||||
}
|
||||
if (occ.id !== finalId) {
|
||||
defReids.push({ pos: occ.pos, node: occ.node, newId: finalId });
|
||||
}
|
||||
satisfiedIds.add(finalId);
|
||||
}
|
||||
|
||||
// Placement: only empty paragraphs may follow the list.
|
||||
const listEnd = pos + node.nodeSize;
|
||||
let onlyEmptyParasAfter = true;
|
||||
// 3) Referenced ids with no surviving definition need a fresh empty one.
|
||||
const missingIds = referenceIds.filter((id) => !satisfiedIds.has(id));
|
||||
|
||||
// 4) Determine list topology.
|
||||
const hasRefs = referenceIds.length > 0;
|
||||
|
||||
// Pick the primary list: prefer the last canonically-placed list.
|
||||
const listIsTrailing = (listPos: number, listNode: ProseMirrorNode) => {
|
||||
const listEnd = listPos + listNode.nodeSize;
|
||||
let ok = true;
|
||||
doc.nodesBetween(listEnd, doc.content.size, (child, childPos) => {
|
||||
// Only inspect top-level children that start at/after the list end.
|
||||
if (childPos >= listEnd && child !== node) {
|
||||
if (!isEmptyParagraph(child)) onlyEmptyParasAfter = false;
|
||||
if (childPos >= listEnd && child !== listNode) {
|
||||
if (!isEmptyParagraph(child)) ok = false;
|
||||
}
|
||||
return false; // do not descend
|
||||
});
|
||||
|
||||
alreadyCanonical = sameDefs && onlyEmptyParasAfter;
|
||||
return ok;
|
||||
};
|
||||
let primaryList: { pos: number; node: ProseMirrorNode } | null = null;
|
||||
for (let i = info.lists.length - 1; i >= 0; i--) {
|
||||
if (listIsTrailing(info.lists[i].pos, info.lists[i].node)) {
|
||||
primaryList = info.lists[i];
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!primaryList && info.lists.length > 0) {
|
||||
primaryList = info.lists[info.lists.length - 1];
|
||||
}
|
||||
// Extra lists (everything except the primary) must be consolidated away.
|
||||
const extraLists = info.lists.filter((l) => l !== primaryList);
|
||||
const inExtraList = (pos: number) =>
|
||||
extraLists.some((l) => pos > l.pos && pos < l.pos + l.node.nodeSize);
|
||||
|
||||
if (alreadyCanonical) return null;
|
||||
// Definitions inside an extra list are migrated (recreated with the right
|
||||
// id) into the primary list, so drop their in-place re-id markups — the
|
||||
// whole extra list is deleted below and the markup would be wasted.
|
||||
const defReidsToApply = defReids.filter((r) => !inExtraList(r.pos));
|
||||
|
||||
// 3) Rebuild: produce exactly ONE transaction that reaches the end-state.
|
||||
// 5) Decide whether anything must change. The document is canonical when:
|
||||
// - no collisions were resolved (refs or defs), AND
|
||||
// - no orphan definitions, AND
|
||||
// - no missing definitions, AND
|
||||
// - exactly the right number of lists (0 when no refs, else 1) AND the
|
||||
// single list is canonically placed (trailing).
|
||||
const noChangeNeeded =
|
||||
!plan.changed &&
|
||||
defReids.length === 0 &&
|
||||
orphanDefs.length === 0 &&
|
||||
missingIds.length === 0 &&
|
||||
extraLists.length === 0 &&
|
||||
(hasRefs
|
||||
? info.lists.length === 1 && primaryList !== null
|
||||
: info.lists.length === 0);
|
||||
|
||||
if (noChangeNeeded) return null;
|
||||
|
||||
// 6) Apply the targeted, minimal mutations in ONE transaction. We never
|
||||
// delete-and-recreate an unchanged definition subtree; we only:
|
||||
// (a) re-id specific colliding references and definitions (attr-only),
|
||||
// (b) delete genuine orphan definitions and extra/empty lists,
|
||||
// (c) insert genuinely-missing empty definitions and migrate defs out
|
||||
// of extra lists into the primary list,
|
||||
// (d) create the primary list if references exist but none does yet.
|
||||
const tr = newState.tr;
|
||||
|
||||
// 3a) Re-id colliding body references FIRST. A footnoteReference is an
|
||||
// inline atom, so setNodeMarkup changes only its attrs (not its size),
|
||||
// leaving every other position valid for the list deletions/insert
|
||||
// that follow.
|
||||
// 6a) Re-id colliding references (inline atoms: attr-only, size-stable).
|
||||
for (const reid of plan.refReids) {
|
||||
tr.setNodeMarkup(reid.pos, undefined, {
|
||||
tr.setNodeMarkup(tr.mapping.map(reid.pos), undefined, {
|
||||
...reid.node.attrs,
|
||||
id: reid.newId,
|
||||
});
|
||||
}
|
||||
// 6b) Re-id colliding definitions IN PLACE (attr-only). This preserves the
|
||||
// definition's content subtree — never delete+recreate it.
|
||||
for (const reid of defReidsToApply) {
|
||||
tr.setNodeMarkup(tr.mapping.map(reid.pos), undefined, {
|
||||
...reid.node.attrs,
|
||||
id: reid.newId,
|
||||
});
|
||||
}
|
||||
|
||||
// Delete every existing footnotesList (from the end so earlier positions
|
||||
// stay valid while we mutate).
|
||||
[...info.lists]
|
||||
.sort((a, b) => b.pos - a.pos)
|
||||
.forEach(({ pos, node }) => {
|
||||
tr.delete(pos, pos + node.nodeSize);
|
||||
// 6c) Migrate non-orphan definitions out of every extra list into the
|
||||
// primary list (or, if there is no primary list, into a new one we
|
||||
// build), then delete the extra (now drained) lists. This is the only
|
||||
// path that moves a definition subtree, and it runs ONLY in the
|
||||
// abnormal multi-list case (paste/collab merge) — never on a plain
|
||||
// reorder, which keeps a single list untouched.
|
||||
const migrated: ProseMirrorNode[] = [];
|
||||
for (const extra of extraLists) {
|
||||
extra.node.forEach((defChild) => {
|
||||
if (defChild.type !== defType) return;
|
||||
const finalId = finalIdByNode.get(defChild) ?? defChild.attrs.id;
|
||||
if (!referenceIdSet.has(finalId)) return; // orphan: drop it
|
||||
migrated.push(
|
||||
defChild.attrs.id === finalId
|
||||
? defChild
|
||||
: defType.create({ id: finalId }, defChild.content),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
// 6c-bis) The definitions to INSERT into the primary list: migrated defs
|
||||
// from extra lists + freshly synthesized empty defs for references
|
||||
// that have no definition at all. Computed before deletions so we can
|
||||
// decide whether the primary list would be left empty.
|
||||
const toInsert: ProseMirrorNode[] = [
|
||||
...migrated,
|
||||
...missingIds.map((id) =>
|
||||
defType.create({ id }, paragraphType.create()),
|
||||
),
|
||||
];
|
||||
|
||||
// Does the primary list keep at least one definition after we strip its
|
||||
// orphans AND counting the defs we are about to insert? If it ends up
|
||||
// empty (an empty footnotesList is invalid schema), delete the WHOLE list
|
||||
// instead of leaving a hollow shell. Only the primary list can receive
|
||||
// inserts; extra lists are always deleted wholesale.
|
||||
let primarySurvivors = 0;
|
||||
if (primaryList) {
|
||||
primaryList.node.forEach((defChild) => {
|
||||
if (defChild.type !== defType) return;
|
||||
const finalId = finalIdByNode.get(defChild) ?? defChild.attrs.id;
|
||||
if (referenceIdSet.has(finalId)) primarySurvivors += 1;
|
||||
});
|
||||
}
|
||||
const primaryWillBeEmpty =
|
||||
!!primaryList && primarySurvivors === 0 && toInsert.length === 0;
|
||||
|
||||
// 6d) Delete orphan definitions, extra lists, and any list that would be
|
||||
// left empty. Sort deletions from the end so earlier positions stay
|
||||
// valid; map through tr.mapping to account for the (size-stable) re-id
|
||||
// markups and earlier deletions.
|
||||
const deletions: Array<{ from: number; to: number }> = [];
|
||||
const wholeListDeletes = new Set(extraLists);
|
||||
if (primaryWillBeEmpty && primaryList) wholeListDeletes.add(primaryList);
|
||||
|
||||
for (const occ of orphanDefs) {
|
||||
// Skip orphans inside a list that is being deleted wholesale.
|
||||
const inWholeDeleted = [...wholeListDeletes].some(
|
||||
(l) => occ.pos > l.pos && occ.pos < l.pos + l.node.nodeSize,
|
||||
);
|
||||
if (inWholeDeleted) continue;
|
||||
deletions.push({ from: occ.pos, to: occ.pos + occ.node.nodeSize });
|
||||
}
|
||||
for (const l of wholeListDeletes) {
|
||||
deletions.push({ from: l.pos, to: l.pos + l.node.nodeSize });
|
||||
}
|
||||
deletions
|
||||
.sort((a, b) => b.from - a.from)
|
||||
.forEach(({ from, to }) => {
|
||||
tr.delete(tr.mapping.map(from), tr.mapping.map(to));
|
||||
});
|
||||
|
||||
// If we deleted the primary list wholesale, it can no longer receive the
|
||||
// inserts below — null it out so a fresh list is created when needed.
|
||||
if (primaryWillBeEmpty) primaryList = null;
|
||||
|
||||
// 6e) Insert the migrated + synthesized definitions.
|
||||
if (hasRefs) {
|
||||
// Insert a single canonical list holding the desired definitions. Place
|
||||
// it after the last meaningful (non-empty-paragraph) top-level block, so
|
||||
// it lands before any trailing empty paragraph the trailing-node plugin
|
||||
// maintains. This keeps both plugins idempotent.
|
||||
if (primaryList) {
|
||||
if (toInsert.length > 0) {
|
||||
// Append at the end of the (mapped) primary list, just before its
|
||||
// closing token, so its existing definition subtrees are untouched.
|
||||
// We only changed attrs (size-stable) and deleted OTHER nodes, so
|
||||
// mapping the original list-end position forward lands at the same
|
||||
// boundary; -1 puts us just inside the list's closing token.
|
||||
const insertAt =
|
||||
tr.mapping.map(primaryList.pos + primaryList.node.nodeSize) - 1;
|
||||
tr.insert(insertAt, Fragment.fromArray(toInsert));
|
||||
}
|
||||
} else {
|
||||
// No usable list exists yet but references do — create one holding the
|
||||
// migrated + synthesized definitions, placed after the last meaningful
|
||||
// (non-empty-paragraph) top-level block so it sits before any trailing
|
||||
// empty paragraph the trailing-node plugin maintains.
|
||||
const mappedDoc = tr.doc;
|
||||
let insertPos = mappedDoc.content.size;
|
||||
for (let i = mappedDoc.childCount - 1; i >= 0; i--) {
|
||||
const child = mappedDoc.child(i);
|
||||
if (isEmptyParagraph(child)) {
|
||||
// skip trailing empty paragraphs; insert before them
|
||||
insertPos -= child.nodeSize;
|
||||
} else {
|
||||
break;
|
||||
if (isEmptyParagraph(child)) insertPos -= child.nodeSize;
|
||||
else break;
|
||||
}
|
||||
const list = listType.create(null, Fragment.fromArray(toInsert));
|
||||
tr.insert(insertPos, list);
|
||||
}
|
||||
|
||||
const merged = listType.create(null, Fragment.fromArray(desiredDefs));
|
||||
tr.insert(insertPos, merged);
|
||||
}
|
||||
|
||||
if (!tr.docChanged) return null;
|
||||
|
||||
@@ -6,10 +6,13 @@ import { Text } from "@tiptap/extension-text";
|
||||
import { Superscript } from "@tiptap/extension-superscript";
|
||||
import { Plugin, PluginKey } from "@tiptap/pm/state";
|
||||
import { Node as PMNode } from "@tiptap/pm/model";
|
||||
import { EditorState } from "@tiptap/pm/state";
|
||||
import { FootnoteReference } from "./footnote-reference";
|
||||
import { FootnotesList } from "./footnotes-list";
|
||||
import { FootnoteDefinition } from "./footnote-definition";
|
||||
import { TrailingNode } from "../trailing-node";
|
||||
import { footnoteSyncPlugin } from "./footnote-sync";
|
||||
import { getFootnoteNumber } from "./footnote-numbering";
|
||||
import {
|
||||
computeFootnoteNumbers,
|
||||
collectReferenceIds,
|
||||
@@ -688,3 +691,258 @@ describe("footnote sync plugin (no infinite loop — live editor)", () => {
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Data-loss-window regression guard (Fix 1). A pure reference REORDER must not
|
||||
* cause the sync plugin to delete-and-recreate any definition subtree — doing so
|
||||
* (the previous behaviour) would, through Yjs, replace the CRDT subtree of every
|
||||
* definition and could lose a collaborator's in-flight characters on merge.
|
||||
*
|
||||
* Numbering is decoration-only (footnote-numbering.ts derives numbers from
|
||||
* reference order), so the bottom list's PHYSICAL order need not match reference
|
||||
* order for the displayed numbers to be correct. We therefore assert: the
|
||||
* existing definition NODE INSTANCES are preserved (identity-equal) after the
|
||||
* sync pass, AND the derived numbers follow the new reference order.
|
||||
*/
|
||||
describe("footnote sync plugin (no rebuild on reorder — data-loss guard)", () => {
|
||||
function reorderedDoc() {
|
||||
// The "out of order" end-state of a reorder: references occur as [b, a] but
|
||||
// the bottom list still physically holds definitions in [a, b] order. This
|
||||
// is exactly the situation a reference reorder produces (decoration-only
|
||||
// numbering keeps the displayed numbers correct without physically moving
|
||||
// the definition subtrees). The sync plugin must leave the definitions
|
||||
// ALONE here — no delete/recreate of any definition subtree.
|
||||
return {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{ type: "text", text: "p" },
|
||||
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "b" } },
|
||||
{ type: "text", text: "q" },
|
||||
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "a" } },
|
||||
],
|
||||
},
|
||||
{
|
||||
type: FOOTNOTES_LIST_NAME,
|
||||
content: [
|
||||
{
|
||||
type: FOOTNOTE_DEFINITION_NAME,
|
||||
attrs: { id: "a" },
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "A" }] },
|
||||
],
|
||||
},
|
||||
{
|
||||
type: FOOTNOTE_DEFINITION_NAME,
|
||||
attrs: { id: "b" },
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "B" }] },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function getDefNodesById(doc: PMNode): Map<string, PMNode> {
|
||||
const m = new Map<string, PMNode>();
|
||||
doc.descendants((node) => {
|
||||
if (node.type.name === FOOTNOTE_DEFINITION_NAME) m.set(node.attrs.id, node);
|
||||
});
|
||||
return m;
|
||||
}
|
||||
|
||||
it("does NOT delete/recreate existing definition subtrees for an out-of-order list (numbers still correct)", () => {
|
||||
const editor = makeEditor(reorderedDoc());
|
||||
|
||||
// Capture the exact definition NODE INSTANCES before any sync pass.
|
||||
const before = getDefNodesById(editor.state.doc);
|
||||
// Sanity: both carry their content right now.
|
||||
expect(before.get("a")!.textContent).toBe("A");
|
||||
expect(before.get("b")!.textContent).toBe("B");
|
||||
|
||||
// Trigger a local edit elsewhere in the body so the sync plugin runs.
|
||||
editor.commands.insertContentAt(1, "z");
|
||||
|
||||
const doc = editor.state.doc;
|
||||
|
||||
// Reference order is [b, a]; the displayed numbers follow reference order
|
||||
// (decoration-only numbering): b -> 1, a -> 2 — regardless of physical list
|
||||
// order.
|
||||
expect(collectReferenceIds(doc)).toEqual(["b", "a"]);
|
||||
const numbers = computeFootnoteNumbers(doc);
|
||||
expect(numbers.get("b")).toBe(1);
|
||||
expect(numbers.get("a")).toBe(2);
|
||||
|
||||
// CRITICAL regression guard: both definitions still exist and are the SAME
|
||||
// node instances as before the edit — the plugin did NOT delete/recreate the
|
||||
// list (which would replace every definition's CRDT subtree and open the
|
||||
// concurrent-edit data-loss window). Identity equality proves the subtree
|
||||
// was preserved verbatim.
|
||||
const after = getDefNodesById(doc);
|
||||
expect(after.get("a")).toBe(before.get("a"));
|
||||
expect(after.get("b")).toBe(before.get("b"));
|
||||
// Content intact, exactly one list, both definitions present.
|
||||
expect(after.get("a")!.textContent).toBe("A");
|
||||
expect(after.get("b")!.textContent).toBe("B");
|
||||
expect(countType(doc, FOOTNOTES_LIST_NAME)).toBe(1);
|
||||
expect(countType(doc, FOOTNOTE_DEFINITION_NAME)).toBe(2);
|
||||
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Sync-plugin guard paths that are awkward to exercise through a live editor:
|
||||
* the remote-transaction skip and the enableSync:false (read-only) mode.
|
||||
*/
|
||||
describe("footnote sync plugin (guards)", () => {
|
||||
// Build a non-canonical document (an orphan reference with no definition) so a
|
||||
// sync pass would normally append a transaction.
|
||||
function nonCanonicalState() {
|
||||
const schema = getSchema(extensions);
|
||||
const doc = PMNode.fromJSON(schema, {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{ type: "text", text: "x" },
|
||||
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "orphan" } },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
return EditorState.create({ schema, doc });
|
||||
}
|
||||
|
||||
it("isRemoteTransaction => true: appendTransaction returns null (no rebuild on remote txns)", () => {
|
||||
// The sync plugin must SKIP remote/collab transactions so orphan cleanup and
|
||||
// structural rewrites only ever run on local edits.
|
||||
const plugin = footnoteSyncPlugin(() => true);
|
||||
const state = nonCanonicalState();
|
||||
|
||||
// Produce a doc-changing transaction (insert a space) and feed it to the
|
||||
// plugin's appendTransaction exactly as ProseMirror would.
|
||||
const tr = state.tr.insertText(" ", 1);
|
||||
const newState = state.apply(tr);
|
||||
const result = plugin.spec.appendTransaction!(
|
||||
[tr],
|
||||
state,
|
||||
newState,
|
||||
);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("isRemoteTransaction => false: appendTransaction DOES rebuild (sanity)", () => {
|
||||
// Control: with a local (non-remote) transaction the same non-canonical doc
|
||||
// triggers a sync transaction, proving the null above is the remote guard
|
||||
// and not a no-op everywhere.
|
||||
const plugin = footnoteSyncPlugin(() => false);
|
||||
const state = nonCanonicalState();
|
||||
const tr = state.tr.insertText(" ", 1);
|
||||
const newState = state.apply(tr);
|
||||
const result = plugin.spec.appendTransaction!([tr], state, newState);
|
||||
expect(result).not.toBeNull();
|
||||
expect(result!.docChanged).toBe(true);
|
||||
});
|
||||
|
||||
it("enableSync:false: the plugin never mutates the doc (read-only viewer)", () => {
|
||||
// Build an editor with sync disabled. An orphan reference (no definition)
|
||||
// must NOT trigger a definition insertion — the document is left untouched.
|
||||
const editor = new Editor({
|
||||
extensions: [
|
||||
Document,
|
||||
Paragraph,
|
||||
Text,
|
||||
FootnoteReference.configure({ enableSync: false }),
|
||||
FootnotesList,
|
||||
FootnoteDefinition,
|
||||
],
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{ type: "text", text: "x" },
|
||||
{ type: FOOTNOTE_REFERENCE_NAME, attrs: { id: "orphan" } },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
// A local edit that would normally trigger orphan-definition synthesis.
|
||||
editor.commands.insertContentAt(1, "y");
|
||||
|
||||
const doc = editor.state.doc;
|
||||
// No definition (and no list) was ever created — sync is disabled.
|
||||
expect(countType(doc, FOOTNOTE_DEFINITION_NAME)).toBe(0);
|
||||
expect(countType(doc, FOOTNOTES_LIST_NAME)).toBe(0);
|
||||
// Numbering decorations still work: the reference is numbered 1.
|
||||
expect(getFootnoteNumber(editor.state, "orphan")).toBe(1);
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Numbering cache (Fix 2). NodeViews must read footnote numbers from the
|
||||
* numbering plugin's cached map (updated once per doc change) rather than
|
||||
* recomputing the whole map per render. We assert the cache exists, is correct,
|
||||
* and stays current across edits.
|
||||
*/
|
||||
describe("footnote numbering cache", () => {
|
||||
it("exposes correct numbers via getFootnoteNumber and updates on edits", () => {
|
||||
const editor = makeEditor({
|
||||
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" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// The cache mirrors computeFootnoteNumbers — but is read in O(1) per id.
|
||||
expect(getFootnoteNumber(editor.state, "x")).toBe(1);
|
||||
expect(getFootnoteNumber(editor.state, "y")).toBe(2);
|
||||
// The cached map is the SAME values a fresh full computation would yield.
|
||||
const fresh = computeFootnoteNumbers(editor.state.doc);
|
||||
expect(getFootnoteNumber(editor.state, "x")).toBe(fresh.get("x"));
|
||||
expect(getFootnoteNumber(editor.state, "y")).toBe(fresh.get("y"));
|
||||
|
||||
// After inserting a new earlier reference, the cache updates so the numbers
|
||||
// shift (decoration-only numbering follows reference order).
|
||||
editor.commands.insertContentAt(1, {
|
||||
type: FOOTNOTE_REFERENCE_NAME,
|
||||
attrs: { id: "z" },
|
||||
});
|
||||
expect(getFootnoteNumber(editor.state, "z")).toBe(1);
|
||||
expect(getFootnoteNumber(editor.state, "x")).toBe(2);
|
||||
expect(getFootnoteNumber(editor.state, "y")).toBe(3);
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user