monaco-react: ControlledEditor doesn't fire onChange upon Undo/Redo

Awesome 🎉 🎉 happy to help 😃

@patrickshipe @seraph0x @RickeyWard @SivaKranthiKumar @jdroigk @jpeterson @travisdahl @rosslovas hello everyone 🙂

The problem is solved. A check has been missed during rewriting. Let’s thank @patrickshipe for a great catch.

The new version (v3.4.2) has already published 🎉 If it’s possible, please check it.

Thank you all for your support!

Hello @rosslovas, @travisdahl. Sorry for the late answer guys.

I can approve - there is such kind of problem. We have faced it a couple of months ago. The explicit check you have already noticed is added intentionally. I’ll work on it, and keep you up to date here.

Thank you for your support!

Maybe I am missing something @suren-atoyan, but it seems that part of the issue is that the controlled editor will always run the executeEdits when value prop changes, even if value === editorRef.current.getModel().getValue(), which quickly clutters the undo stack. If the value prop changes but it strictly equals the model’s current value, why do we make the update?

Hello @patrickshipe. You are right. There is no need, moreover, it will “spoil” the undo stack. I think the problem is that when I was rewriting it from a class component into a function component with hooks (a while ago), and converting big componentDidMount into small useUpdates, I missed that check and got an illusion of necessity of executeEdits as there was a dependency array with value (something like this useUpdate(...., [value])). Long story short, that’s a great catch, thank you, I am sure it will solve the problem with undo/redo. Let me check it today. And if everything is okay, I’ll publish a new version.

@RickeyWard Thanks for sharing your implementation! This worked great for me to fix this bug.

I noticed another bug whereby holding backspace would skip some characters. (Basically, two onChange events would go up the React component chain before the first one’s new value came back down the chain. I’m using Redux, so this might be related to a delay in Redux.)

I solved it by treating <Editor> as a true uncontrolled component (value prop is constant and all value updates happen imperatively) and debouncing the onChange handler which also drastically improved performance for the controlled editor. If anyone is interested, I’ll include my modifications to @RickeyWard’s code below.

import React, { useRef, useEffect, useMemo } from 'react'
import PropTypes from 'prop-types'
import debounce from 'lodash.debounce'

import Editor from '@monaco-editor/react'

const noop = (_) => {}

const DEFAULT_UPDATE_DEBOUNCE_DELAY = 100

const ControlledEditor = ({ value, onChange, editorDidMount, ...props }) => {
  const previousValue = useRef(value)
  const editorRef = useRef()
  const debouncedOnChange = useMemo(
    () =>
      debounce((event, value) => {
        previousValue.current = value
        onChange(event, value)
      }, DEFAULT_UPDATE_DEBOUNCE_DELAY),
    [onChange]
  )

  useEffect(() => {
    if (value !== previousValue.current && editorRef.current) {
      editorRef.current.setValue(value)
    }
    previousValue.current = value
  }, [value])

  const handleEditorDidMount = (getValue, editor) => {
    editorRef.current = editor
    editor.setValue(previousValue.current)
    editor.onDidChangeModelContent((ev) => {
      const currentValue = editor.getValue()
      if (currentValue !== previousValue.current) {
        debouncedOnChange(ev, currentValue)
      }
    })

    editorDidMount(getValue, editor)
  }

  return <Editor value="" editorDidMount={handleEditorDidMount} {...props} />
}

ControlledEditor.propTypes = {
  value: PropTypes.string,
  editorDidMount: PropTypes.func,
  onChange: PropTypes.func
}

ControlledEditor.defaultProps = {
  editorDidMount: noop,
  onChange: noop
}

export default ControlledEditor

I noticed that the _isControlledEditor prop can be removed with no apparent ill effects. All it seems to do is force tokenization on every update - why do you need that?

Hello @jpeterson, if you want to solve the problem in your particular case, I can suggest you use a custom controlled component made by you. I mean, the Controlled Editor component we have now is just a small wrapper around the main Editor component. So, you can easily implement it, for a hint you can examine this file. Until we will find a universal solution to solve problems of all.