Set prop values in callback

Question

I'm new to React and was wondering whether I took the right approach here.

I have a profile page called <App /> and a lower level component called <ContactDetails />. I wanted to store the state of <ContactDetails /> on <App />, so I only have to write AJAX logic in one place for all components. Is this thinking correct?

And more specifically, I'm interested whether the way I pass event.target.value to the <App /> when the user changes the input is correct?

ContactDetails:

import React from 'react';

class ContactDetails extends React.Component {
  render() {
    return (
      <div>
        <input value={this.props.contactDetails.email} onChange={event => this.props.onContactDetailsChange(Object.assign(this.props.contactDetails, {email: event.target.value}))} />
        <input value={this.props.contactDetails.firstName} onChange={event => this.props.onContactDetailsChange(Object.assign(this.props.contactDetails, {firstName: event.target.value}))} />
      </div>
    )
  }
}

export default ContactDetails;

App:

import React from 'react';
import ReactDOM from 'react-dom';
import ContactDetails from './components/contact_details';

class App extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      contactDetails: {
        email: 'a@a.com',
        firstName: ''
      }
    }
  }

  render() {
    return (
      <ContactDetails
        onContactDetailsChange={contactDetails => this.setState({ contactDetails })}
        contactDetails={this.state.contactDetails}
      />
    );
  }
}

ReactDOM.render(<App />, document.querySelector('.container'));

Is this code not giving you warnings about not changing props directly? — Jeroen Wienk, Aug 06 '16 at 00:12
I have no idea how you can run this. Your contact details component returns 2 elements which are not wrapped in an enclosing tag and I get this error. `Parsing error: Adjacent JSX elements must be wrapped in an enclosing tag`. — Jeroen Wienk, Aug 06 '16 at 00:22
Your overall logic is about right, pass the value back up to App and store it in state. This code should not run though. — David Gilbertson, Aug 06 '16 at 07:15
Ah sorry, I'll update that now, I simplified the code to make it easier to read and forgot to put in a wrapper div. — migu, Aug 06 '16 at 22:10

score 3 · Accepted Answer · edited May 23 '17 at 12:33

This is how I would write your solution:

import React from 'react';

// Class based component handles the logic
class App extends React.Component {
  constructor(props) {
    super(props);

    this.state = {
      contactDetails: {
        email: 'a@a.com',
        firstName: '',
        lastName: '',
        mobile: ''
      }
    }
  }

  // One function to handle input changes
  handleContactDetailsChange = (value) => {
    // Object.assign() first argument is the target object.
    // Object.assign() returns the target object into contactDetails.
    // Second argument are the old contactDetails.
    // Third argument is the new input value. 
    // Because the third argument comes later it overwrites anything from
    // contactDetails that has the same key.
    this.setState({
      contactDetails: Object.assign({}, this.state.contactDetails, { ...value })
    });
    // { ...value } can also be written as just value, but this creates a copy.
  }

  render() {
    return (
     <ContactDetails
      onContactDetailsChange={this.handleContactDetailsChange}
      contactDetails={this.state.contactDetails}/>
    );
  }
}

// Stateless functional component.
// Takes props as an argument.
const ContactDetails = (props) => {
    // Pull of the two props we need from the props object(cleaner syntax).
    const { onContactDetailsChange, contactDetails } = props;
    return (
     <div>
       <input
        value={contactDetails.email}
        onChange={event => onContactDetailsChange({ email: event.target.value })}
       />
       <input
        value={contactDetails.firstName}
        onChange={event => onContactDetailsChange({ firstName: event.target.value })}
       />
     </div>
    )
};

export default App;

ContactDetails would normally be in it's own file. Also you modified the props by calling Object.assign(this.props.contactDetails, {firstName: event.target.value})since the target is this.props.contactDetails. The React philosophy is that props should be immutable and top-down.

Didn't forget, ES6 arrow functions have this behavior built in. Else calling this.setState() would not even work in my code. — Jeroen Wienk, Aug 06 '16 at 07:10
Indeed. My eyes slipped over the class property with the arrow function :) — Ori Drori, Aug 06 '16 at 07:18
for performance reasons, react recommends 'this' binding at constructor method `this.handleContactDetailsChange = this.handleContactDetailsChange.bind(this);` Documentation Reference: https://facebook.github.io/react/docs/reusable-components.html#no-autobinding is using fat arrow syntax as used in your example has the same performance benefit? — Galeel Bhasha, Aug 06 '16 at 15:15
I think what they mean with that is that if you bind them in your render method they will get bound again on every render and in the constructor only once. Since the arrow function belongs to the class and is not in render I think there will not be any performance difference. — Jeroen Wienk, Aug 06 '16 at 18:06

Set prop values in callback

1 Answers1