CODEWITHSUNDEEP
reactjs
felix Zhou
felix Zhou

Reputation: 3

How to fix "TypeError: Cannot read property 'comment' of undefined"

I'm writing a timer to auto-update the posted time. The code works well without this._timer, and the app crushed with "TypeError: Cannot read property 'comment' of undefined" when I set the timer up.

I tried this._timer = setInterval(this._updateTimeString.bind(this), 3000); instead of this._timer = setInterval(this._updateTimeString, 3000);, it works but I don't know why.

componentWillMount() {
  this._updateTimeString();
  this._timer = setInterval(this._updateTimeString, 3000);
}

_updateTimeString() {
  const now = Date.now();
  const duration = (now - this.props.comment.createdTime) / 1000;
  const timeDuration =
    duration > 60
      ? duration >= 60 * 2
        ? duration > 60 * 60
          ? duration >= 60 * 60 * 2
            ? duration > 60 * 60 * 24
              ? duration > 60 * 60 * 24 * 2
                ? `${Math.round(duration / (60 * 60 * 24))} days ago`
                : `${Math.round(duration / (60 * 60 * 24))} day ago`
              : `${Math.round(duration / (60 * 60))} hours ago`
            : `${Math.round(duration / (60 * 60))} hour ago`
          : `${Math.round(duration / 60)} mins ago`
        : `${Math.round(duration / 60)} min ago`
      : `${Math.round(Math.max(duration, 1))} s ago`;
  this.setState({
    timeString: timeDuration
  });
}

Upvotes: 0

Views: 253

Answers (4)

Sławomir Ossowski
Sławomir Ossowski

Reputation: 84

You are calling out of context, use arrow function:

componentWillMount() {
  this._updateTimeString();
  this._timer = setInterval(() => this._updateTimeString(), 3000);
}

But first of all nested ternary is EVIL. Use something else. Simple if's or something like:

function formatTime(duration) {
  switch(true) {
    case duration > 60 * 60 * 24:
      return 'days'
    case duration > 60 * 60:
      return 'hours'
    case duration > 60:
      return 'mins'      
    case duration > 1:
      return 'secs'            
    default:
      return ''
  }  
}

or whatever but not nested ternary.

Upvotes: 2

Sławomir Ossowski
Sławomir Ossowski

Reputation: 84

For your binding concerns, check this out:

new class A {
  constructor() {
    setInterval(this.tick, 1000)
    setInterval(() => this.tick(), 1000)
  }

  tick() {
    console.log(this)
  }
}()

The difference is in context of invocations. Basically by passing this.tick into setInterval you pass a pointer to the function body itself not the pointer to object method and therefore inside tick() this points to where setInterval is being executed which is Window object.

enter image description here

Remember JS passes function parameters by value not by pointer - even if that parameter is function its being passed over by value i.e. function body is being copied not a pointer so this will not get "copied" with it look:

let A = {
  me: 'A',
  whoAmI: function () { return `I am ${this.me}` }
}

console.log(A.whoAmI())
// prints "I am A"
// so method was invoked inside A context
// ( this points to A )

function someFunction( callback ) {
  return callback()
}

console.log( someFunction(A.whoAmI) )
// "I am undefined"
// so method was invoked out of context
// ( this points to undefined )
// this is because argument passed a function body
// not a method pointer

// to simulate passing argument by pointer
// we have to pass another function that
// invokes A.whoAmI within A context
// we do that by defining new function that actually calls A.whoAmI()
// and pass it over
console.log( someFunction(() => A.whoAmI()) )
// "I am A"

A.whoAmI.bind(A) is just another syntactic form of () => A.whoAmI()

Upvotes: 0

bflemi3
bflemi3

Reputation: 6790

this isn't bound to the component instance in _updateTimeString. In your constructor bind the method to the instance.

class MyComponent extends Component {
    constructor(props) {
        super(props);

        this._updateTimeString = this._updateTimeString.bind(this);
    }
}

And if you don't like the way that looks, here's some alternatives. Enjoy!

Upvotes: 1

Vencovsky
Vencovsky

Reputation: 31625

You can use .bind but I recommend using arrow functions.

Just change:

_updateTimeString() {

To:

_updateTimeString = () => {

Upvotes: 0

Related Questions