0

This is not a question as much "how to make this work" as much as it is a "was this the best way." Here's my code:

/**
 * React Static Boilerplate
 * https://github.com/koistya/react-static-boilerplate
 * Copyright (c) Konstantin Tarkus (@koistya) | MIT license
 */

 import React, { Component } from 'react';
 // import './InputWidgetText.scss';
 import ContentBlock from '../ContentBlock';


 var i = 0;
 var contentBlocks = [];

 var ContentContainer = React.createClass({

   addNewBlock: function(){
     i++;
     contentBlocks.push(<ContentBlock key={i} index={i}/>)
     this.forceUpdate();
   },
   render: function(){

     if (this.props.inputs) {
       contentBlocks = this.props.inputs.map(function(item, index){
        i++;
        return(<ContentBlock key={index} index={index} content={item} />)
     });

     }
     return (
       <div>
       {contentBlocks}
       <button onClick={this.addNewBlock}>+</button>
       </div>
       )

   }
 });


 export {ContentContainer as default};

The problem is that every so often on a refresh the props.inputs are not getting passed down to this component and throwing an error when I try to map undefined. So the simple solution is to put the map process in an if check for whether or not the props are there yet - is that actually the right way to handle this? My data is passed in via a reflux mixin on the parent. I just feel like there might be a more proper way to handle this. Thanks for the feedback!

Davin Tryon
  • 66,517
  • 15
  • 143
  • 132
motleydev
  • 3,327
  • 6
  • 36
  • 52
  • 1
    You can specify the propTypes on each react component and specify it as `required`. That way, you don't need to check the prop types or if they exist or not. You can read more about it here: https://facebook.github.io/react/docs/reusable-components.html – Pavlin Nov 24 '15 at 08:48
  • Thanks! That worked. I had to follow the same logic up the chain of parents but then it all worked. Now I just need to get reflux to do a better job sending the data! – motleydev Nov 24 '15 at 09:33

1 Answers1

0

May I strongly suggest you refactor your code to do away with the file variables i and contentBlocks.

The contentBlocks variable seems completely unnecessary, whilst your i variable should be part of the state. Whilst you're at it, give i a more meaningful name, e.g. blockCount.

getInitialState: function () {
  return {
    blockCount: 0
  };
},

Then define your click event handler to modify the state:

addNewBlock: function () {
  this.setState({
    blockCount: this.state.blockCount + 1
  });
},

Every time you call setState(), React will trigger a re-render. You should never need to call forceUpdate().

Finally, your render() function should return its content based SOLELY on this.props and this.state. That is, for any given props and state, the output will be predictable. Think of this.props and this.state as input parameters to the render() function. That is all render() can, or needs to, know about.

I won't try to write the render() function as I'm not sure exactly what you're trying to achieve with this component. But for a given this.props.input and this.state.blockCount (or whatever you choose to use as props and state) you should know exactly what you're outputting.

I know I haven't directly answered the question you put, but I hope this clarifies some React concepts.

David L. Walsh
  • 24,097
  • 10
  • 61
  • 46
  • well, since I was looking for best practice and you're the only one to write the response as an answer, I guess credit goes to you. Thanks for the refactor tips. – motleydev Nov 25 '15 at 15:00