Skip to content

Add Yjs Collaborative Editing to RichEditor and Fix Editor Teardown Crash - #344

Merged
garrity-miepub merged 6 commits into
mieweb:mainfrom
Dharp02:feat/richeditor-collab-yjs
Aug 4, 2026
Merged

Add Yjs Collaborative Editing to RichEditor and Fix Editor Teardown Crash#344
garrity-miepub merged 6 commits into
mieweb:mainfrom
Dharp02:feat/richeditor-collab-yjs

Conversation

@Dharp02

@Dharp02 Dharp02 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Overview

Adds opt-in Yjs collaborative editing to RichEditor, and fixes a ProseMirror crash that takes the editor down permanently when it is unmounted while the pointer is over it.

The crash

@kerebron/extension-ui's autocomplete and hover extensions debounce their DOM handlers by 200ms and then call dispatchMeta:

// HoverPlugin.ts
this.onMouseLeave = debounce(this.onMouseLeave.bind(this), 200);

dispatchMeta(meta: HoverMeta) {
  const tr = this.editor.state.tr;        // no guard that the editor is alive
  this.editor.dispatchTransaction(tr);
}

HoverPlugin's destroy() hook tears down the renderer but never cancels those pending timers. So any teardown inside the debounce window lands the deferred call on a dead view and throws null.matchesNode() inside ProseMirror's EditorView.updateStateInner, leaving the view permanently broken.

Two ways to reach it:

  1. Remount while hovering — a key change destroys the view, and the debounced mouseleave fires ~200ms later against it. Hits every editor, collab or not.
  2. Yjs remote update — a remote change replaces the document tree under a pending callback. Collab only.

Changes

  1. RichEditor gains a collab prop — when set, the editor joins a Yjs room over the /yjs websocket relay and every peer co-edits one shared document. Uncontrolled like value; remount via key to switch rooms.

  2. Authenticated websocket provider — deliberately not using @kerebron/editor-kits' own YjsEditorKit, which constructs WebsocketProvider with no query params and so leaves no way to authenticate the socket. Caller-supplied params (e.g. an auth token) are threaded through so the backend can authorize the room.

  3. Both modes now drop autocomplete and hover — the first commit removed them for collab mode only; plain mode still built a raw AdvancedEditorKit and kept the crash. Both paths now go through createEditorKits().

  4. history stays collab-only — it is dropped there because ExtensionYjs supplies its own CRDT-aware undo/redo and the editor throws Extension conflict: yjs vs history. That does not apply to plain mode, which keeps working undo/redo.

  5. collabKits.tseditorKits.ts — the module assembles kits for both modes now, so the old name was misleading.

Trade-off

This removes features to dodge a dependency bug rather than fixing it. The real fix belongs in @kerebron/extension-ui, which already ships debounceWithCancel in the same utilities module — HoverPlugin just uses plain debounce and never cancels on destroy.

Removing them is cheap in the meantime: autocomplete popups and node-hover tooltips are compositor conveniences, not required for editing, and nothing currently registers a hover source. Worth reverting once the extensions guard their deferred dispatches.

Acceptance Criteria

  • RichEditor accepts a collab prop and joins the given Yjs room
  • Websocket provider forwards caller-supplied auth params
  • Plain (non-collab) editors no longer crash when unmounted with the pointer over them
  • Plain mode retains undo/redo; collab mode uses the Yjs CRDT history
  • npm run build and npm run typecheck pass

Out of Scope (for Now)

  • Fixing the debounce-after-destroy bug in @kerebron/extension-ui itself
  • Restoring autocomplete / hover under collab
  • Presence indicators / remote cursors

🤖 Generated with Claude Code

Dharp02 and others added 3 commits July 24, 2026 16:51
Add a `collab` prop to RichEditor that enables live co-editing over a
`/yjs` websocket relay. Because ExtensionYjs conflicts with the default
history extension, collaborative mode swaps AdvancedEditorKit for the same
extensions minus `history`, plus MarkYChange + ExtensionYjs. The websocket
provider threads caller-supplied query params (e.g. an auth token) so the
relay can authorize the room — unlike editor-kits' own YjsEditorKit.

Room is joined after the local markdown is loaded; the Yjs binding seeds an
empty shared doc from that content on first join and overwrites the editor
when the room already has edits, so stored markdown stays the source of truth.
Both extensions store stale ProseMirror node refs in debounced callbacks.
When a Yjs remote update replaces the document tree, their deferred
dispatchMeta calls crash with null.matchesNode() inside EditorView.updateStateInner,
leaving the view permanently broken and blocking further sync.

Removing them in collab mode has no functional cost — these are UI convenience
features, not required for collaborative text editing.
`autocomplete` and `hover` debounce their DOM handlers by 200ms and then
call `dispatchMeta`, which reads `this.editor.state` with no guard that the
editor is still alive. HoverPlugin's `destroy()` hook tears down the
renderer but never cancels those pending timers, so any unmount inside the
debounce window lands the deferred call on a dead view and throws
`null.matchesNode()` inside ProseMirror's `EditorView.updateStateInner`,
leaving the view permanently broken.

14b896d removed both extensions for collaborative mode, where a Yjs remote
update replacing the document tree triggers it. But the same crash fires
without Yjs: remounting the editor (a `key` change) while the pointer is
over it dispatches the debounced `mouseleave` after the view is destroyed.
Plain mode still built a raw `AdvancedEditorKit`, so it kept both.

Route both modes through `createEditorKits()` so the filter applies
everywhere. `history` stays collab-only — it is dropped there because
ExtensionYjs supplies its own CRDT-aware undo/redo and the editor throws
`Extension conflict: yjs vs history`, which does not apply to plain mode.

Renames `collabKits.ts` to `editorKits.ts` since it now assembles kits for
both modes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 20:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in Yjs collaborative mode to RichEditor and routes both plain + collab editor initialization through a shared kit builder that removes teardown-unsafe extensions to prevent a ProseMirror crash on unmount.

Changes:

  • Add collab?: CollabConfig to RichEditor and join a Yjs room after initial markdown load.
  • Introduce editorKits.ts to build “safe” editor kits for both modes (dropping autocomplete/hover, and swapping undo/redo behavior in collab).
  • Add Yjs-related peer dependencies and update Kerebron package versions in devDependencies.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/components/RichEditor/RichEditor.tsx Adds collab prop, uses shared kit assembly, and joins Yjs room after seeding initial content.
src/components/RichEditor/index.ts Re-exports CollabConfig from the RichEditor entrypoint.
src/components/RichEditor/editorKits.ts New kit assembly module for plain vs collab modes; configures Yjs provider and drops unsafe extensions.
package.json Adds Yjs-related peers (marked optional) and bumps Kerebron devDependency versions.
Comments suppressed due to low confidence (3)

src/components/RichEditor/RichEditor.tsx:101

  • After unmount, any pending async onTransaction() call will still see a non-null editorInstance.current and may set state. Clearing the ref during cleanup prevents post-unmount state updates and avoids invoking editor APIs after destroy().
    return () => {
      editor.removeEventListener('transaction', onTransaction);
      editor.destroy();
    };

package.json:274

  • y-protocols is imported unconditionally by editorKits.ts, so it’s required for any consumer that imports RichEditor, even in non-collab mode. Marking it as an optional peer hides missing-dependency warnings and can lead to runtime Cannot find module errors. Consider making this peer non-optional unless the collab kit is lazy-loaded.
    "y-protocols": {
      "optional": true
    },

package.json:277

  • yjs is imported unconditionally by editorKits.ts, so it’s required for any consumer that imports RichEditor, even in non-collab mode. Marking it as an optional peer hides missing-dependency warnings and can lead to runtime Cannot find module errors. Consider making this peer non-optional unless the collab kit is lazy-loaded.
    "yjs": {
      "optional": true
    },

Comment thread src/components/RichEditor/RichEditor.tsx Outdated
Comment thread package.json
Comment thread package.json Outdated
Comment thread src/components/RichEditor/RichEditor.tsx
… lazy-load yjs kit

- guard all async continuations with a disposed flag so nothing touches the
  editor after destroy(); clear editorInstance ref on cleanup
- move Yjs imports into collabKit.ts behind a dynamic import() so yjs,
  y-protocols and @kerebron/extension-yjs are truly optional peers
- bump @kerebron/* peer minimums to >=0.8.6 to match tested devDeps
- add collab-path tests: kit construction with url/params, changeRoom,
  plain mode never loads the kit, unmount-before-load never joins
- add Collaborative story with an in-page loopback websocket relay
  (new WebSocketPolyfill option on CollabConfig)
Copilot AI review requested due to automatic review settings August 3, 2026 22:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (5)

src/components/RichEditor/RichEditor.test.tsx:75

  • This test currently asserts collab isn't loaded, but it doesn't verify the key acceptance criteria for plain mode: autocomplete/hover are removed (crash avoidance) while history is retained (undo/redo). Add an assertion on the SafeAdvancedEditorKit's filtered extensions.
  it('plain mode does not load the yjs collab kit', async () => {
    renderWithTheme(<RichEditor />);
    await waitFor(() => expect(coreEditorCreate).toHaveBeenCalled());
    expect(huddleYjsKit).not.toHaveBeenCalled();
    expect(changeRoom).not.toHaveBeenCalled();
  });

src/components/RichEditor/RichEditor.test.tsx:102

  • The collab-mode test verifies the Yjs kit is present, but it doesn't assert the collab-specific part of the filtering logic: history should be dropped to avoid the yjs vs history extension conflict. Add an assertion on the filtered extensions from the advanced kit instance.
    const kits = (
      coreEditorCreate.mock.calls[0][0] as {
        editorKits: { name: string }[];
      }
    ).editorKits;
    expect(kits.map((k) => k.name)).toEqual(['advanced-editor', 'yjs-editor']);
  });

src/components/RichEditor/RichEditor.tsx:93

  • joinRoom assumes editor.run exists; the optional chaining only applies to changeRoom, so if run is missing/undefined this will throw during mount in collab mode. Add optional chaining (or a runtime guard) on run itself before dereferencing changeRoom.
        if (collab && editor && !disposed) {
          (
            editor.run as Record<string, (...args: unknown[]) => boolean>
          ).changeRoom?.(collab.room);
        }

src/components/RichEditor/RichEditor.test.tsx:28

  • The current AdvancedEditorKit mock returns no extensions, so tests can't verify that SafeAdvancedEditorKit actually drops the teardown-unsafe extensions (and drops history only in collab mode). Mock a representative extension list so the new filtering logic is exercised.

This issue also appears in the following locations of the same file:

  • line 70
  • line 96
vi.mock('@kerebron/editor-kits/AdvancedEditorKit', () => ({
  AdvancedEditorKit: vi.fn(() => ({ getExtensions: () => [] })),
}));

src/components/RichEditor/RichEditor.stories.tsx:131

  • LoopbackWebSocket stores every sent frame forever in room.history, so leaving this story open while typing can grow memory without bound. Since this is a demo-only relay, consider bounding the history buffer (or periodically compacting it) to keep Storybook sessions stable.
  send(data: Uint8Array) {
    const frame = data.slice().buffer as ArrayBuffer;
    this.room.history.push(frame);
    for (const socket of this.room.sockets) {

…ures

The kerebron 0.7.9 -> 0.8.x bump exposed two upstream bugs in the static
storybook build CI runs against:

- CodeCrock.getSelection() invokes the native getSelection through dnt's
  globalThis merge-proxy, throwing "Illegal invocation" when the editor is
  detached during nodeview init — crashed the Code story render
- the code-block language <select> has no accessible name (axe select-name)

Patched via pnpm patchedDependencies; drop when fixed upstream in kerebron.
Copilot AI review requested due to automatic review settings August 3, 2026 23:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (1)

src/components/RichEditor/RichEditor.test.tsx:76

  • The crash fix depends on filtering out the teardown-unsafe hover/autocomplete extensions (and removing history only in collab mode), but the current tests only assert kit names / room join. Adding a small unit test that feeds a fake extension list through the mocked AdvancedEditorKit.getExtensions() and asserts the filtered output would protect this regression-prone behavior.
  it('plain mode does not load the yjs collab kit', async () => {

@garrity-miepub

Copy link
Copy Markdown
Collaborator

Heads up on the a11y CI failure (Code › smoke-test: Illegal invocation) — root cause was the kerebron 0.7.9 → 0.8.x bump in this PR, not the collab work itself. Fixed in 817cfba.

Two upstream bugs in @kerebron/extension-codecrock 0.8.x, both only visible against the static Storybook build that CI tests (dev server passes fine):

  1. CodeCrock.getSelection() crash — its fallback calls the native getSelection() with dnt's globalThis merge-proxy as this, which throws TypeError: Illegal invocation. The fallback path is only reached when the editor element is detached during ProseMirror nodeview init, which is production-build timing — hence CI-only.
  2. axe select-name (critical) — once the render crash was fixed, the code block's new language <select class="codecrock-select"> was flagged for having no accessible name.

Both are patched via pnpm patchedDependencies (patches/@kerebron__extension-codecrock@0.8.9.patch): the selection fallback now goes through the real document, and the select gets aria-label="Code language".

Verified locally against a static build served the same way CI does: full storybook a11y suite 1424/1424 passing.

Note: the patch only covers this repo's install — consumers of @mieweb/ui get their own unpatched codecrock, so the real fix belongs upstream in kerebron (same family as the debounce-after-destroy issue already described in the PR body). The patch is pinned to 0.8.9 and should be dropped once kerebron ships a fix.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Suppressed comments (3)

src/components/RichEditor/RichEditor.test.tsx:118

  • await Promise.resolve() only flushes a single microtask turn. Since the setup path awaits both loadDocumentText() and then saveDocument() inside onTransaction(), joinRoom() could run after this assertion (making the test a false-positive). Use a macrotask (or multiple microtask flushes) to ensure all continuations have had a chance to run before asserting.
    resolveLoad();
    await Promise.resolve(); // flush the continuation
    expect(changeRoom).not.toHaveBeenCalled();

src/components/RichEditor/RichEditor.tsx:92

  • The changeRoom cast claims the command returns boolean, but the return value is ignored and may not be a boolean. Using an unknown/specific signature avoids baking in an incorrect API contract and improves type safety around editor.run.
        if (collab && editor && !disposed) {
          (
            editor.run as Record<string, (...args: unknown[]) => boolean>
          ).changeRoom?.(collab.room);
        }

src/components/RichEditor/RichEditor.tsx:49

  • onTransaction can still call setMd() / onChange after the component unmounts: the editorInstance.current guard is checked only before await saveDocument, so unmounting during the await will still run the continuation. Adding a post-await disposed/stale-editor check prevents state updates after teardown.
    const onTransaction = async () => {

@garrity-miepub
garrity-miepub merged commit ec4bbe0 into mieweb:main Aug 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants