HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Updating state with React

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withupdatingreactstate

Problem

I am creating a blog using React, MongoDB, Express and Node. I have three components: App, List, and Item. The item is a blog post; the list is a list of the blog posts, and the app includes a place to enter text and submit it. I will eventually add more functionality, but I want to determine if I am adhering to best practices for React (I doubt I am).

So in App, I getInitialState with an array of posts (posts) and a string of text for the input (postbody). I used the componentDidMount to make an AJAX GET request to my database, so the user can see all the posts.

To handle entering text I just made a simple handleChange function which updates the state of postbody.

I also have a handleClick function, which grabs this.state.postbody and then POSTs it database. However the same function also makes a separate GET request of the database to update the state of the posts array. This doesn't seem right! Shouldn't that be handled some other way and updated automatically? This is the primary question I have.

Also, please let me know if I need to break the components down further, or if I am violating best practices using React (e.g. changing state in the wrong place, or using props incorrectly).

```
var React = require('react');

var Item = React.createClass({
render: function() {
return (

{this.props.postbody}

)
}
})

var List = React.createClass({

render: function() {
return (

{this.props.array.map(function(post) {

return (

)
})}

)
}
})

var App = React.createClass({

getInitialState: function() {
return {
posts: [],
postbody: ''
}
},

componentDidMount: function() {
$.ajax({
type: 'GET',
url: '/api/blogPosts',
success: function(data) {
this.setState({posts: data});
}.bind(this)
})
},

handleClick: function() {

event.preventDefault();

var blogPost = this.sta

Solution

I'll start from what I perceive as "top" and work my way down.


  
    
    Submit
  
  


You'd probably want to abstract the ` into its own component. You can then expose a submit event, essentially just a callback handed down to a prop on the component. Your form component will call that callback, passing in the details you need for your AJAX.

Additionally,
List and array don't really tell me anything. What kind of list is it? What's in the array? Better name them properly before you start scratching heads and lose hair.

Also,
doesn't need a closing tag.

// Better

  
  


componentDidMount: function() {
    $.ajax({
        type: 'GET',
        url: '/api/blogPosts',
        success: function(data) {
            this.setState({
                posts: data
            });
        }.bind(this)
    })
},

handleClick: function() {

    event.preventDefault();

    var blogPost = this.state.postbody;

    $.ajax({
        type: 'POST',
        url: '/api/blogPosts',
        data: {
            postbody: blogPost
        }
    });

    $.ajax({
        type: 'GET',
        url: '/api/blogPosts',
        success: function(data) {
            this.setState({
                posts: data
            });
        }.bind(this)
    })
},


I see that your GET request is redundant here. You can put that in a separate function, say
loadPosts. Your functions don't really tell me anything either. Sure, they're handling a click, or respond to a mount but what are they doing?

Also, I see you're using jQuery. POST and GET requests have shorthand versions,
$.post and $.get respectively.

When you handle the click, you call two AJAX operations in sequence. That sequence isn't guaranteed, especially when network latency is introduced. By the time you launched the GET, the POST might not have finished yet, and your GET return an incomplete list. It could also be that your POST succeeded, but
GET failed. Usually, a POST request returns an updated version of the resource. Use that to just patch in the new data instead of retrieving the entire list.

Also, it's recommended you use the Promise interface of jQuery operations. tl;dr: use
then instead of success, error, done or fail.

Since you're on React, I assume you use a transpiler setup. You can take advantage of new ES6 syntax, like arrow functions, shorthand literal assignment, shorthand literal functions

componentDidMount(){
  this.loadPosts();
},
onSubmit(post){
  this.submitPost(post);
},
loadPosts(){
  return $.get('/api/blogPosts').then(posts => this.setState({posts}));
},
submitPost(postBody){
  return $.post('/api/blogPosts', {postBody}).then(post => this.state.posts.push(post));
}


Now if your API can't handle returning the updated resource from a POST, you can simply call your GET after the POST suceeds.

submitPost(postBody){
  $.post('/api/blogPosts', {postBody})
   .then(() => this.loadPosts())            
}


var List = React.createClass({

  render: function() {
    return (
      
        {this.props.array.map(function(post) {

          return (
            
          )
        })}
      
    )
  }
})


Usually when I see nested functions, I just move out the function into the class instead of inline. Much cleaner, and less visual clutter. Also,
Item doesn't really tell me what it is. What kind of item is this? If this is a post, then it should be named Post or BlogPost`.

var List = React.createClass({
  createPost(post){
    return 
  },
  render() {
    return (
      
        {this.props.array.map(this.createPost)}
      
    )
  }
});

Code Snippets

<div>
  <form action="/api/blogPosts" method="post">
    <input onChange={this.handleChange} type="text" name="postbody"></input>
    <button type="button" onClick={this.handleClick}>Submit</button>
  </form>
  <List array={this.state.posts} />
</div>
// Better
<div>
  <PostForm onSubmit={this.handleSubmit} />
  <Posts posts={this.state.posts} />
</div>
componentDidMount: function() {
    $.ajax({
        type: 'GET',
        url: '/api/blogPosts',
        success: function(data) {
            this.setState({
                posts: data
            });
        }.bind(this)
    })
},

handleClick: function() {

    event.preventDefault();

    var blogPost = this.state.postbody;

    $.ajax({
        type: 'POST',
        url: '/api/blogPosts',
        data: {
            postbody: blogPost
        }
    });

    $.ajax({
        type: 'GET',
        url: '/api/blogPosts',
        success: function(data) {
            this.setState({
                posts: data
            });
        }.bind(this)
    })
},
componentDidMount(){
  this.loadPosts();
},
onSubmit(post){
  this.submitPost(post);
},
loadPosts(){
  return $.get('/api/blogPosts').then(posts => this.setState({posts}));
},
submitPost(postBody){
  return $.post('/api/blogPosts', {postBody}).then(post => this.state.posts.push(post));
}
submitPost(postBody){
  $.post('/api/blogPosts', {postBody})
   .then(() => this.loadPosts())            
}

Context

StackExchange Code Review Q#111250, answer score: 4

Revisions (0)

No revisions yet.