-1

I have a react component that, on load, gets a list of 'issues' from an api endpoint. It then displays them in a table. When an element is clicked, it sends a POST request telling that server that the issue has been resolved. This all works fine, but now, if the POST request returns 200, I need it to remove that issue from the state (and therefore from the table). I've tried the code below, but it says that it can't read the property 'state' of undefined, on the line this.state.issues, suggesting that it doesn't know what 'this' is.

class IssueScreen extends Component {
  constructor(props) {
    super(props);
    this.state = {};
  }
  componentDidMount() {
    fetch(`${apiRoot}/${this.props.match.params.id}`, {
      method: "GET",
      headers: {
        Accept: "application/json",
        "Content-Type": "application/json",
        Authorization: `Token ${this.props.auth.token}`
      }
    })
      .then(response => response.json())
      .then(response =>
        this.setState({
          id: response.id,
          issues: response.issues
        })
      );
  }

  resolveIssue(issueID) {
    fetch(`${apiRoot}resolve_issue/`, {
      method: "POST",
      headers: {
        Accept: "application/json",
        "Content-Type": "application/json",
        Authorization: `Token ${this.props.auth.token}`
      },
      body: JSON.stringify({
        id: issueID
      })
    }).then(function(res) {
      if (res.status === 200) {
        for (var i = 0; i < this.state.issues.length - 1; i++) {
          if (this.state.issues[i].id === issueID) {
            this.setState({ issues: this.state.issues.splice(i, 1) });
          }
        }
      } else {
        console.log("Problem");
      }
    });
  }

The resolveIssue is called by: this.resolveIssue(id); further on in the code.

Alex
  • 2,270
  • 3
  • 33
  • 65

4 Answers4

0

First of all change your function expression into an arrow function, so this will point to the proper namespace.

resolveIssue = (issueID) => {

as well ass:

.then((res) => {

Secondly, Array#splice works in situ, which means that will directly mutate the state. After properly binding your resolveIssue function or just changing it into an arrow function, your app will probably crash due to the mutation state error. I would suggest you to use spread syntax instead.

Something like:

this.setState({ issues: 
    [...this.state.issues.slice(0, i), ...this.state.issues.slice(i + 1)],
});
kind user
  • 40,029
  • 7
  • 67
  • 77
  • That makes sense, thank you, but even with an arrow function I get the same issue. It started to work when I made the function(res) an arrow function too – Alex Dec 14 '18 at 15:26
  • @Alex Just try to use arrow functions generally. It helps to avoid a lot of problems. – kind user Dec 14 '18 at 15:29
  • I'm starting to get that impression ;) I was under the impression that it was just shorthand, I didn't realise it changed the namespace. Lesson learnt! – Alex Dec 14 '18 at 15:29
0

You need to use an arrow function for the this keyword to work in your anonymous function towards the end of your script.

The last part of your code should look like this.

 resolveIssue = (issueID) => {
    fetch(`${apiRoot}resolve_issue/`, {
      method: "POST",
      headers: {
        Accept: "application/json",
        "Content-Type": "application/json",
        Authorization: `Token ${this.props.auth.token}`
      },
      body: JSON.stringify({
        id: issueID
      })
    }).then((res) => {
      if (res.status === 200) {
        for (var i = 0; i < this.state.issues.length - 1; i++) {
          if (this.state.issues[i].id === issueID) {
            this.setState({ issues: this.state.issues.splice(i, 1) });
          }
        }
      } else {
        console.log("Problem");
      }
    });
  }

About arrow functions:

Quoting developer.mozilla.org:

Two factors influenced the introduction of arrow functions: shorter functions and no existence of this keyword.

It further states that arrow functions have no separate this:

Until arrow functions, every new function defined its own this value (based on how function was called, a new object in the case of a constructor, undefined in strict mode function calls, the base object if the function is called as an "object method", etc.). This proved to be less than ideal with an object-oriented style of programming.

An arrow function does not have its own this; the this value of the enclosing lexical context is used i.e. Arrow functions follow the normal variable lookup rules. So while searching for this which is not present in current scope they end up finding this from its enclosing scope . Thus, in the following code, the this within the function that is passed to setInterval has the same value as this in the lexically enclosing function:

function Person(){

      this.age = 0;

      setInterval(() => {
        this.age++; // |this| properly refers to the Person object
      }, 1000);
    }

var p = new Person();

Emphasis in the quoted text from developer.mozilla.org are mine. Hope that helps.

kev
  • 1,011
  • 8
  • 15
0

When you use this inside of a function() {} is references a different context than the one where state is defined. If you were to use an arrow function this would be correct as it 'autobinds'.

You can read more here: https://medium.com/komenco/react-autobinding-2261a1092849

If I could suggest a more elegant solution to removing this item from state (untested).

  if (res.status === 200) {
        this.setState({ issues: this.state.issues.filter(issue => issue.id !== issueID) });
  } else {
    console.log("Problem");
  } 
Geraint
  • 3,314
  • 3
  • 26
  • 41
  • Definitely more elegant, but it seems to remove all issues *except* the one I clicked, so I think it needs inverting. Easily fixed, even by me. Thank you :) – Alex Dec 14 '18 at 15:33
  • Sorry! Yes, exactly that, wrote quickly and didn't validate but glad it helped – Geraint Dec 16 '18 at 20:22
  • It certainly did; and more importantly, taught me something about writing idiomatic JavaScript! – Alex Dec 16 '18 at 20:26
0

Alex.

For this portion of your code:

}).then(function(res) {
      if (res.status === 200) {
        for (var i = 0; i < this.state.issues.length - 1; i++) {
          if (this.state.issues[i].id === issueID) {
            this.setState({ issues: this.state.issues.splice(i, 1) });
          }
        }
      } else {
        console.log("Problem");
      }
    });

... it appears that this is not bound to the correct context. It may be helpful to pull the function contained inside the then out into a class method. Then bind this to your new method inside of the constructor below your state object.

An arrow function would work here; however, I would suggest you don't use an arrow function as others have suggested since you are essentially rebinding context each time you call resolveIssue (which could cause a hit to performance.) Better to pull this function out into a new method and bind this context once in the constructor both for performance and readability if that works for you.

Best of luck.