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

componentWillReceiveProps calls previous onChange if onChange prop was updated #1708

Closed
2 of 3 tasks
FunkMonkey opened this issue Apr 18, 2020 · 3 comments
Closed
2 of 3 tasks

Comments

@FunkMonkey
Copy link

Prerequisites

Description

When a form is updated with new props which contain a new onChange callback, the previous onChange callback will be called instead, if the validated formData doesn't deep equal the new formData. This causes problems if onChange is a closure containing important data that changed inbetween.

Steps to Reproduce

  1. Create a form with an onChange callback in the props
  2. Update the form (same key) with a new onChange callback in the props

Reduced Example:

function SomeComponent(props) {
  const onChange = () {
    console.log(props.someValue);
  }
  return <Form onChange={onChange} />
}

Considering someValue is counting up like 1, 2 etc. then the calls to onChange will log 1, 1, 2, 3 etc.

Expected behavior

New onChange callback called.

Actual behavior

Previous onChange callback called.

The problem lies in Form.js#44:

  UNSAFE_componentWillReceiveProps(nextProps) {
    const nextState = this.getStateFromProps(nextProps, nextProps.formData);
    if (
      !deepEquals(nextState.formData, nextProps.formData) &&
      !deepEquals(nextState.formData, this.state.formData) &&
      this.props.onChange // should be nextProps.onChange
    ) {
      this.props.onChange(nextState); // should be nextProps.onChange
    }
    this.setState(nextState);
  }

Version

"react-jsonschema-form": "1.8.1"

@kirkov kirkov mentioned this issue May 16, 2020
7 tasks
@epicfaace epicfaace added this to To do in PRs May 23, 2020
@SivaGanesh56
Copy link

any update on this ?

@DanielHabenicht
Copy link

There is a PR that is not yet merged?

We have the same problem. Anything I could help with unblocking?

@heath-freenome
Copy link
Member

heath-freenome commented Aug 28, 2022

Fixed in v5 beta via #3052, see the 5.x migration guide

Issues - @rjsf/core Legacy project automation moved this from Low priority to Closed Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants