React: Is this a good way to use props and states

Question

I finally got a responsive setup that actually works. But I would very much like to get some feedback on whether this a good setup and data flow.

I made the following:

App.js

import React, { Component } from 'react';
import './App.css';
import AB_eval from './components/AB_eval';

class App extends Component {
  constructor() {
    super();
    this.updateDB = this.updateDB.bind(this);
    // getInitialState
    this.state = {
      DB: []
    };
  }

  updateDB(k, bar) {
      const DB = {...this.state.DB}
      DB[k] = bar;
      this.setState({ DB });
  }

  render() {
    return (
      <div className="App">

        <AB_eval A={2} B={3} updateDB={this.updateDB} />

      </div>
    );
  }
}

export default App;

AB_eval.js

import React, { Component } from 'react';
import Slider from 'react-rangeslider';

class AB_eval extends Component {
  constructor(props, context){
    super(props, context);

    this.updateDB = this.updateDB.bind(this);
    this.refreshAB = this.refreshAB.bind(this);

    let {A, B} = this.props;
    let AB = A * B;
    this.state = {
      A: A,
      B: B,
      AB: AB
    }
  }

  componentDidMount () {
    this.updateDB();
  }

  refreshAB () {
    const AB = this.state.A * this.state.B;
    this.setState({
      AB: AB
    });
  }

  updateDB () {
    const bar = {
        A: this.state.A,
        B: this.state.B,
        AB: this.state.AB
      }
    this.props.updateDB(0, bar) // updating the App's state
  }

  render() {
    let {A, B, AB} = this.state;
    return (
      <div>
        <h1>AB_eval: {AB}</h1>


        <h2>A</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={A}
          onChange={ (value)=> {
            this.setState({
              A: value
            });
            this.refreshAB()
            this.updateDB() }
          }
        />

        <h2>B</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={B}
          onChange={ (value)=> {
            this.setState({
              B: value
            });
            this.refreshAB()
            this.updateDB() }
          }
        />
      </div>
    )
  }
}

AB_eval.propTypes = {
  A: React.PropTypes.number.isRequired,
  B: React.PropTypes.number.isRequired,
  updateDB: React.PropTypes.func.isRequired
};

export default AB_eval;

Next up is to add a C slider - so it will become an ABC_eval. But first I need to know:

• Are the data flow between App and the AB_eval a good approach?

• Would it make sense to break things down to more components?

---

Update

I realised that using a setState inside the AB_eval component wasnt such a good idea after all when looping through the state.

I updated the code to the following, but it isn't working.. think Im missing a couple of things..

dummy-data.js

module.exports = {
  one:  {A: 1,
      B: 2,
      AB: 2
      },
  two: { A: 3,
        B: 4,
        AB: 12
      }
};

App.js

import React, { Component } from 'react';
import './App.css';
import AB_eval from './components/AB_eval';
import dummyData from './dummy-data';

class App extends Component {
  constructor() {
    super();
    this.updateDB = this.updateDB.bind(this);

    // getInitialState
    this.state = {
      DB: dummyData
    };
  }

  updateDB(key, bar) {
      const DB = {...this.state.DB}
      DB[key] = bar;
      this.setState({ DB });
  }

  render() {
    return (
      <div className="App">
        <ul className="list-of-ABs">
            { Object
              .keys(this.state.DB)
              .map(key =>
              <AB_eval
                key = {key}
                ID = {key}
                Data = {this.state.DB[key]}                
                updateDB={this.updateDB} />
                )
            }
          </ul>
      </div>
    );
  }
}

export default App;

AB_eval.js

import React, { Component } from 'react';
import Slider from 'react-rangeslider';

class AB_eval extends Component {
  constructor(props, context){
    super(props, context);

    this.updateDB = this.updateDB.bind(this);

    // Trying to make these variable instance variables
    const {ID, Data} = this.props;

  }

  updateDB () {
    this.props.updateDB(this.ID, this.Data) // updating the App's state
  }

  render() {
    console.log(this.Data);
    let {A, B, AB} = this.Data;
    return (
      <div>
        <h1>AB_eval: {this.ID}</h1>

        <h2>A</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={A}
          onChange={ (value)=> {
              this.A = value;
              this.AB = this.A * this.B;
              this.updateDB();
            }
          }
        />

        <h2>B</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={B}
          onChange={ (value)=> {
              this.B = value;
              this.AB = this.A * this.B;
              this.updateDB();
            }
          }
        />
      </div>
    )
  }
}

AB_eval.propTypes = {
  ID: React.PropTypes.string.isRequired,
  Data: React.PropTypes.object.isRequired,
  updateDB: React.PropTypes.func.isRequired
};

export default AB_eval;

(not sure if I use the const and let at the right places)

---

SOLUTION

With help from @Pineda and some trying I found the following solution to work as I wish:

scr/App.js

import React, { Component } from 'react';
import './App.css';
import ABEval from './components/ABEval';
import dummyData from './dummy-data';

class App extends Component {
  constructor() {
    super();
    this.updateDB = this.updateDB.bind(this);

    // getInitialState
    this.state = {
      DB: dummyData
    };
  }

  updateDB(key, bar) {
      const DB = {...this.state.DB}
      DB[key] = bar;
      this.setState({ DB });
  }

  render() {
    return (
      <div className="App">
        <ul className="list-of-ABs">
            { Object
              .keys(this.state.DB)
              .map(key =>
              <ABEval
                key={key}
                ID={key}
                Data={this.state.DB[key]}
                updateDB={this.updateDB} />
                )
            }
          </ul>
      </div>
    );
  }
}

export default App;

scr/dummy-data.js

module.exports = {
  one:  {A: 1,
      B: 2,
      AB: 2
      },
  two: { A: 3,
        B: 4,
        AB: 12
      }
};

scr/components/ABEval.js

import React, { Component } from 'react';
import XSlider from './XSlider';

class AB_eval extends Component {
  constructor(props, context){
    super(props, context);
    console.log(`${this.props.ID}: Constructed`);
  }

  componentDidMount(){
    console.log(`${this.props.ID}: Mounted`);
  }

  render() {
    console.log(`${this.props.ID}: rendered`)
    const { Data, ID } = this.props;
    const { A, B, AB } = Data;
    return (
      <div>
        <h1>ABEval: {ID}</h1>
        <p>A: {A} B: {B} AB:{AB}</p>

        <XSlider
          title={'A'}
          value={A}
          valueHandler={
            (val)=> this.props.updateDB(ID, {A: val, B: B, AB: val*B} )}
        />

        <XSlider
          title={'B'}
          value={B}
          valueHandler={
            (val)=> this.props.updateDB(ID, {B: val, A: A, AB: val*A} )}
        />
      </div>
    )
  }
}

AB_eval.propTypes = {
  ID: React.PropTypes.string.isRequired,
  Data: React.PropTypes.object.isRequired,
  updateDB: React.PropTypes.func.isRequired
};

export default AB_eval;

scr/components/XSlider.js

import React from 'react';
import Slider from 'react-rangeslider';

export default ({title, value, valueHandler}) => {
  return (
    <div>
      <h2>{title}</h2>
      <Slider min={1} max={4} step={1} value={value} onChange={valueHandler} />
    </div>
  );
}

Answer 1

Honestly speaking, I can't understand what your code is for but from the code samples and your questions here is what I can think of:

dummyData.js

export default {
  one:  {A: 1,
    B: 2,
    AB: 2
  },
  two: { A: 3,
    B: 4,
    AB: 12
  }
};

App.js

import React, { Component } from 'react';
import './App.css';
import dummyData from './dummyData';
import AB_eval from './components/AB_eval';

class App extends Component {
  constructor() {
    super();

    // getInitialState
    this.state = dummyData;

    this.updateDB = (id, value) => this.setState({ id: value });

  }

 render() {
   return (
    <div className="App">
      <ul className="list-of-ABs">
          { Object
            .keys(this.state)
            .map(key =>
              <AB_eval
                key = {key}
                ID = {key}
                Data = {this.state[key]}            
                updateDB={this.updateDB} />
            )
          }
        </ul>
     </div>
    );
  }
}

export default App;

If you are going to have a key: value storage only in your app's state then you don't need that DB key, just set the state to be a key: value storage directly like above this.state = dummyData, I think this way the state tree would be less deep and esier to think about and operate on it.

You can use arrow functions here to "bind" this like here this.updateDB = (id, value) => this.setState({ id: value });. This way you don't need first define a function and then "bind" this into it. It is less code and less code is often esier to think about I guess.

AB_eval.js

import React, { Component } from 'react';
import Slider from 'react-rangeslider';

class AB_eval extends Component {

  constructor(props, context){
    super(props, context);

    this.updateDB = (type, value) => {
      const { ID, DATA } = this.props
      switch(type) {
        case "A":
          this.props.updateDB(
            ID, 
            { ...DATA, A: value, AB: (value * DATA.B) }
          );
          break;
        case "B":
          this.props.updateDB(
            ID, 
            { ...DATA, B: value, AB: (DATA.A * value) }
          );
          break;
        default:
          this.props.updateDB(
            ID, DATA
          );
          break;
      }
    };

  }

  componentDidMount () {
    this.updateDB();
  }

  render() {
    const {A, B, AB} = this.props.DATA;
    return (
      <div>
        <h1>AB_eval: {AB}</h1>

        <h2>A</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={A}
          onChange={ (value)=> this.updateDB("A", value) }
          }
        />

        <h2>B</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={B}
          onChange={ (value)=> this.updateDB("B", value) }
        />
      </div>
    )
  }
}

AB_eval.propTypes = {
  ID: React.PropTypes.string.isRequired,
  DATA: React.PropTypes.object.isRequired,
  updateDB: React.PropTypes.func.isRequired
};

export default AB_eval;

The first think I could think of about your AB_eval component that it doesn't need a state of it's own. Because all of it's state's contents are availble through the props. As it is said on the React documentation you should:

Figure out the absolute minimal representation of the state your application needs and compute everything else you need on-demand.

Putting state in one place, usually in the component which is on the top of the component hierarchy, makes it a lot esier to comprehense about your app.

I think this awsome material on the React's documentation site can help you a lot when you want to create an app in React.

Here is a good tutorial about const and let here on youtube. But in short, use const whenever you don't need to change the variable. Think about it as a constant variable. And it is a good practice to define a value as a constant with const, so you don't change it accedentally. Use let only when you need to change the variable later.

Answer 2

With the information you've provided it's difficult to make many re-factoring recommendations but the data flow looks ok.

Issues within your latest AB_eval.js:

const {ID, Data} = this.props;  // block scoped, DOES NOT create class members
                                // referencable by this.ID or this.DATA
                                // outside the scope of its containing block

const is block scoped so your destructuring of props within the constructor will only create a useable value of ID and Data within the constructor block.

This breaks later references of this.ID and this.Data (in the updateDB() method). References to this.A, this.AB, this.B and this.updateDB in the render() method will also be broken. To fix this, I suggest destructuring the props within the block scope of your render and onChange handler(s).

this.AB = this.A * this.B;  // state should be treated as immutable
                            // and since props are propogated by changes in state
                            // mutating them is ill-advised

Attempting to set values on this.props.Data directly within the onChange methods which is effectively mutating what should be treated as immutable.

There are a couple ways you can ensure no mutations using ES6/ES7 notation, one is using Object.assign and the other is using the Spread syntax notation.

Solution

AB_eval.js:

import React, { Component } from 'react';
import Slider from 'react-rangeslider';

class AB_eval extends Component {
  constructor(props, context){
    super(props, context);

    this.onChangeA = this.onChangeA.bind(this);
    this.onChangeB = this.onChangeB.bind(this);
  }

  onChangeA (value) {
    const {ID, Data, updateDB} = this.props;
    updateDB(ID, {
      ...Data,
      A: value,
      AB: value * Data.B
    });
  }

  onChangeB (value) {
    const {ID, Data, updateDB} = this.props;
    updateDB(ID, {
      ...Data,
      B: value,
      AB: value * Data.B
    });
  }

  render() {
    const {A, B, AB} = this.props.Data;
    return (
      <div>
        <h1>AB_eval: {this.props.ID}</h1>

        <h2>A</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={A}
          onChange={this.onChangeA}
        />

        <h2>B</h2>
        <p>A: {A} B: {B} AB:{AB}</p>
        <Slider min={1} max={4} step={1}
          value={B}
          onChange={this.onChangeB}
          }
        />
      </div>
    )
  }
}

AB_eval.propTypes = {
  ID: React.PropTypes.string.isRequired,
  Data: React.PropTypes.object.isRequired,
  updateDB: React.PropTypes.func.isRequired
};

export default AB_eval;

• The single updateDB method was replaced with two onChange handlers (onChangeA and onChangeB respectively) to handle the slider cases for A and B.
• the props were destructured accordingly within these handlers as well as within the render function, notice the use of the spread syntax in order to create the updated an object that doesn't mutate the existing this.Data.props
• The onChange handlers no longer use the fat arrow notation you had before so it is necessary to bind both the handlers to this.

---

UPDATE Regarding refactoring:
You could extract the Slider code out into a functional component (I've added a folder structure just as a suggestion):

./components/AB_eval_slider.js

import React from 'react';

export default ({key, db, handler}) => {
  return (
    <div>
      <h2>{key}</h2>
      <p>A: {db.A} B: {db.B} AB:{db.AB}</p>
      <Slider min={1} max={4} step={1} value={key} onChange={handler} />
    </div>
  );
}

So you'd also have to edit my suggested AB_eval.js to include:
import AB_eval_Slider from './components/AB_eval_slider';

and the render method would now be:

render() {
  return (
    const { Data } = this.props;
    <div>
      <h1>AB_eval: {this.ID}</h1>
      <AB_eval_Slider key={'A'} db={Data}, handler={this.onChangeA} />
      <AB_eval_Slider key={'B'} db={Data}, handler={this.onChangeB} />
    </div>
  );
}

Answer 3

In your example, it is not necessary to exec updateDB in App component. You can just do the updateDB operation in AB_eval. And you can init the A,B in your AB_eval constructor instead of getting from App component, since they are constants.