Why, Oh Why Was This Added?

#programming #react

To read this post you will need to use a bit of your imagination.

Suppose you are checking a component called DatePicker that is built on top of a 3rd party date picker component. You are tasked to find if the component is causing problems now that you have updated another project dependency.

It is a larger update, so there are some breaking changes. From the migration guide, you even know exactly what can be the cause of the issues: event.stopPropagation(). That is exactly why you have DatePicker opened - it's using event.stopPropagation() in one of its event handlers:

function DatePicker() {
  function handleDateChange(event) {
    event.stopPropagation();
  }

  return (
    <ThirdPartyDatePicker
      onChange={handleDateChange}
    />
  );
}

A simplified version of the component

But, why was that event.stopPropagation() added? To know if the migration broke something, you first have to find the reason why it exists in the first place. Taking a sip of your coffee, you embark on your debugging journey.

Debugging Journey

When you remove event.stopPropagation(), you don't see any difference. All the tests are passing. You check the 3rd party date picker project/documentation if there is a specific reason but return empty-handed. You then revert all the migration changes and remove event.stopPropagation() in your current app version. You also don't see a noticeable difference. You start asking yourself if it serves a purpose at all.

You then check the Git history and see event.stopPropagation() is there since the initial version and there is no info about it in the commit message. Another dead end. Finally, you check who implemented it. Maybe they know some secret you don't. Maybe it's something super obvious that you are missing. Finally, they will solve this mystery and you will be able to go to bed peacefully tonight! Well, it turns out the original author no longer works at the company.

But you are still hesitant to remove it. Surely it was not added for no reason? You don't want to make assumptions and repeat your past mistakes. So you search your app where the component is used and try to spot any special use cases. And after a while you do. In one use case (out of 100+) the DatePicker component is placed inside another component in which preventing event bubbling is required. To make sure you don't forget, you quickly add a comment explaining why event.stopPropagation() is there. You are hopeful that it will help save time for someone in the future. That future someone might even be you.

After that, you are finally able to test if the migration is causing an issue and fix it if necessary. And so you come at the end of your debugging journey. Well, sort of. You come to the end for this particular event.stopPropagation(). There are still quite a few more in the codebase.

You move on to the next event.stopPropagation() and see that this one also does not include a comment. You read the surrounding code but again don't know why someone added it.

And just like that, we are again at step 1 of our debugging journey.

Backstory

Now to give you more context.

This week I was migrating our app to React 17. I suspect you might be thinking: "React 17? You do know that React 18 is already out, right?" I am solely to blame here. I had concerns with React 17 breaking another dependency and postponed the update several times.

Nevertheless, one of the things that React 17 changes is how event delegation works. More specifically, React no longer attaches event handlers to the document object but instead attaches them to the root DOM container.

My plan for migrating to React 17 was to search our codebase for event.stopPropagation() and then test the UI to make sure everything was working correctly. But while I was going through each use case, I realized just how little information event.stopPropagation() provides. We usually call it at the top of event handlers without any extra info. When checking the code you know what it is doing (preventing event bubbling) but you don't know why.

Some use cases were obvious by looking at the surrounding code but for a lot of them, it took me a while to figure it out. These ambiguous use cases fell into four categories:

They prevent triggering events in components nested many levels higher.
They solve very specific edge cases.
They prevent triggering events in 3rd party libraries.
They exist for no reason.

Please note that I am not writing this post to point fingers at my coworkers. I have written this post primarily for myself as I have also written many times event.stopPropagation() without including a comment. I write it automatically when the need arises without giving it a second thought.

Conclusion

While not commenting your event.stopPropagation() calls is usually not a big deal, it may be a problem in situations like mine where I had to go over many ambiguous use cases at once. This resulted in a lot of wasted time that could be otherwise avoided if the original author added a comment.

So the next time you are writing event.stopPropagation(), stop and think if adding a comment where you explain why it is there would benefit a future reader. Your colleagues or your future self will be grateful that you did.