Use @handlewithcare/react-codemirror in CodeBlock - #145
Conversation
| useStopEvent((_view, event) => { | ||
| if (event instanceof InputEvent) return true; | ||
| return false; | ||
| }); |
There was a problem hiding this comment.
We want CodeMirror to handle all of the input events, and do not want them to propagate up to ProseMirror. I guess I did not test whether this is actually necessary to accomplish that
There was a problem hiding this comment.
Hm. Yeah, it seems like this doesn't do anything haha
| if ( | ||
| editorState.selection.from >= getPos() && | ||
| editorState.selection.to <= getPos() + node.nodeSize && | ||
| (codeMirrorState.selection.main.anchor !== | ||
| editorState.selection.$anchor.parentOffset || | ||
| codeMirrorState.selection.main.head !== | ||
| editorState.selection.$head.parentOffset) | ||
| ) { | ||
| setCodeMirrorState( | ||
| (prev) => | ||
| prev.update({ | ||
| selection: { | ||
| anchor: editorState.selection.$anchor.parentOffset, | ||
| head: editorState.selection.$head.parentOffset, | ||
| }, | ||
| }).state | ||
| ); | ||
| } |
There was a problem hiding this comment.
What's going on here? Syncing the selection from the parent editor into the CM editor?
| // We need to maintain extension state and selection | ||
| // between renders, so we can't recompute the CodeMirror | ||
| // EditorState from node.textContent on each render. | ||
| // | ||
| // The next best thing is to update state during render | ||
| // when the node content doesn’t match the EditorState. | ||
| // This will trigger another render, but React will abort | ||
| // this render without running effects or committing | ||
| // to the DOM, so we avoid any state tearing. | ||
| if (node.textContent !== codeMirrorState.doc.toString()) { | ||
| setCodeMirrorState((prev) => { | ||
| const current = prev.doc.toString(); | ||
| const incoming = node.textContent; | ||
| const diffed = diff(current, incoming); | ||
|
|
||
| return prev.update({ | ||
| changes: diffed.map((change) => ({ | ||
| from: change.fromA, | ||
| to: change.toA, | ||
| insert: incoming.slice(change.fromB, change.toB), | ||
| })), | ||
| }).state; | ||
| }); | ||
| } |
There was a problem hiding this comment.
This kind of concern comes up everywhere I've ever seen a nested editor. I'd love a nice pattern for it. Is it safe to do the set in render or is it better done in an effect?
There was a problem hiding this comment.
I wonder if we could get by assuming that we can always ignore outside changes, because if the outside change were to create/destroy/replace the content totally this node would get rekeyed and we'd remount anyway. Like, I wonder if we can lean on our React keys plugin to make some assumptions here!
There was a problem hiding this comment.
Is it safe to do the set in render or is it better done in an effect?
Yes, and actually it's specifically recommended to do this rather than an effect: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes. As noted in the comment, React will abort the render when a state setter is called during render and restart with the new state, so it's significantly more performant to do this than an effect (also avoids some state tearing).
I wonder if we could get by assuming that we can always ignore outside changes,
In this literal demo editor, maybe, but no, I don't think this is safe, generally. Collab changes, for example, would change the node content without causing a rekey/remount. Also, like, find/replace, etc. Any transaction could conceivably change the contents of this node!
| ref={(el) => { | ||
| ref.current = el; | ||
| if (!outerRef) { | ||
| return; | ||
| } | ||
| if (typeof outerRef === "function") { | ||
| outerRef(el); | ||
| } else { | ||
| outerRef.current = el; | ||
| } | ||
| }} |
There was a problem hiding this comment.
I'm not seeing any place where the inner ref is used. Can we just pass through the forwarded ref without this ceremony?
There was a problem hiding this comment.
Oh, yes, this is probably stale (this diff appears much larger than it actually is because stuff got moved around, this is leftover from the previous implementation)
| state.selection.from >= getPos() && | ||
| state.selection.to <= getPos() + node.nodeSize | ||
| ) { | ||
| view.focus(); |
There was a problem hiding this comment.
note: we could use a useSetSelection to provide this functionality through the node view spec of the desc.
There was a problem hiding this comment.
Ah, nice! Yeah that would be great.
82e6083 to
29d7d10
Compare
3fe4b6e to
33eed37
Compare
@handlewithcare/react-codemirror has some behavior over @uiw/react-codemirror that makes it better suited for a ProseMirror integration. It allows you to lift the EditorState out of the CodeMirror component, which means that we can derive parts of the state, like the selection from the ProseMirror state. We can also compute smaller diffs when updating the state, rather than completely replacing the doc (which triggers recalculation of the syntax highlighting, decorations, etc) any time the node content is updated from outside the CodeMirror editor.
33eed37 to
7c05d98
Compare
3 participants
@handlewithcare/react-codemirror has some behavior that makes it better suited for a ProseMirror integration than @uiw/react-codemirror. It allows you to lift the EditorState out of the CodeMirror component, which means that we can derive parts of the state, like the selection from the ProseMirror state.
We can also compute smaller diffs when updating the state, rather than completely replacing the doc (which triggers recalculation of the syntax highlighting, decorations, etc) any time the node content is updated from outside the CodeMirror editor.