Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with setState in event handlers #1023

Closed
2 tasks done
MatejBransky opened this issue Sep 4, 2018 · 14 comments
Closed
2 tasks done

Bug with setState in event handlers #1023

MatejBransky opened this issue Sep 4, 2018 · 14 comments
Labels
bug possibly close To confirm if this issue can be closed

Comments

@MatejBransky
Copy link

MatejBransky commented Sep 4, 2018

Prerequisites

  • I have read the documentation;
  • In the case of a bug report, I understand that providing a SSCCE example is tremendously useful to the maintainers.

Description

There is an App which renders rjsf Form. When you call App's setState(updates value unrelated with the form's props) in Form.props.onChange then Form loses its internal state.

Steps to Reproduce

  1. Create a stateful component (A) which renders the Form with some simple schema.
  2. In this component (A) create onChange handler which is passed into the Form.
  3. In this handler call setState which updates some value which is not passed into the Form component.
  4. Type something into the input and you'll see that it immediately removes the typed value.

Here is a reproducible demo.

Expected behavior

Form shouldn't loose its state.

Actual behavior

Form lost its state.

Version

1.0.4

@MatejBransky
Copy link
Author

MatejBransky commented Sep 4, 2018

Here is the new test which should fail:

import React from "react";
import { render, fireEvent } from "react-testing-library";
import Form from "../src";

it("should call provided change handler and keep the form state", () => {
  const schema = { title: "Foo", type: "string" };
  class App extends React.Component {
    state = { calls: 0 };

    handleChange = ({ formData }) => {
      this.setState(prevState => ({ calls: prevState.calls + 1 }));
    };

    render() {
      return (
        <Form
          schema={schema}
          onChange={this.handleChange}
          safeRenderCompletion={true}
        />
      );
    }
  }

  const { getByLabelText } = render(<App />);
  const input = getByLabelText("Foo");

  fireEvent.change(input, { target: { value: "bar" } });
  expect(input.value).toBe("bar"); // => fails, received: ""
});

It doesn't fail in CodeSandbox (bug?) but if you download the demo and try it on the local then you see that it fails.

@MatejBransky MatejBransky changed the title Bug with onChange handler Bug with setState in handlers Sep 4, 2018
@MatejBransky MatejBransky changed the title Bug with setState in handlers Bug with setState in event handlers Sep 4, 2018
@MatejBransky
Copy link
Author

MatejBransky commented Sep 4, 2018

I've found the source of the issue. It's in the componentWillReceiveProps in the Form component.
Everytime when you update a parent component then Form will reevaluate the internal state. You should check props used in the getStateFromProps and only after changes in these props you should call getStateFromProps.

I'm working with my fork (which uses templates from #1013 and it uses jest) but here is related commit.

@MatejBransky
Copy link
Author

Now I'm not using cWRP (I've completely removed it) at all. It looks like that the prop key is the better way for uncontrolled components.

@Leksat
Copy link

Leksat commented Jul 5, 2019

Uh, it took me several hours to find this.

My current workaround: extend Form class and override componentWillReceiveProps to call the parent method only if it is really required.

@affan-speridian
Copy link

affan-speridian commented Aug 10, 2020

Any update on this? @Leksat can you please guide how can I follow your work around using hooks?
@epicfaace any work around would be very appreciated. Thanks

@Leksat
Copy link

Leksat commented Aug 10, 2020

@affan-speridian

import Form from 'react-jsonschema-form';

class FixedForm extends Form {
  componentWillReceiveProps(nextProps) {
    const shouldSkipUpdate =
      Object.keys(nextProps).length === Object.keys(this.props).length &&
      Object.keys(nextProps).filter(
        key =>
          key !== 'children' &&

          // Maybe some other logic.

          nextProps[key] !== this.props[key]
      ).length === 0;
    if (shouldSkipUpdate) {
      return;
    }
    super.componentWillReceiveProps(nextProps);
  }
}

@affan-speridian
Copy link

@Leksat hey, thanks for responding. Much appreciated. I just tested it. It works but for a few seconds.
I mean it preserves the internal state but as I go along filling the form. After few seconds older one got refreshed..

Here are my code:

form.js

import React from 'react'
import Form from 'react-jsonschema-form';

class FixedForm extends Form {

    constructor(props){
        super(props)
    }

    componentWillReceiveProps(nextProps) {
        const shouldSkipUpdate =
            Object.keys(nextProps).length === Object.keys(this.props).length &&
            Object.keys(nextProps).filter(
                key =>
                    nextProps[key] !== this.props[key]
            ).length === 0;
        if (shouldSkipUpdate) {
            return;
        }
        super.componentWillReceiveProps(nextProps);
    }

    render() {
        return <Form {...this.props}/>
    }
}

export default FixedForm

my custom-form.js in components folder

import React from 'react'
import RForm from './form'

export const Form = (props) => {
  return (
    <RForm {...props} widgets={{
      BaseInput: TextInput,
      CheckboxWidget,
      FileWidget: FileUpload,
      SelectWidget
    }}>
      <Spacer />
      <Button submit type="primary">{props.submitText}</Button>
      <Loader loading={props.loading} />
    </RForm>
  )
}

then in my component where I use it, I do

import { Form } from '../../custom-form'

<Form schema={schema} onSubmit={onSubmit} onChange={onChange} submitText="Add" noValidate={true} loading={loading} />

@domercuri
Copy link

Hi, I am having this issue too, is there any update on this bug already ? In the meantime I'll use this workarround :)

@Kos
Copy link

Kos commented Aug 3, 2021

#2010 looks relevant to this and might help with remediation.

@heath-freenome
Copy link
Member

?This may actually be fixed in the v5 beta... Can someone please verify?

@MiguelFreire
Copy link

Nop, still persists check https://codesandbox.io/s/sparkling-cdn-yrmhq5
Type something in the text input and click "OK".

@mikolysz
Copy link

A much simpler workaround is to store the full form data and provide it on each render.

The code looks something like this:

const [formData, setFormData] = useState({});
const onChange = (e) => {
  setFormData(e.formData);
  // Set whatever else you need here.
};

return <Form
  onChange = {onChange}
  formData = {formData}
  // other properties omitted.
/>

Copy link

stale bot commented Dec 28, 2023

This issue has been automatically marked as possibly close because it has not had recent activity. It will be closed if no further activity occurs. Please leave a comment if this is still an issue for you. Thank you.

@stale stale bot added the possibly close To confirm if this issue can be closed label Dec 28, 2023
Copy link

stale bot commented Jan 27, 2024

This issue was closed because of lack of recent activity. Reopen if you still need assistance.

@stale stale bot closed this as completed Jan 27, 2024
Issues - @rjsf/core Legacy project automation moved this from Low priority to Closed Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug possibly close To confirm if this issue can be closed
Development

No branches or pull requests

9 participants