Misusing optional chaining

Optional chaining is easy to misuse and can potentially confuse future readers of your code in certain scenarios.

screenshot of code snippet showing optional chaining usage

The optional chaining operator is a fairly recent addition to the javascript syntax. This operator makes it easier for devs to express chained nullable references in a much concise manner. For example, instead of:

if(bookStore && bookStore.bookShelf && bookStore.bookShelf.book) {...} 

You can simply write

if(bookStore?.bookShelf?.book) {...}

This syntax is objectively much easier to write, and arguably easier to read and understand.

All these benefits aside, after using it for some time, I have come to realize that though it is easy to write, it is liable to be misused / abused in not-so-easy-to-observe ways, and the easy syntax leads to a subtle shift in thought process which may lead to more bugs than the earlier syntax.

Disclaimer: These observations are mostly based on my team’s usage of optional chaining with React components.

Consider this React component:

function Book({ book }) {
  return (
    <div classname="detail">{book.name}</div>
    <div classname="detail author-name">{book.author.name}</div>
    <div classname="detail author-contact">{book.author.details.contact}</div>
  );
}

Consider that sometime later you come to know that the author info can be undefined. And if author information is not available in the book object, it would break the UI, as the component is currently written.

To fix this, some devs would end up doing:

function Book({ book }) {
  return (
    <div classname="detail">{book.name}</div>
    <div>{book.author?.name}</div>
    <div classname="detail author-contact">{book.author?.details?.contact}</div>
  );
}

Do you see any problems with the above?

And because optional chaining is so cheap, some devs would even end up doing:

function Book({ book }) {
  return (
    <div classname="detail">{book?.name}</div>
    <div classname="detail author-name">{book?.author?.name}</div>
    <div classname="detail author-contact">{book?.author?.details?.contact}</div>
  );
}

Because why not, it works the same, doesn’t it? And it may even handle some more cases where the book object itself might be undefined, without breaking the code.

Please note that this is more likely to happen when the information that the author object can be undefined, is an afterthought, and possibly not known / missed at the time the component was first created.

Well, both of the above approaches have issues unfortunately.

Let’s discuss the first approach:

function Book({ book }) {
  return (
    <div classname="detail">{book.name}</div>
    <div>{book.author?.name}</div>
    <div classname="detail author-contact">{book.author?.details?.contact}</div>
  );
}

The code would no more break, if the author info is not available, no author details would be shown on the screen, which is great, BUT:

• the author details div would still be rendered (with empty content), the styles would still be applied to the div, and although the code is not breaking anymore, the UI state might be broken / incorrect.

So what? you would surely test the UI after you make changes to the component, and when testing for the failure case, it will be visible to you that the UI is broken, and hence you would fix it. Ok, agreed.

Let’s see a potentially better approach.

Again, some devs would still fix it like this:

function Book({ book }) {
  return (
    <>
      <div classname="detail">{book.name}</div>
      {book.author ? (
        <>
          <div classname="detail author-name">{book?.author?.name} </div>
          <div classname="detail author-contact">{book?.author?.details?.contact}</div>
        </>
      ) : null}
   </>
  );
}

Although the behaviour / UI state is now correct, notice the optional chaining inside the check for book.author, it is not required, and now it is just confusing to future readers.

Anyway, let’s assume that the classes that were being applied to the author details divs, did not break the UI, or the changes were unnoticeable. The dev would not notice that the approach is potentially incorrect, and move on.

They might move on to the next step, to be safe against future code breaking, (who knows if tomorrow the book object itself may be undefined ):

function Book({ book }) {
  return (
    <div classname="detail">{book?.name}</div>
    <div classname="detail author-name">{book?.author?.name}</div>
    <div classname="detail author-contact">{book?.author?.details?.contact}</div>
  );
}

In this case, ignoring the issues that we have already discussed in the previous approach, the code would not break, it would behave the same with or without the optional chaining on the book object, and say, even the UI does not break, and the applied classes do not have any impact, there is still a subtle bug here.

The problem here is in the thought process and the future understanding of this component. In future, once some other dev picks up this component, they might notice that book prop is optional, but the UI handling for book not being available is not satisfactory, may be it would be better to show some empty state UI , instead of not showing anything at all, so they go back to the designer / product to understand what should be done here, and then possibly they would get to know that the book prop would never be undefined. So, even though the UI works alright, the functionality works alright, just adding a simple optional chaining on the book prop by the previous dev would lead to unnecessary cognitive load on the future devs (note that this is just one prop in a small component here, this can very easily snowball in larger components with multiple props, and multiple levels of nesting). In its current form, the code does not correctly exhibit the intended possible states of the UI, just by reading the UI, rather it makes it more confusing.

For some, this might not be a big deal, but working with this in a team of about 15 devs for some time now, we have realized that this happens more often than not. Personally, I have observed that about 5–10% of my PR reviews, are still about fixing this understanding of optional chaining. We have even created a separate code convention rule for optional chaining usage, just to make newer devs aware of this.

Summarizing my understanding

With the older syntax, the test condition was explicit

{ book && book.author && 
  <>
   <div> {book.author.name} </div>
   <div> {book.author.contact} </div>
  </>
}

with the newer syntax, it is much easier / likely for devs to actually do this:

  <>
   <div> {book?.author?.name} </div>
   <div> {book?.author?.contact} </div>
  </>

instead of this:

{ book?.author && 
  <>
   <div> {book.author.name} </div>
   <div> {book.author.contact} </div>
  </>
}

Due to the easier to use syntax, some devs have actually started using book?.author? as a replacement of the usage ofbook.author instead of the test condition if(book && book.author) . some devs consider optional chaining as a mechanism to handle the nullable values in-place, and not as a replacement of the test condition. I actually think that the easier to use syntax seems inexpensive, has no special cognitive load for the developer that introduces it, and so, it inadvertently encourages the incorrect usage.

The easier to use syntax has led to a subtle shift in thought process and devs consider adding extra optional chaining to be a no-op, which is not true as we have seen above.