The Future of @nytimes/react-prosemirror

SMores · September 15, 2023, 9:21pm

Hey all!

A little while ago, we shared our React ProseMirror integration library, @nytimes/react-prosemirror. Thank you so much to everyone who gave feedback, tried out the library, and posted issues on our repo!

For the past several months, I’ve been hard at work on a very different approach to integrate React and ProseMirror. The pull request is here for anyone who wishes to take a look.

The summary is this: the new approach completely replaces ProseMirror’s DOM management system with one built in React. We’re still using prosemirror-view for everything outside of change detection and DOM updates, which means we are exposing exactly the same API. In fact, I’ve ported over most of the unit tests from prosemirror-view to ensure that behavior matches the default library.

This change allows us to have a much cleaner integration with React, especially with regard to implementing node views with React components. Context flows as expected from parents to children, events bubble as expected, and node view components are just React components, without any special work needed. It also fixes a few edge case-y bugs, including significant issues with IME composition input. Overall, I feel confident that this is a huge improvement over the previous implementation, and one that could even be eventually pulled into higher level libraries like ReMirror and TipTap.

This new approach is implemented by subclassing EditorView and overriding some of the default behavior to essentially disable the DOM management. To do this subclassing, we needed to make two very small changes to the EditorView class, and I’m curious about how open you would be, Marijn, to incorporating these changes into prosemirror-view.

We changed updateStateInner to be protected, rather than private. This allowed us to override its implementation in our subclass, to essentially drop everything except for the selection syncing.
We moved all of the side effect-y code out of the EditorView constructor and into a new, protected, init method. This is still called in the constructor, but having it in an override-able method allows us to again drop the pieces of this that are managed by our new, React-based system.

Here’s the place in the pull request where this happens, in case anyone wants to take a look:

github.com

nytimes/react-prosemirror/blob/334bab87a88c524370113874809c94854fe5e907/src/prosemirror-view/index.ts#L70-L105


      
          constructor(place: null | DOMNode | ((editor: HTMLElement) => void) | {mount: HTMLElement}, props: DirectEditorProps) {
            this._props = props
            this.state = props.state
            this.directPlugins = props.plugins || []
            this.directPlugins.forEach(checkStateComponent)
          
            this.dispatch = this.dispatch.bind(this)
          
            this.dom = (place && (place as {mount: HTMLElement}).mount) || document.createElement("div")
            if (place) {
              if ((place as DOMNode).appendChild) (place as DOMNode).appendChild(this.dom)
              else if (typeof place == "function") place(this.dom)
              else if ((place as {mount: HTMLElement}).mount) this.mounted = true
            }
          
            this.editable = getEditable(this)
            this.nodeViews = buildNodeViews(this)
            this.docView = null as unknown as NodeViewDesc
          
            this.domObserver = new (props.DOMObserver ?? DOMObserver)(this, (from, to, typeOver, added) => readDOMChange(this, from, to, typeOver, added))

This file has been truncated. show original

There are also a few internal functions that we’re using. It would obviously make my life easier if these were exposed in some way, but I don’t want to ask for a huge increase in the API surface of prosemirror-view if that’s not something you’re interested in. These are the internal functions we’re using:

storeScrollPos and resetScrollPos (These are used in our updateStateInner override)
selectionFromDOM and selectionToDOM
computeDocDeco

Anyway, I’m looking for thoughts and feedback! In particular, Marijn, I’m very interested in hearing whether you would be open to a pull request against prosemirror-view with some or all of these changes. Please let me know if you have any questions!

marijn · September 21, 2023, 8:57am

This is interesting.

Your PR mentions replacing the DOM mutation observer with beforeinput handlers. Do I understand correctly that you’re no longer parsing the modified DOM? How do you reliably derive the appropriate changes from these events (which, on platforms like Android Chrome, tend to be fired in weird, incoherent batches)?

What do you use to replace the DOM-linked ViewDesc tree? Or are you still keeping a tree like that and adding it to a DOM node expando property?

Have you considered approaches not based on inheritance? I’ve worked on enough systems where inheritance-based extension mechanisms led to all kinds of fragile, hard-to-reason about messes to be very wary of that style. Would, maybe, something based on new ProseMirror view props that allow you to override certain subsystems (say, the renderer) with your own custom object be a viable route here?

SMores · September 21, 2023, 2:15pm

Your PR mentions replacing the DOM mutation observer with beforeinput handlers. Do I understand correctly that you’re no longer parsing the modified DOM?

That’s correct

How do you reliably derive the appropriate changes from these events (which, on platforms like Android Chrome, tend to be fired in weird, incoherent batches)?

Ah, actually I don’t have an Android device and hadn’t realized that this was an issue! I’ll have to do some testing and see; it’s possible that this approach simply doesn’t work on platforms like Android Chrome with poor beforeInput support. I think that the beforeInput approach ended up not being strictly necessary; it should be possible to go back to using prosemirror-view’s DOMObserver in this case.

Edit: I just did some testing; I see what you mean! This approach does indeed fall apart on Android Chrome. I’ll spend some time tomorrow seeing how much work it would be to go back to the DOMObserver; I suspect the biggest challenge will be IME composition events (which I otherwise haven’t really accounted for; they seem to “just work” with beforeInput).

What do you use to replace the DOM-linked ViewDesc tree? Or are you still keeping a tree like that and adding it to a DOM node expando property?

We do in fact still keep a DOM-linked ViewDesc tree on an expando property. This ended up being necessary for compatibility with prosemirror-view; parts of the library expect to be able to pull the ViewDesc off of a DOM node, so we maintained that behavior. The ViewDescs themselves are slightly modified, however; they don’t have update methods (since React does the actual DOM maintenance) and some of the constructors have changed slightly, since we have different data available at different points in the ViewDesc lifecycle than the default EditorView implementation. I think this won’t matter; I don’t remember seeing anywhere outside of the ViewDesc’s themselves that checked instanceof ViewDesc.

Have you considered approaches not based on inheritance? I’ve worked on enough systems where inheritance-based extension mechanisms led to all kinds of fragile, hard-to-reason about messes to be very wary of that style. Would, maybe, something based on new ProseMirror view props that allow you to override certain subsystems (say, the renderer) with your own custom object be a viable route here?

Absolutely. I agree with you on the fragility of inheritance-based extension systems. I started with this route because I was hesitant to add props that felt too specific to this use case, but I’m definitely happy to work on an approach that allows customization through EditorView props, rather than inheritance.