The use of reset create a rendering infinite loop

[WillFr]

1. What version of RJEA are you using (react-json-editor-ajrm version)? 2.5.8
2. What operating system and processor architecture are you using? MacOS
3. What did you do? attempted to use the reset flag
4. What did you expect to see? I expected to see the component display the placeholder when the placeholder is updated
5. What did you see instead? An infinite rendering loop is triggered because index.js line 631 uses setState. This will rerender the component, which in turn will call showPlaceholer again, which will call setState, etc etc.

Repro steps:

• set reset to true and use a placeholder
• attempt to modify the content

[SachsKaylee]

Hello - Thank you for reporting this & sorry for getting back at you so late.

This behavior was present in the original implementation of the reset feature, but was fixed before it was merged or released. See this comment - https://github.com/AndrewRedican/react-json-editor-ajrm/pull/73#issuecomment-427592640

Can you reproduce your problem with this setup, if not how does your code differ? (Maybe you could post a simple self contained example that is able to reproduce the issue)?

[WillFr]

Hello Patrick, In essence this is the code I am using :

export default class ServiceRow extends React.Component {
constructor(props) {
this.state = { modified: false }
this.placeholder = { a: 1, b:2, c:3 }
this.onChange = this.onChange.bind(this);
}

onChange(e) {
this.setState({ modified: true})
}

render() {
        const { modified } = this.state;
       return (
              <div class=`component${modified ? " modified" : ""}`>
              <JSONInput
                                id={"json-input"}
                                placeholder={this.placeholder}
                                theme="light_mitsuketa_tribute"
                                onChange={this.onChange}
                                waitAfterKeyPress={100}
                               reset={true}
                            />
       )

[SachsKaylee]

Hello Will,

First of all: After some digging the current version of the editor does indeed have some issues regarding nested update calls, but I'm not sure if this is related to it.

I see several issues with this code. One would be that the onChange function never fires, since the onChange handler get the oneChange function which will evaluate to undefined.

The next one would be that the placeholder is declared as a class level variable(this.placeholder = { a: 1, b:2, c:3 }) instead of a state property, but is used as such(placeholder={this.state.placeholder}). Regardless of this, having class variables for data is discouraged in react as it can create strange update behavior.

Furthermore, I don't see any necessity for using the reset={true} prop in this scenario, as the actual placeholder prop never changes. If this is the code used in the current app I'd like to encourage you to address the points mentioned above. Otherwise I'd need a more complete example.

[WillFr]

Hi Patrick, I should have mentioned it but this snippet of obviously not production code. It only intends to give you a simple repro (it was actually coded locally and not copy pasted).

I edited the snippet above to remove the two typo you caught (thank you btw)

I used a class variable for the placeholder to show that it was not related to the placeholder changing (class variable don't trigger rerender)

Basically, I wanted to show that when reset = true infinite rendering loop will happen. Why we need reset is not so relevant to the issue really, unless reset is never intended to be hardcoded to true.

[AndrewRedican]

Hi @WillFr,

Thanks for your feedback.

Can you clarify wht you use reset in your projet?

Whenever reset=true, th expected behavior is for for RJEA to accept a new placeholder value and re-render if either A) the existing placeholder value is different or B) if the current content displayed is any different from the new placeholder value that is being assigned.

If the current placeholder value AND contents within are exactly the same, the behavior is for reset=true configuration not to do anything, thus re-render, and no onChange callback.

I will review this today with the example provided.

[WillFr]

Hi Andrew, We use the reset property for a "reset form" mechanism: basically, RJEA is part of component that has a reset button, when the reset button is clicked, we want to render the original place holder, even though the place holder has not changed.

Currently we worked around it by not using reset and doing a reference equality check line 612, rather than calling the identical function, then we just clone the placeholder when we want to reset the form. It works, but it sounds like our use case align with what reset is for, let me know if that is not the case.

[AndrewRedican]

I no longer intend to update this project. I am working instead on a complete rewrite.

I'd like to thank you for using and taking interest in this project.

I also would like to apologize for not following up sooner. However, do realize for the most part this has been a one-man show, and the occasional contributions I used to receive. What I am saying is, I am in dire need of volunteers.

If this is something you would be interested in participating in, you can join in the discussion.

I've taken note of this thread, and I'll keep this under consideration for the new project.