2
\$\begingroup\$

I am presently working on refactoring ReactJS code away from directly manipulating this.state and towards using this.setState() only (which I should have been doing from the beginning).

The contents of this.state in my application include containers with varying numbers of objects. They are entirely JSON-serializable, but I realized I found myself doing something a lot that I don't recall seeing in the ReactJS documentation (hence the request for a code review). I'm interested in knowing, besides general code quality, how close it is to ReactJS idiomatic usage and how it might be moved closer.

The clone() method I have now is intended to provide a deep clone of JSON-serializable objects, without guarantees as to objects that can't be serialized:

var clone = function(original) {
    if (typeof original === 'undefined' ||
      original === null ||
      typeof original === 'boolean' ||
      typeof original === 'number' ||
      typeof original === 'string') {
      return original;
    }
    if (Object.prototype.toString.call(original) === '[object Array]') {
      var result = [];
      for(var index = 0; index < original.length; index += 1) {
        result[index] = clone(original[index]);
      }
    } else {
      for(var current in original) {
        var result = {};
        if (original.hasOwnProperty(current)) {
          result[current] = original[current];
        }
      }
    }
    if (typeof original.prototype !== undefined) {
      result.prototype = original.prototype;
    }
    return result;
}

What I find myself doing repeatedly is of the form (here using classical form Hijaxing to add a new entry):

handle_submit: function(event) {
      event.preventDefault();
      var entry_being_added = clone(this.state.entry_being_added);
      entry_being_added.month = parseInt(jQuery('#month').val());
      entry_being_added.date = parseInt(jQuery('#date').val());
      entry_being_added.year = parseInt(jQuery('#year').val());
      if (jQuery('#all_day').is(':checked')) {
        entry_being_added.all_day = true;
      } else {
        entry_being_added.all_day = false;
      }
      // Additional assignments omitted for the sake of brevity.
      var entries = clone(this.state.entries);
      entries.push(entry_being_added);
      this.setState({'entries': entries});

I'm not sure I've seen this approach in the ReactJS documentation, and I wanted to ask for a sanity check at least. Is it idiomatic to clone container types, modify the clone, and call this.setState() on the modified clone? Am I deviating from idiom in another way, perhaps by having container types in this.state? Or is this spot on?

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$
  • React is all about avoiding side-effects as well as rendering the same state exactly the same every time. You'd want to avoid operations that mutate the existing data structure like push or object assignment.

  • I'm pretty sure your clone is to prevent your object from being mutated. Instead of clone, create a function that returns your object structure. Just pass in anything that's configurable.

  • Instead of mutating the entries array with push, create a new array with your new data added using concat.

  • Use refs to fetch values from inputs instead of jQuery.

Your code could have been summarized into something like this:

function createNewEntry(month, date, year, allDay){
  return {
    month: month,
    date: date,
    year: year,
    allDay: allDay
  };
}

handleSubmit: function(event){
  var month = React.findDOMNode(this.refs.month).value;
  var date = React.findDOMNode(this.refs.date).value;
  var year = React.findDOMNode(this.refs.year).value;
  var allDay = React.findDOMNode(this.refs.allDay).value;
  var newEntry = createNewEntry(month, date, year, allDay);
  var entries = this.state.entries.concat(newEntry);

  this.setState({'entries': entries});
}

If you happen to have interest in persistent data structures (immutable data), then have a look at immutable.js. It's recommended in cases where you don't want to unwantedly mutate data.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.