Issue with pressing Enter when using with React

[tylerlaws0n]

This is an issue in the sandbox:

When you are in Code mode and press enter it resets the cursor to the top of the editor. I am assuming this has something to do with the implementation of the value or onChange in the ace editor, but would appreciate help with debugging this.

[josdejong]

Thanks for reporting. I first tried in Firefox, there the issue doesn't happen. On Chrome I can reproduce your issue.

[tylerlaws0n]

So I found out that the issue is related to the controlling of content on the AceEditor. When you remove the content prop from JsonEditor and allow the onChange to be called on the uncontrolled component it edits flawlessly without this issue. With this in mind I believe it would be best to add a new prop to JSONEditor called defaultContent or initialValue which would set the ace editor value and the tree mode value on the initial render, but would not continue to set the value as changes are made in the editor.

I'd say that uncontrolled should also be the recommendation for the React demo as well; this thread facebook/react#955 shows a long running confusion with controlled inputs with regards to state in React, so considering the editor works as expected and can pass updates when uncontrolled, I think that makes most sense.

[josdejong]

Just to have some context, I want svelte-jsoneditor not only to work with React, but want to have it work in the following cases:

• in Svelte, using 2-way binding (bind:content)
• in Svelte, React, Vue, Angular, etc using 1-way binding (content and onChange)
• in plain JavaScript using methods get(), set(), update() (uncontrolled component)

Having a mix of controlled and non-controlled use cases makes things complicated indeed. I'm afraid there is no way around it except when removing some of the use-cases above.

I've done a bit of debugging in the sandbox, here an adapted version with some logging that gives insight:

https://codesandbox.io/s/svelte-jsoneditor-react-issue-23-yfobq?file=/src/App.js

What I noticed is that when you press enter in code mode, in most of the cases this will trigger two changes: first a change with the new return character added, directly followed by a change that has added indentation to the new line.

In React, the useEffect (and render?) is executed asynchronously, which causes a nasty race condition in this case. So what happens is:

• the editor has contents A
• the user presses Enter
• the editor's contents is changed from A into B (return character added), and the editor fires the first onChange event
• React starts rendering the updated content to B
• the editor's contents is changed from B into C (indentation added), and the editor fires the second onChange event
• react applies the first content update B in the useEffect hook, whilst the editor itself has contents C. This resets the caret since the contents changed from C to B
• React starts rendering the updated content to C
• react applies the second content update C in the useEffect hook. This resets the caret since the contents changed from B to C

I'm not sure why the caret is only reset on Chrome and not Firefox. Maybe Firefox has some smartness in place to keep caret positions in place or so.

So how to solve this?

1. You're idea of using it in React as an uncontrolled component would indeed solve it but feels like a workaround, it's against the spirit of React. But I guess it's the only 100% safe solution. You can set the content once and not update it after that.
2. A simple, pragmatic solution could be to debounce the output of the code editor, so only one onChange event is triggered, and this issue will not occur in practice.

Any thoughts?

[josdejong]

I've implemented the de-bouncing solution in bc2c559 and published this in v0.3.3 Can you give this a try and let me know what you think about this approach? (We can refine/change if needed)

[tylerlaws0n]

Thanks for all the help debugging here, this is super helpful. The debounce feels like a safe, but less than ideal solution. I agree that making it uncontrolled in react is a workaround. This seems like a much better workaround than just leaving it uncontrolled, but it is still a workaround to deal with the double call to onChange that is emitted from ace editor.

I feel like the best possible solution here would be for a correction to the indentation inside ace editor before emitting the first onChange. What do you think?

[josdejong]

I feel like the best possible solution here would be for a correction to the indentation inside ace editor before emitting the first onChange. What do you think?

Well, yes that would prevent this issue, but it's not really an issue in ace editor itself. It is a React specific issue, caused by the combination of the async rerendering of react and us wanting to control a controlled component from outside. In theory it can happen anyway in Ace editor when pressing two characters infinitely fast after each other, resulting in two onChange events like we see with the indentation. Anyway, I think it's fine having this debouncing in place. It does not only solve the async issue in React, but also reduces the amount of rerenders/roundtrips when triggering onChange less often.