megahertz / react-simple-wysiwyg

Simple and lightweight React WYSIWYG editor
https://megahertz.github.io/react-simple-wysiwyg/
MIT License
119 stars 21 forks source link

Stale onChange listener #11

Closed TrevorMills closed 1 year ago

TrevorMills commented 2 years ago

Hi there,

Thanks for your project. I'm using it with good success on a project I'm working on.

However, I've come across an issue that I'm hoping you can help me resolve. I've created a repo that demonstrates the issue. It's at https://github.com/TrevorMills/react-simple-wysiwyg-issue.

Consider a component that looks like this:

import {useState} from 'react';
import {Editor, EditorProvider} from 'react-simple-wysiwyg';

const App = () => {

    const [state, setState] = useState({
        plain: 'Initial plain text',
        rich: 'Initial rich text',
    });

    const onChange = (which, value) => {
        setState({
            ...state,
            [which]: value,
        });
    }

    return (
        <div className="App">
            <div>
                <label htmlFor="plain-text">Plain Text</label><br/>
                <input type="text" id="plain-text" value={state.plain}
                       onChange={e => onChange('plain', e.target.value)}/>
            </div>
            <div>
                <label>Rich Text</label>
                <EditorProvider>
                    <Editor value={state.rich}
                            onChange={e => onChange('rich', e.target.value)}/>
                </EditorProvider>
            </div>
        </div>
    );
}

export default App;

You'll notice that I have an onChange listener that contains a dependency on the current state. My actual application is quite a bit more complicated than this, but the issue is the same. The onChange listener uses values from the current state when it makes the update.

If you make a change on the plain text field and then make a change on the rich text field, the plain text field reverts to its original value. That's the issue.

I can see in your code that the ContentEditable component is returned as a memoized function with dependencies on a few things, but not the onChange prop. Because of that, when onChange is called from the wysiwyg editor, a stale version of my onChange function is called, causing the plain text field reverts to its initial value.

I'm not sure if this can be solved by just adding onChange as a dependency to your useMemo call, or if that has other implications.

I noticed that there is a dependency on the onBlur and onKeyDown props, so I tried passing in new functions for those on every change. The plain text field then retained its changed value, but any time I made a change to the wysiwyg editor, the caret moved to the beginning of the editable content area, making it unusable. I couldn't figure out a way to get your component running the fresh version of my onChange. I'm hoping you might be able to address this.

Again, thank you for your work on this and for releasing it open source.

megahertz commented 2 years ago

Thank you for the example, I'll try to investigate that issue on the next week. BTW, have you tried passing a callback to setState instead?

TrevorMills commented 2 years ago

The example is contrived, in my real project, I'm receiving the onChange callback from a higher order component that is not using setState.

megahertz commented 1 year ago

I apologise for the delay. Fixed in v2.1.2