diff --git a/packages/prosemirror-markdown/test/schema-editor-ext-contract.test.ts b/packages/prosemirror-markdown/test/schema-editor-ext-contract.test.ts index 4778ea5c..7570ff11 100644 --- a/packages/prosemirror-markdown/test/schema-editor-ext-contract.test.ts +++ b/packages/prosemirror-markdown/test/schema-editor-ext-contract.test.ts @@ -16,14 +16,16 @@ import * as editorExt from "@docmost/editor-ext"; // or mark added upstream that the mirror forgets to vendor fails CI loudly // (otherwise it is silently dropped on the markdown <-> ProseMirror round-trip). // -// LIMITATION (intentional, see schema-surface-snapshot.test.ts): this is a -// NAME-LEVEL contract only, not a full attribute-level structural compare. -// editor-ext's Tiptap representation (node views, commands, suggestion plugins, -// addGlobalAttributes spread across separate extensions) differs from this -// minimal mirror, so a mechanical attribute-by-attribute equality would be -// fragile and produce false drift. Attribute parity is guarded by the inline -// surface snapshot (reviewed in every diff); this test guards that no canonical -// node/mark TYPE goes unmirrored. StarterKit-provided types (paragraph, bold, +// This file now holds TWO contracts (see the two describe blocks): the original +// NAME-LEVEL type contract (no canonical node/mark TYPE goes unmirrored) AND, as +// of #493, an ATTRIBUTE-LEVEL contract that compares each editor-ext node/mark's +// OWN declared attributes (names + defaults) against the mirror's built schema. +// A full mechanical attribute-by-attribute EQUALITY would be fragile (the mirror +// is a deliberate superset: it injects the global id/textAlign/indent attrs and +// normalizes some editor-ext defaults to null), so the attribute contract is +// asymmetric — editor-ext -> mirror — with a small, reasoned, stale-guarded +// allowlist for the two blessed divergence kinds (non-round-trippable omissions +// and null-normalized defaults). StarterKit-provided types (paragraph, bold, // heading, …) are contributed by @tiptap/starter-kit in the mirror rather than // by editor-ext, so they are naturally covered by the mirror's superset. // @@ -85,3 +87,191 @@ describe("docmost schema vs @docmost/editor-ext (name-level contract)", () => { expect(missing).toEqual([]); }); }); + +// ── ATTRIBUTE-LEVEL CONTRACT (#493 commit 2) ──────────────────────────────── +// +// The name-level contract above catches a WHOLE node/mark type going unmirrored, +// but not ATTRIBUTE drift within a vendored type — the exact class that silently +// dropped `subpages.recursive`: editor-ext grew an attribute the hand-synced +// mirror forgot, so documents using it lost that attribute on a git-sync +// round-trip while CI stayed green. This closes that gap by comparing each +// editor-ext node/mark's OWN declared attributes (names + defaults) against the +// mirror's built ProseMirror schema `spec.attrs`. +// +// DIRECTION: editor-ext -> mirror. The mirror is deliberately a SUPERSET (it +// injects the global `id`/`textAlign`/`indent` attributes and normalizes some +// editor-ext "required" attrs to a `null` default), so a reverse compare would +// be pure false drift; the meaningful failure is an editor-ext attribute the +// mirror DROPS (name) or whose DEFAULT it silently changes. Both directions of +// staleness are guarded so the allowlists cannot rot. + +/** + * The attributes an editor-ext Tiptap Node/Mark DECLARES itself, read from its + * `config.addAttributes()`. Global attributes injected by separate extensions + * (unique-id, indent, textAlign) are NOT included here — they are the mirror's + * superset and are not part of a per-type declaration — so this isolates each + * type's own contribution. A declared attribute with no explicit `default` is a + * required attr (Tiptap default `undefined`); we surface that as-is so the + * default compare can skip it (the mirror makes such attrs optional/`null`). + */ +function editorExtOwnAttrs(): Map< + string, + { kind: "node" | "mark"; attrs: Record } +> { + const out = new Map< + string, + { kind: "node" | "mark"; attrs: Record } + >(); + for (const value of Object.values(editorExt)) { + if (!isTiptapNodeOrMark(value)) continue; + const ext = value as unknown as { + name: string; + type: "node" | "mark"; + options?: unknown; + storage?: unknown; + config?: { addAttributes?: () => Record }; + }; + const fn = ext.config?.addAttributes; + // addAttributes reads `this.options`/`this.name`; bind a minimal context + // (verified sufficient for every editor-ext extension — none reach for + // `this.editor` here). A type with no addAttributes contributes no attrs. + const declared = + typeof fn === "function" + ? fn.call({ + options: ext.options ?? {}, + name: ext.name, + parent: undefined, + storage: ext.storage ?? {}, + } as never) + : {}; + const attrs: Record = {}; + for (const [attr, spec] of Object.entries(declared || {})) { + // `undefined` marks a required (no-default) attr; keep it so the default + // compare can distinguish "no default declared" from "default is null". + attrs[attr] = (spec as { default?: unknown })?.default; + } + out.set(ext.name, { kind: ext.type, attrs }); + } + return out; +} + +/** The mirror's built-schema `spec.attrs` for a type: attr name -> default. */ +function mirrorAttrs( + name: string, + kind: "node" | "mark", +): Record | null { + const schema = getSchema(docmostExtensions as never); + const spec = kind === "node" ? schema.nodes[name]?.spec : schema.marks[name]?.spec; + if (!spec) return null; + const out: Record = {}; + for (const [attr, def] of Object.entries(spec.attrs || {})) { + out[attr] = (def as { default?: unknown }).default; + } + return out; +} + +// An editor-ext attribute the mirror deliberately does NOT vendor because it has +// NO markdown round-trip representation — dropping it loses nothing on the +// git-sync cycle (the same rationale the flat-roundtrip property suite uses to +// allowlist e.g. `tableCell.backgroundColorName`). Blessed by the hand-curated +// surface snapshot (schema-surface-snapshot.test.ts), reviewed in every diff. +const ACCEPTED_ATTR_OMISSIONS = new Set([ + "highlight.colorName", // only `highlight.color` round-trips (==text==); the + // secondary palette-name is presentational and has no markdown form. +]); + +// An editor-ext attribute the mirror vendors but with a DIFFERENT default: the +// mirror normalizes an "absent" value to `null` (its uniform optional-attr +// convention) rather than editor-ext's UI-oriented default. None of these attrs +// is emitted on the markdown surface (the converter round-trips only the +// serializable ones), so the default never round-trips and the divergence is +// inert — but pinned here so a NEW default change on either side forces review. +const ACCEPTED_DEFAULT_DIVERGENCE = new Set([ + "image.src", // mirror null vs editor "" (an image is never emitted src-less) + "link.internal", // mirror null vs editor false (routing attr, not in md link) + "pdf.width", // mirror null vs editor 800 (presentational sizing, not in md) + "pdf.height", // mirror null vs editor 600 (presentational sizing, not in md) +]); + +describe("docmost schema vs @docmost/editor-ext (attribute-level contract)", () => { + it("vendors every editor-ext attribute (name) of every shared type — no silently-dropped attrs", () => { + const dropped: string[] = []; + for (const [name, { kind, attrs }] of editorExtOwnAttrs()) { + const mirror = mirrorAttrs(name, kind); + if (!mirror) continue; // whole-type omission is the name-level test's job + for (const attr of Object.keys(attrs)) { + const key = `${name}.${attr}`; + if (!(attr in mirror) && !ACCEPTED_ATTR_OMISSIONS.has(key)) { + dropped.push(key); + } + } + } + // Any entry here exists on the editor-ext node/mark but NOT in the mirror + // (and is not a blessed non-round-trippable omission): documents using it + // lose that attribute on a git-sync round-trip — the subpages.recursive + // class. Re-sync src/lib/docmost-schema.ts (and the surface snapshot) or add + // a reasoned ACCEPTED_ATTR_OMISSIONS entry before clearing. + expect(dropped.sort()).toEqual([]); + }); + + it("keeps every editor-ext attribute DEFAULT in sync — no silent default drift", () => { + const drift: string[] = []; + for (const [name, { kind, attrs }] of editorExtOwnAttrs()) { + const mirror = mirrorAttrs(name, kind); + if (!mirror) continue; + for (const [attr, extDefault] of Object.entries(attrs)) { + const key = `${name}.${attr}`; + // Skip attrs editor-ext declares WITHOUT a default (required attrs): + // the mirror deliberately makes them optional (`null`), a safe superset. + if (extDefault === undefined) continue; + if (!(attr in mirror)) continue; // a drop, reported by the name test + if ( + JSON.stringify(mirror[attr]) !== JSON.stringify(extDefault) && + !ACCEPTED_DEFAULT_DIVERGENCE.has(key) + ) { + drift.push( + `${key}: mirror=${JSON.stringify(mirror[attr])} editor-ext=${JSON.stringify(extDefault)}`, + ); + } + } + } + expect(drift.sort()).toEqual([]); + }); + + it("the attribute allowlists have no stale rows (each is really omitted / divergent)", () => { + const ext = editorExtOwnAttrs(); + const staleOmission: string[] = []; + for (const key of ACCEPTED_ATTR_OMISSIONS) { + const [name, attr] = key.split("."); + const entry = ext.get(name); + const mirror = entry ? mirrorAttrs(name, entry.kind) : null; + // Stale if editor-ext no longer declares it, or the mirror now DOES vendor + // it (so it should be removed from the omission allowlist). + if (!entry || !(attr in entry.attrs) || (mirror && attr in mirror)) { + staleOmission.push(key); + } + } + expect(staleOmission, "stale ACCEPTED_ATTR_OMISSIONS rows").toEqual([]); + + const staleDivergence: string[] = []; + for (const key of ACCEPTED_DEFAULT_DIVERGENCE) { + const [name, attr] = key.split("."); + const entry = ext.get(name); + const mirror = entry ? mirrorAttrs(name, entry.kind) : null; + const extDefault = entry?.attrs[attr]; + // Stale if the divergence no longer exists (attr gone, or defaults now + // agree) — the row should be dropped so the allowlist stays honest. + if ( + !entry || + !mirror || + !(attr in entry.attrs) || + !(attr in mirror) || + extDefault === undefined || + JSON.stringify(mirror[attr]) === JSON.stringify(extDefault) + ) { + staleDivergence.push(key); + } + } + expect(staleDivergence, "stale ACCEPTED_DEFAULT_DIVERGENCE rows").toEqual([]); + }); +});