Add Yjs Collaborative Editing to RichEditor and Fix Editor Teardown Crash - #344
Conversation
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>
There was a problem hiding this comment.
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?: CollabConfigtoRichEditorand join a Yjs room after initial markdown load. - Introduce
editorKits.tsto build “safe” editor kits for both modes (droppingautocomplete/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-nulleditorInstance.currentand may set state. Clearing the ref during cleanup prevents post-unmount state updates and avoids invoking editor APIs afterdestroy().
return () => {
editor.removeEventListener('transaction', onTransaction);
editor.destroy();
};
package.json:274
y-protocolsis imported unconditionally byeditorKits.ts, so it’s required for any consumer that importsRichEditor, even in non-collab mode. Marking it as an optional peer hides missing-dependency warnings and can lead to runtimeCannot find moduleerrors. Consider making this peer non-optional unless the collab kit is lazy-loaded.
"y-protocols": {
"optional": true
},
package.json:277
yjsis imported unconditionally byeditorKits.ts, so it’s required for any consumer that importsRichEditor, even in non-collab mode. Marking it as an optional peer hides missing-dependency warnings and can lead to runtimeCannot find moduleerrors. Consider making this peer non-optional unless the collab kit is lazy-loaded.
"yjs": {
"optional": true
},
…lab-yjs # Conflicts: # package.json
… 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)
There was a problem hiding this comment.
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/hoverare removed (crash avoidance) whilehistoryis 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:
historyshould be dropped to avoid theyjs vs historyextension 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
joinRoomassumeseditor.runexists; the optional chaining only applies tochangeRoom, so ifrunis missing/undefined this will throw during mount in collab mode. Add optional chaining (or a runtime guard) onrunitself before dereferencingchangeRoom.
if (collab && editor && !disposed) {
(
editor.run as Record<string, (...args: unknown[]) => boolean>
).changeRoom?.(collab.room);
}
src/components/RichEditor/RichEditor.test.tsx:28
- The current
AdvancedEditorKitmock returns no extensions, so tests can't verify thatSafeAdvancedEditorKitactually drops the teardown-unsafe extensions (and dropshistoryonly 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
LoopbackWebSocketstores every sent frame forever inroom.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.
There was a problem hiding this comment.
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/autocompleteextensions (and removinghistoryonly 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 mockedAdvancedEditorKit.getExtensions()and asserts the filtered output would protect this regression-prone behavior.
it('plain mode does not load the yjs collab kit', async () => {
|
Heads up on the a11y CI failure ( Two upstream bugs in
Both are patched via pnpm 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 |
There was a problem hiding this comment.
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 bothloadDocumentText()and thensaveDocument()insideonTransaction(),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
changeRoomcast claims the command returnsboolean, but the return value is ignored and may not be a boolean. Using anunknown/specific signature avoids baking in an incorrect API contract and improves type safety aroundeditor.run.
if (collab && editor && !disposed) {
(
editor.run as Record<string, (...args: unknown[]) => boolean>
).changeRoom?.(collab.room);
}
src/components/RichEditor/RichEditor.tsx:49
onTransactioncan still callsetMd()/onChangeafter the component unmounts: theeditorInstance.currentguard is checked only beforeawait saveDocument, so unmounting during the await will still run the continuation. Adding a post-awaitdisposed/stale-editor check prevents state updates after teardown.
const onTransaction = async () => {
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'sautocompleteandhoverextensions debounce their DOM handlers by 200ms and then calldispatchMeta:HoverPlugin'sdestroy()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 throwsnull.matchesNode()inside ProseMirror'sEditorView.updateStateInner, leaving the view permanently broken.Two ways to reach it:
keychange destroys the view, and the debouncedmouseleavefires ~200ms later against it. Hits every editor, collab or not.Changes
RichEditorgains acollabprop — when set, the editor joins a Yjs room over the/yjswebsocket relay and every peer co-edits one shared document. Uncontrolled likevalue; remount viakeyto switch rooms.Authenticated websocket provider — deliberately not using
@kerebron/editor-kits' ownYjsEditorKit, which constructsWebsocketProviderwith no query params and so leaves no way to authenticate the socket. Caller-suppliedparams(e.g. an auth token) are threaded through so the backend can authorize the room.Both modes now drop
autocompleteandhover— the first commit removed them for collab mode only; plain mode still built a rawAdvancedEditorKitand kept the crash. Both paths now go throughcreateEditorKits().historystays collab-only — it is dropped there becauseExtensionYjssupplies its own CRDT-aware undo/redo and the editor throwsExtension conflict: yjs vs history. That does not apply to plain mode, which keeps working undo/redo.collabKits.ts→editorKits.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 shipsdebounceWithCancelin the same utilities module —HoverPluginjust uses plaindebounceand 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
RichEditoraccepts acollabprop and joins the given Yjs roomnpm run buildandnpm run typecheckpassOut of Scope (for Now)
@kerebron/extension-uiitselfautocomplete/hoverunder collab🤖 Generated with Claude Code