References to methods are a JavaScript minefield

So, tell me what’s wrong with the following code:

//               ms  s  m  h
const ONE_DAY = 1000*60*60*24
const ONCE_A_DAY = ONE_DAY

class Certificate {
  #renewalDate = null

  constructor() {
    // Renewal is in thirty days.
    #renewalDate = new Date(Date.now() + ONE_DAY*30)

    // In a real app, you’d save the returned interval id
    // and clear it when you no longer need it.
    setInterval(this.checkForRenewal, ONCE_A_DAY)
    checkForRenewal()
  }

  async checkForRenewal() {
    console.log('Checking if certificate needs to be renewed…')
    const currentDate = new Date()
    if (currentDate >= this.#renewalDate) {
      await this.renewCertificate()
      console.log('Certificate renewed.\n')
    } else {
      console.log('Certificate does not need renewing yet.\n')
    }
  }
}

Nothing?

What if I told you it crashes when it tries to check for renewal the second time?

Not very obvious is it?

Checking if certificate needs to be renewed…
Certificate does not need renewing yet.

Checking if certificate needs to be renewed…
(node:3035) UnhandledPromiseRejectionWarning: TypeError: Cannot read private member #renewalDate from an object whose class did not declare it

(You might want to set the duration to something lower, like 3 seconds, if you don’t want to wait around for a day to see the error for yourself.) :)

References to methods in JavaScript are a runtime minefield

So the problem is on this line in the constructor:

setInterval(this.checkForRenewal, ONCE_A_DAY)

Can you see it yet?

(I don’t blame you, if not.)

Do as I think, not as I say

Debugging is the subtle art of reconciling what you think you asked a computer to do with what you actually asked a computer to do.

What we think we’re doing here is passing a reference to the checkForRenewal() method on the Certificate class to the setInterval() function.

But what we’re actually doing – because method references are not closures in JavaScript – is passing a reference to the checkForRenewal() function to setInterval().

What’s the difference? Oh, about one crash.¹

But wait a minute, if that’s the reason, isn’t the error message confusing? Yes, yes it is. (It’s so confusing that I’m writing this revised blog post because of it. – thanks, Tom!)

The red herring

Let’s remember what the error says:

TypeError: Cannot read private member #renewalDate from an object whose class did not declare it

This makes it sound like a private member called #renewalDate exists on the this reference but you don’t have permission to access it when actually, it doesn’t.

The actual issue

Let’s remove the use of private fields to make things clearer and easier to understand.

Remove all the octothorpes from the code and run it again, adding a console.log(this) statement to the start of the checkForRenewal() method.

When you run it, you should see something similar to the following when the first the renewal check is run as a method call:

Certificate { renewalDate: 2021-03-09T17:36:40.821Z }

Right, so that’s what we expect. But the second time we see any console output, we’re running the unbound function reference, not the method, so we get the following for this:

Timeout {
  _idleTimeout: 3000,
  _idlePrev: null,
  _idleNext: null,
  _idleStart: 3057,
  _onTimeout: [AsyncFunction: checkForRenewal],
  _timerArgs: undefined,
  _repeat: 3000,
  _destroyed: false,
  [Symbol(refed)]: true,
  [Symbol(kHasPrimitive)]: false,
  [Symbol(asyncId)]: 2,
  [Symbol(triggerId)]: 1
}

So our initially unbound function reference has been bound by the setInterval() method to a Timeout instance.

Also note what we don’t get: an error.

As far as our code is concerned, the certificate does not need renewing. This is because of the implicit type conversions JavaScript performs:

// Our original conditional
currentDate >= this.renewalDate

// Becomes:
Number(currentDate) >= Number(this.renewalDate)

// Becomes:
Number(currentDate) >= NaN // which is always false.

So without the use of private class fields, our code just fails silently (the silent killer).

Things are slightly better when you try to access a private class field from the callback function since at least it fails. But, sadly, we get a misleading error message that makes it sound like our initial unbound function reference was actually a closure all along.

The fix

The fix, simply, is to make sure you bind the function reference to its instance:²

setInterval(this.checkForRenewal.bind(this), ONCE_A_DAY)

Now, I can bet that the number of times you’re going to write that code out of the gate without getting bitten by this bug is exactly zero. Even after you’ve read this.³

But maybe you’ll remember this post and catch yourself after you’ve written it and fix it.

And it also shows you private class fields do help by at least not failing silently, even if the error they do fail with is confusing.

Stay safe out there, kids! :)