uiwjs / react-codemirror

CodeMirror 6 component for React. @codemirror https://uiwjs.github.io/react-codemirror/
https://uiwjs.github.io/react-codemirror/
MIT License
1.67k stars 133 forks source link

Merge view - jumping onChange #681

Open natocTo opened 2 months ago

natocTo commented 2 months ago

Hello,

we are trying to implement merge view into our app and we came across this behaviour.

There is a simple sandbox: https://codesandbox.io/p/sandbox/competent-tom-8zpsy7

Please try to enter severals new lines (for example 50) into second editor (I just keep presed enter for while). When it hits certain point the view "jump" up over active cursor. And anytime you enter another char there (on some latest lines usually), it behave same (jump a little bit up). So it is really hard to write something there.

Did I miss some setting I have to use? Thank you very much for any help.

jaywcjlove commented 2 months ago

@natocTo upgrade v4.23.1

import React, { useState } from 'react';
import CodeMirrorMerge from 'react-codemirror-merge';

const Original = CodeMirrorMerge.Original;
const Modified = CodeMirrorMerge.Modified;
let doc = `one
two
three
four
five`;

export default function App() {
  const [value, setValue] = useState(doc);
  const [valueModified, setValueModified] = useState(doc);
  return (
    <div>
      <CodeMirrorMerge destroyRerender={false}>
        <Original
          onChange={(val) => {
            console.log('~~:1', val);
            setValue(val);
          }}
          value={value}
        />
        <Modified
          onChange={(val) => {
            console.log('~~:2', val);
            setValueModified(val);
          }}
          value={valueModified}
        />
      </CodeMirrorMerge>
      <div style={{ display: 'flex', marginTop: 10 }}>
        <pre style={{ flex: 1 }}>{value} </pre>
        <pre style={{ backgroundColor: '#fff', flex: 1 }}>{valueModified} </pre>
      </div>
    </div>
  );
}
cjayyy commented 1 month ago

@jaywcjlove Thank you for the update! The destroyRerender={false} seems to solve the glitch for us. But with that the custom theme and extensions no longer work. Do you know why by any chance?

jaywcjlove commented 1 month ago

@cjayyy After the MergeView is initialized, it doesn't update the extensions. I guess it might be that your extensions are loaded asynchronously.

https://github.com/uiwjs/react-codemirror/blob/e79b1f3b58a6c0bb0d1068a3f188ba87bafbb9c1/merge/src/Internal.tsx#L67-L90

If your extensions are asynchronous, add them to the editor.

https://github.com/uiwjs/react-codemirror/blob/e26ca151a61bff6d03314e5e52b738285d8d7128/www/src/pages/examples/Example681.tsx#L49-L53

Implementing extensions updates inside the editor causes errors, and I haven't found a solution for this. It may require support from the CodeMirror team, and you can seek answers from their community.

https://github.com/uiwjs/react-codemirror/blob/e26ca151a61bff6d03314e5e52b738285d8d7128/merge/src/Internal.tsx#L99-L102

cjayyy commented 1 month ago

Not using any async extensions. Even if I use only minimal setup like below, destroyRerender={false} completely disables the functionality of those two extensions (both editors are editable). When setting destroyRerender={true}, extensions work.

<CodeMirrorMerge destroyRerender={false}>
   <CodeMirrorMerge.Original
      value="first"
      extensions={[EditorView.editable.of(false), EditorState.readOnly.of(true)]}
   />
   <CodeMirrorMerge.Modified value="second" />
</CodeMirrorMerge>
jaywcjlove commented 1 month ago

@cjayyy My example test didn't have any issues -> https://uiwjs.github.io/react-codemirror/#/examples/681

https://github.com/uiwjs/react-codemirror/blob/e26ca151a61bff6d03314e5e52b738285d8d7128/www/src/pages/examples/Example681.tsx#L59-L66

cjayyy commented 1 month ago

Oh I overlooked you published version 4.23.2 - with that it works! (4.23.1. didn't work for me). Thank you very much. Will test it more thoroughly tomorrow. 🤞

cjayyy commented 1 month ago

Thank you. It looks like the original issue with lines jumping is fixed now.

We are still struggling with another (probably) unrelated issue though; When changing the doc value fast enough the change handlers (our outside onChange handler` and your internal change dispatching) get mixed up and cycled in and infinite loop causing last change undoing and redoing forever leaving the editor stuck unusable. My guesses:

Could please point me into the right direction? 🙏

natocTo commented 4 days ago

@jaywcjlove it looks like after update to v4.23.1 (and newer) the onChange is actually not called at all.

Check this sandbox - https://codesandbox.io/p/sandbox/gracious-chandrasekhar-zqmsy9?file=%2Fsrc%2FApp.js it should show alert when updating a value. But it is not called.

jaywcjlove commented 3 days ago

@natocTo I suggest using the approach in the example below to handle the onChange event.

<Modified
  onChange={(val) => {
    console.log('~~:2', val);
    setValueModified(val);
  }}
  value={valueModified}
/>
natocTo commented 3 days ago

Hi @jaywcjlove I did not realize the sandbox may change after I send a link, I used onChange originally. Please check sandbox again. There is onChange but it is never called.

https://codesandbox.io/p/sandbox/gracious-chandrasekhar-zqmsy9?file=%2Fsrc%2FApp.js%3A1%2C1-39%2C1

jaywcjlove commented 3 days ago

@natocTo https://codesandbox.io/p/sandbox/nostalgic-leakey-3frkm2?workspaceId=dca383b2-9fa2-4898-b548-3527e348e0e1

https://github.com/user-attachments/assets/758e4685-e33c-49f0-a3e2-fc977af04d8d

import React, { useState } from "react";

import CodeMirrorMerge from "react-codemirror-merge";
import { EditorView } from "codemirror";
import { EditorState } from "@codemirror/state";

const Original = CodeMirrorMerge.Original;
const Modified = CodeMirrorMerge.Modified;
let doc = `one
two
three
four
five`;

export default function App() {
  const [value, setValue] = useState(doc);
  const [valueModified, setValueModified] = useState(doc);
  return (
    <div>
      <CodeMirrorMerge destroyRerender={false}>
        <Original
          extensions={[
            EditorView.editable.of(false),
            EditorState.readOnly.of(true),
          ]}
          onChange={(val) => {
            console.log("~~:1", val);
            setValue(val);
          }}
          value={value}
        />
        <Modified
          extensions={[
            EditorView.editable.of(true),
            EditorState.readOnly.of(false),
          ]}
          onChange={(val) => {
            console.log("~~:2", val);
            setValueModified(val);
          }}
          value={valueModified}
        />
      </CodeMirrorMerge>
      <div style={{ display: "flex", marginTop: 10 }}>
        <pre style={{ flex: 1 }}>{value} </pre>
        <pre style={{ backgroundColor: "#fff", flex: 1 }}>{valueModified} </pre>
      </div>
    </div>
  );
}
natocTo commented 3 days ago

@jaywcjlove thank you! Can you elaborate more why it is working for you? It is because of destroyRerender?

It looks like it is false by default https://github.com/uiwjs/react-codemirror/blob/v4.23.6/merge/src/index.tsx#L12

jaywcjlove commented 2 days ago

https://github.com/uiwjs/react-codemirror/blob/8b091c37ff52077bf63bf97302a90b45f57c8646/merge/src/Internal.tsx#L118-L159

@natocTo destroyRerender is designed to address the issue of extensions failing to refresh updates.