mirror of
https://github.com/opf/openproject.git
synced 2026-06-13 19:20:00 +00:00
Fix CTRL+Z in documents
https://community.openproject.org/wp/STC-779 Three independent defects together caused Y.UndoManager state to be lost in the documents module: 1. `useCreateBlockNote(editorParams, [activeUser])` used `[activeUser]` as the recreation key. BlockNote's `useCreateBlockNote(options, deps)` uses `deps` as the sole `useMemo` key (options is intentionally NOT in deps). Since `block-note-element.ts` parses a fresh `activeUser` from the host attribute via `JSON.parse` on every `renderCallback` invocation, any path that re-rendered the React tree handed in a new object reference and rebuilt the editor (and its UndoManager). 2. `LiveCollaborationManager.initializeYjsProvider` was not idempotent. Stimulus's `init-yjs-provider` controller can fire `connect()` a second time without firing `disconnect()` (HMR replay, Turbo morph, parent re-attach). The duplicate call destroyed the live Y.Doc + provider and rebuilt both, remounting the editor and wiping its history. The controller now adopts the existing session via `getCurrentSessionFor(documentName)` instead of constructing a duplicate. 3. The `React.StrictMode` wrap in `block-note-element.ts` caused BlockNoteView to destroy and recreate the ProseMirror view between StrictMode's two dev-mode mounts. `y-prosemirror`'s `yUndoPlugin` destroys the `Y.UndoManager` on view-destroy (removing its `afterTransaction` handler from the Y.Doc), but the plugin state retains the now-dead UndoManager reference. After the second mount the editor reused the destroyed UndoManager, no handler was re-attached, no stack items were recorded, and Ctrl+Z was a no-op. StrictMode is dev-only and incompatible with the current y-prosemirror lifecycle, so it is removed from the BlockNote tree. Verified on release/17.5: typing produces stack items, Ctrl+Z reverts to the previous state, Ctrl+Shift+Z reapplies, and the Y.Doc has exactly one `afterTransaction` observer (the live UndoManager's). Synthetic duplicate `connect()` no longer remounts the editor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -88,9 +88,14 @@ class BlockNoteElement extends HTMLElement {
|
||||
this.reactRoot = createRoot(this.editorMount);
|
||||
|
||||
this.renderCallback = (provider:HocuspocusProvider) => {
|
||||
this.reactRoot?.render(
|
||||
React.createElement(React.StrictMode, null, this.BlockNoteReactContainer(provider))
|
||||
);
|
||||
// Do NOT wrap in React.StrictMode. StrictMode's dev-mode double-mount causes
|
||||
// BlockNoteView to destroy and recreate the ProseMirror view between the two mounts.
|
||||
// y-prosemirror's `yUndoPlugin` destroys the Y.UndoManager on view-destroy (removing
|
||||
// its `afterTransaction` handler from the Y.Doc), but the plugin's STATE retains the
|
||||
// now-destroyed UndoManager reference. On the second mount the editor reuses the
|
||||
// destroyed UndoManager, no `afterTransaction` handler is ever re-attached, no stack
|
||||
// items are recorded, and Ctrl+Z becomes a no-op.
|
||||
this.reactRoot?.render(this.BlockNoteReactContainer(provider));
|
||||
};
|
||||
|
||||
LiveCollaborationManager.onReady(this.renderCallback);
|
||||
|
||||
@@ -122,7 +122,11 @@ export function OpBlockNoteEditor({
|
||||
};
|
||||
}, [hocuspocusProvider, doc, activeUser, localeDictionary, attachmentsEnabled, uploadFile, captureExternalLinks]);
|
||||
|
||||
const editor = useCreateBlockNote(editorParams, [activeUser]);
|
||||
// Create the editor exactly once per mount. `useCreateBlockNote(options, deps)` uses `deps`
|
||||
// as the sole `useMemo` key — `options` is intentionally NOT in deps. `[activeUser]` rebuilt
|
||||
// the editor (wiping `Y.UndoManager` history) whenever a fresh `activeUser` reference
|
||||
// reached this component, e.g. on Stimulus reconnect / Turbo morph.
|
||||
const editor = useCreateBlockNote(editorParams, []);
|
||||
useInlineWpEvents(editor);
|
||||
type EditorType = typeof editor;
|
||||
const theme = useOpTheme();
|
||||
|
||||
+12
-1
@@ -74,6 +74,17 @@ export default class extends Controller {
|
||||
connect():void {
|
||||
this.currentToken = this.tokenPayloadValue;
|
||||
|
||||
// If a provider for this document is already live, don't build a duplicate
|
||||
// — adopt it. Stimulus can fire connect() a second time (HMR replay, Turbo
|
||||
// morph, parent re-attach) without firing disconnect(); building a fresh
|
||||
// Y.Doc + provider in that case would destroy the live one and wipe the
|
||||
// editor's Y.UndoManager mid-session.
|
||||
const existing = LiveCollaborationManager.getCurrentSessionFor(this.documentNameValue);
|
||||
if (existing) {
|
||||
this.ownedProvider = existing.provider;
|
||||
return;
|
||||
}
|
||||
|
||||
const ydoc:Doc = new Y.Doc();
|
||||
const provider = new HocuspocusProvider({
|
||||
url: this.hocuspocusUrlValue,
|
||||
@@ -87,7 +98,7 @@ export default class extends Controller {
|
||||
},
|
||||
});
|
||||
|
||||
LiveCollaborationManager.initializeYjsProvider(provider, ydoc);
|
||||
LiveCollaborationManager.initializeYjsProvider(provider, ydoc, this.documentNameValue);
|
||||
this.ownedProvider = provider;
|
||||
|
||||
if (this.refreshUrlValue && this.tokenExpiresInSecondsValue) {
|
||||
|
||||
@@ -36,21 +36,47 @@ type Listener = (provider:HocuspocusProvider) => void;
|
||||
class LiveCollaborationManagerClass {
|
||||
yjsProviderInstance:HocuspocusProvider|null = null;
|
||||
yjsDocInstance:Doc|null = null;
|
||||
private currentDocumentName:string|null = null;
|
||||
|
||||
private listeners:Listener[] = [];
|
||||
|
||||
/**
|
||||
* Initializes the YJS Provider
|
||||
* Returns the active session for the given document, or null if none.
|
||||
*
|
||||
* Used by the init-yjs-provider Stimulus controller to detect that a
|
||||
* provider for the same document is already live — letting it adopt the
|
||||
* existing session instead of building a duplicate Y.Doc + provider pair.
|
||||
* Stimulus can fire `connect()` a second time (HMR replay, Turbo morph)
|
||||
* without firing `disconnect()`; without this check, the spurious re-init
|
||||
* would tear down the live Y.Doc and wipe the editor's Y.UndoManager
|
||||
* history mid-session.
|
||||
*/
|
||||
getCurrentSessionFor(documentName:string):{provider:HocuspocusProvider; doc:Doc} | null {
|
||||
if (this.yjsProviderInstance && this.yjsDocInstance && this.currentDocumentName === documentName) {
|
||||
return { provider: this.yjsProviderInstance, doc: this.yjsDocInstance };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Initializes the YJS Provider for the given document.
|
||||
*
|
||||
* Callers SHOULD first check {@link getCurrentSessionFor} and adopt any
|
||||
* existing session rather than calling this with a fresh provider, since
|
||||
* this method unconditionally tears down the previous provider/doc.
|
||||
*
|
||||
* @param provider The provider to use
|
||||
* @param doc The Y.Doc instance to use
|
||||
* @param documentName Logical identifier of the document being edited
|
||||
* @returns void
|
||||
*/
|
||||
initializeYjsProvider(provider:HocuspocusProvider, doc:Doc) {
|
||||
initializeYjsProvider(provider:HocuspocusProvider, doc:Doc, documentName:string) {
|
||||
this.destroyYjsProvider();
|
||||
this.destroyYjsDoc();
|
||||
|
||||
this.yjsProviderInstance = provider;
|
||||
this.yjsDocInstance = doc;
|
||||
this.currentDocumentName = documentName;
|
||||
this.listeners.forEach((listener) => listener(this.yjsProviderInstance!));
|
||||
}
|
||||
|
||||
@@ -82,6 +108,7 @@ class LiveCollaborationManagerClass {
|
||||
private destroy():void {
|
||||
this.destroyYjsProvider();
|
||||
this.destroyYjsDoc();
|
||||
this.currentDocumentName = null;
|
||||
|
||||
this.listeners = [];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user