ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
27.68k stars 2.3k forks source link

[ReactNodeView] Provide more context to update function to enable fewer re-renders #1611

Closed jamesopti closed 3 years ago

jamesopti commented 3 years ago

The problem I am facing I’m always frustrated when … my React NodeViews re-render on every keystroke! This is usually unnecessary (in our application at least) and becomes noticeable when typing fast.

The solution I would like More fine tuned control over when the ReactNodeView invokes updateProps on its renderer instance.

I've forked @tiptap/react and implemented the following update to ReactNodeView.update, which passes more information to the update function provided by the extension which allows it to decide whether or not to call updateProps

Proposed Change

  // In ReactNodeView.update
  update(node: ProseMirrorNode, decorations: Decoration[]) {
    if (typeof this.options.update === 'function') {
      const prevNode = this.node
      const prevDecorations = this.decorations

      this.node = node
      this.decorations = decorations

      const updateProps = () => {
        this.renderer.updateProps({ node, decorations })
        this.maybeMoveContentDOM()
      }

      return this.options.update(
        node,
        decorations,
        prevNode,
        prevDecorations,
        updateProps
      )
    }

    if (node.type !== this.node.type) {
      return false
    }

    if (node === this.node && this.decorations === decorations) {
      return true
    }

    this.node = node
    this.decorations = decorations
    this.renderer.updateProps({ node, decorations })
    this.maybeMoveContentDOM()

    return true
  }

Example Usage

  // In an Extension
  addNodeView() {
    return ReactNodeViewRenderer(DocView, {
      update: (node, decorations, prevNode, prevDecorations, updateProps) => {
        const attrsChanged =
          JSON.stringify(node.attrs) !== JSON.stringify(prevNode.attrs)
        // Only update props and re-render if the node.attrs changed
        if (attrsChanged) {
          updateProps()
        }
        return true
      },
    })
  },
philippkuehn commented 3 years ago

I’m always frustrated when … my React NodeViews re-render on every keystroke!

This really shouldn’t happen. Can you provide a codesandbox for that?

But your changes looks good. Will add that later!

philippkuehn commented 3 years ago

Okay, I can reproduce this. Not sure if we may check for unchanged attributes by default.

jamesopti commented 3 years ago

Okay, I can reproduce this. Not sure if we may check for unchanged attributes by default.

Given that custom node view are, well, custom, it makes sense to me to allow the application to implement its own shouldUpdate logic. For us, there are some wrapper custom nodeviews that are entirely presentational and only care about the node.attrs changing, but others care about specific decorations, and others need to re-render on every change (current behavior).

Extending the options.update function to pass more parameters makes the most sense to me.

In our fork though, I couldnt figure out how to make Typescript happy with 3 new parameters to the underlying NodeView update function :(

philippkuehn commented 3 years ago

you can do this with the new implementation:

// In an Extension
addNodeView() {
  return ReactNodeViewRenderer(Component, {
    update: ({ oldNode, oldDecorations, newNode, newDecorations, updateProps }) => {
      const attrsChanged = JSON.stringify(newNode.attrs) !== JSON.stringify(oldNode.attrs)
      // Only update props and re-render if the node.attrs changed
      if (attrsChanged) {
        updateProps()
      }
      return true
    },
  })
},