react-codemirror2 icon indicating copy to clipboard operation
react-codemirror2 copied to clipboard

on("changes") fails to fire under certain circumstances

Open justinpombrio opened this issue 6 years ago • 2 comments

This may be a stack management issue. It might also be a CodeMirror bug, though I haven't figured out how to reproduce it outside of react-codemirror2.

Background

In the project I work on, for every group of changes, we:

  1. Speculatively commit those changes to a temporary CodeMirror instance, then
  2. Wait for on("changes") to fire
  3. If the changes look valid, replay them on the real CodeMirror instance.

The Issue

We recently ran into a situation in which on("changes") fails to fire after a call to .operation(). Here's the minimal (though unfortunately rather long) code to reproduce the issue:

import React from 'react';
import ReactDOM from 'react-dom';
import CodeMirror from 'codemirror';
import {UnControlled} from 'react-codemirror2';

function keyDown(realCM, event) {
  if (event.key === "Backspace") {
    event.preventDefault();

    // Create a temporary CodeMirror instance                                      
    const tmpDiv = document.createElement('div');
    const tmpCM = CodeMirror(tmpDiv, {value: ""});
    tmpCM.setValue(realCM.getValue());

    // Define an on("changes") handler                                             
    let handler = (_, changeArray) => {
      console.log("!!! Handler called !!!"); // Never called!                      
    };

    // Make a change. This should trigger on("changes"), but it won't.             
    console.log("tmpCM.curOp is set before anything has happened:", tmpCM.curOp);
    tmpCM.on('changes', handler);
    tmpCM.operation(() => {
      console.log("Inside of `.operation()`. Calling `tmpCM.replaceRange`");
      tmpCM.replaceRange("", {line: 0, ch: 0}, {line: 0, ch: 4}, 'cmb:delete');
    });
    tmpCM.off('changes', handler);
  }
}

ReactDOM.render(
  (<UnControlled
      value={"test"}
      onKeyDown={keyDown}
   />),
  document.getElementById('cmb-editor'));

To reproduce, place the cursor at the end of "test", and press backspace.

The issue is that the on("changes") handler fails to fire after the call to .operation(). You can see this because console.log("!!! Handler called !!!") is never logged.

Speculation on Causes

I looked at why on("changes") fails to fire, and see two potential reasons.

  1. After the call to tmpCM.setValue(realCM.getValue()), tmpCM.curOp is not null. As a result, runInOp() (which gets invoked when we call tmpCM.operation) never calls endOperation() (which would eventually call signal()).

  2. The "changes" handler fails to fire because in the outermost stack frame, by the time the handlers are checked, tmpCM.off("changes") has already been called in the above code. (This stack frame looked like it was related to the key press, so I wouldn't expect it to be responsible for calling the "changes" handler, though.) As a demonstration of this, if you wrap the call to tmpCM.off in setTimeout(..., 0), then the issue is resolved and the "changes" handler is called.

justinpombrio avatar May 28 '19 16:05 justinpombrio

@justinpombrio I am a lot shorter on time these days as when I started this project. Codemirror & React APIs are moving to quickly for me to keep atop of for the day-to-day. I am looking for a co-maintainer of this project. Please contact me directly if you are interested. Thank you for understanding.

scniro avatar Jan 19 '20 16:01 scniro

@scniro I don't have time to work on codemirror-react2 myself. That said, maintaining an open source project like this is a valuable public service. Thank you for your service, however much time you have to dedicate to it! It is especially admirable in the fast-paced JS ecosystem.

justinpombrio avatar Jan 20 '20 15:01 justinpombrio