chore: groundwork before the Tiptap 3 migration - #15
Merged
Conversation
The exact prosemirror-* pins were introduced in a5c7042, whose message blames prosemirror-view 1.42 for a history-restore failure: "restoring a snapshot merged into the live document instead of replacing it". ccf5dbd retracts that the next day. The real defect was server-side -- ReplaceWithMarkdown clears the fragment and applies the restore in one transaction, but persisted and broadcast only the caller's update_bytes, which encodes the insertion alone. Peers got the insert without the delete and appended. Its message says so directly: "This is the real cause of the flaky e2e history.spec failure." The comment in pnpm-workspace.yaml and the matching Renovate rule were never updated, so both still describe prosemirror-view 1.42 as quarantined. That is load-bearing misinformation for anyone costing the Tiptap 3 upgrade, which needs >=1.42.3 -- it makes a routine version bump look like a known-bad one. Dedupe remains the real and sufficient reason for the pins, so the versions do not change here. Only the explanation does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mergeAttributes() copied an own `__proto__` key with plain assignment, which promotes it to the merged object's prototype -- so a crafted attribute set could inject inherited executable DOM attributes into every node and mark that renders through it. 2.27.3 defines the key as a data property instead. knot was not reachable. ProseMirror's computeAttrs builds node.attrs into Object.create(null) and iterates only schema-declared attribute names, so no attacker-controlled `__proto__` survives to the merge -- verified by constructing an attachment node from a hostile JSON payload and reading back own keys [url, name, size, contentType] with a null prototype and a clean for-in. The CRDT import path funnels through the same function. Taking it anyway because it costs nothing: one file, twelve lines, unchanged peers, and it removes a standing advisory hit from the dependency review. Note for whoever sees the scanner still red: the advisory's fixed-version range reads ">= 3.30.4" and predates this backport, published 2026-09-04 on the v2-latest tag. The fix is real; the range is stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Baseline for the Tiptap 3 migration. Three of that upgrade's failure
modes compile clean, pass every existing test, and corrupt shared state
at runtime; none of them is observable today. These land on 2.x first so
they are a real before/after, not a story told after the fact.
New fixture: src/test/boundEditor.ts mounts a real Editor over a real
Y.Doc using the shipped createExtensions(). jsdom is enough -- no layout
is involved -- and a mount costs ~40ms, so the binding is available to
ordinary unit tests. The prior assumption that this needed Playwright is
what left the gaps below.
anchor.test.ts -- tested the y-prosemirror primitives against a
hand-built Map, and the null branches against a fake `{ state: {} }`
editor. Neither executes getMapping(). Adds a layer that resolves
anchors through the live ySync binding, including that a range still
covers the same TEXT after a peer inserts ahead of it. Verified by
pointing anchor.ts at a second `new PluginKey("y-sync")` -- exactly what
Tiptap 3 hands us: the four new assertions fail, all seven old ones stay
green.
schema.test.ts -- compared node and mark NAMES only, so it could not see
an extension that registers neither. That is the shape of every
StarterKit default that writes to the document. Adds an exact
registered-extension list, and an attribute-parity check in both
directions against tools/schema.json. Verified against stand-ins for
StarterKit v3's trailingNode/listKeymap and an Image carrying
width/height: both new tests fail, the three name-level ones do not.
ydoc.test.ts -- new. Asserts the editor writes only the user's edits into
the fragment. Fixtures deliberately end in a code block and a list: a
document ending in a paragraph cannot exhibit the failure, because a
tidying extension has nothing to append. Verified against a real
appendTransaction stand-in.
Two pre-existing drifts are recorded rather than fixed, so the parity
check stays honest instead of being loosened: link declares a `title`
attribute that no editor affordance produces, and table cells carry
`colwidth` that to_markdown has no concept of.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found while building the Tiptap 3 baseline coverage. All four predate that
work; each was reproduced before being fixed, and each fix was verified by
reverting it and watching the new test go red.
List keyboard shortcuts threw. Renaming a node does not rewrite the options
its own commands read: BulletList and OrderedList resolve
`options.itemTypeName`, still the camelCase "listItem", so
`toggleBulletList()` raised "There is no node type named 'listItem'" and
Mod-Shift-8 / Mod-Shift-7 did nothing but log. The toolbar buttons pass both
names explicitly, which is why only the shortcuts were affected. ListItem
declares bulletListTypeName/orderedListTypeName too, but nothing in v2 reads
them -- its keymaps pass `this.name` -- so they are deliberately left alone
rather than configured to no effect.
Link titles were dropped. tools/schema.json declares `title` on the link
mark and both from_markdown and to_markdown honour it, but Tiptap's Link has
no such attribute and ProseMirror discards what its schema does not declare.
So `[text](url "title")` survived import and was rewritten without the title
the first time anyone edited the document -- in storage, for every reader.
The parity check added in the previous commit now catches this class outright,
so UNIMPLEMENTED_ATTRS is empty and should stay that way.
Peer colours were an unsupported format. y-prosemirror accepts only
`#rrggbb` for a caret; colorFor() emitted `hsl()`, so every collaborative
session logged "A user uses an unsupported color format" once per peer. v2
still renders the caret, so this was noise rather than breakage -- but the
successor extension replaces a non-matching colour with `transparent`, which
would have made every remote caret invisible. Same palette, converted rather
than re-picked: no avatar changes colour.
Three CSS selectors matched nothing. A React node view's outer element is
Tiptap's own `div.react-renderer`, and the NodeViewWrapper carrying our
attributes renders inside it -- so `.ProseMirror > pre` and
`.ProseMirror > [data-testid=…]` never selected anything. The failure was
invisible because `.ProseMirror > *` still applied: code blocks, Mermaid
diagrams and Excalidraw boards were held at the 712px prose measure in wide
mode instead of taking their own width. Retargeted at the `node-<type>` class
Tiptap stamps on the wrapper. Regression covered in doc-width.spec.ts by
measuring rendered widths, which fails against the old selectors.
Also dead: a `.collaboration-cursor__selection` rule. y-prosemirror emits
`.ProseMirror-yjs-selection` with the peer's colour set inline, so the rule
neither matched nor could have overridden anything. Removed.
Not fixed here, deliberately: to_markdown's `_ => {}` arm silently discards
an unknown mark. tracing is not a dependency of knot-markdown, and the
extension-set and parity checks now prevent an unknown mark being written at
all, which is a stronger guarantee than a log line.
18 e2e specs across doc-width, mermaid, excalidraw, editor-toolbar,
two-users-converge, import-markdown, import-export and collab pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prerequisites for the Tiptap 3 migration, landing separately so the migration PR is only the migration.
docs(deps): correct the ProseMirror pin rationaleweb/pnpm-workspace.yamland the matching Renovate rule both describeprosemirror-view1.42 as having broken history restore.ccf5dbdretracts that — the real defect wasReplaceWithMarkdownbroadcasting only its insertion, so peers appended instead of replacing. The comments were never updated.This matters now: Tiptap 3 requires
prosemirror-view >= 1.42.3, so the stale comment makes a routine bump look like a known-bad one. No version changes here — only the explanation.fix(deps): patch GHSA-cp6q-959q-f8rh via@tiptap/core2.27.3mergeAttributes()copied an own__proto__key by assignment, promoting it to the merged object's prototype. 2.27.3 defines it as a data property instead.knot was not reachable — ProseMirror's
computeAttrsbuildsnode.attrsintoObject.create(null)over schema-declared names only. Taken anyway because it costs nothing: one file, twelve lines, unchanged peers.The advisory's range still reads
>= 3.30.4and predates this backport (published 2026-09-04 onv2-latest), so scanners may stay red. The fix is real; the range is stale.test(editor): cover what only breaks silentlyThree of the migration's failure modes compile clean, pass every existing test, and corrupt shared state at runtime. These land on 2.x first so they are a real before/after baseline.
New fixture
src/test/boundEditor.tsmounts a realEditorover a realY.Docusing the shippedcreateExtensions(). jsdom is enough and a mount costs ~40ms — the assumption that this needed Playwright is what left the gaps.anchor.test.tsgetMapping()never executed — hand-builtMap+ fake editor onlyanchor.tsat a secondnew PluginKey("y-sync"): 4 new fail, 7 old stay greenschema.test.tstrailingNode/listKeymapand an Image withwidth/height: 2 new fail, 3 old don'tydoc.test.ts(new)appendTransactionstand-inEvery new test was watched failing for the right reason before being kept.
Two pre-existing drifts are recorded, not fixed, so the parity check stays honest:
linkdeclares atitleattribute no editor affordance produces, and table cells carrycolwidththatto_markdownhas no concept of.tsc,eslint --max-warnings 0and 138 unit tests green.🤖 Generated with Claude Code