Why, Oh Why Was This Added?
Tags: programming, react
Published: 2022-05-26 16:32:03

---
title: Why, Oh Why Was This Added?
published: true
description: In this post, I take you on a debugging journey and show you why it might be a good idea to add a comment next to a certain method.
tags: programming,react
cover_image: https://dev-to-uploads.s3.amazonaws.com/uploads/articles/2faulme42amsm2a052gl.jpg
canonical_url: https://zigamiklic.com/why-oh-why/
---

<!-- wp:paragraph -->
<p>To read this post you will need to use a bit of your imagination.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Suppose you are checking a component called <code>DatePicker</code> 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. </p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>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: <a href="https://developer.mozilla.org/en-US/docs/Web/API/Event/stopPropagation" target="_blank" rel="noreferrer noopener">event.stopPropagation()</a>. That is exactly why you have <code>DatePicker</code> opened - it's using <code>event.stopPropagation()</code> in one of its event handlers:</p>
<!-- /wp:paragraph -->

<!-- wp:group -->
<div class="wp-block-group"><!-- wp:code {"className":"javascript"} -->
<pre class="wp-block-code javascript"><code>function DatePicker() {
  function handleDateChange(event) {
    event.stopPropagation();
  }

  return (
    &lt;ThirdPartyDatePicker
      onChange={handleDateChange}
    />
  );
}</code></pre>
<!-- /wp:code -->

<!-- wp:paragraph {"align":"center","className":"caption","fontSize":"small"} -->
<p class="has-text-align-center caption has-small-font-size">A simplified version of the component</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

<!-- wp:paragraph -->
<p>But, why was that <code>event.stopPropagation()</code> 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.</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2>Debugging Journey</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>When you remove <code>event.stopPropagation()</code>, 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 <code>event.stopPropagation()</code> in your current app version. You also don't see a noticeable difference. You start asking yourself if it serves a purpose at all.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>You then check the Git history and see <code>event.stopPropagation()</code> 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.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>But you are still hesitant to remove it. Surely it was not added for no reason? You don't want to <a href="https://fs.blog/chestertons-fence/" target="_blank" rel="noreferrer noopener">make assumptions</a> and repeat your <a href="https://zigamiklic.com/cautionary-tale/" target="_blank" rel="noreferrer noopener">past mistakes</a>. 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 <code>DatePicker</code> 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 <code>event.stopPropagation()</code> is there. You are hopeful that it will help save time for someone in the future. That future someone might even be you.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>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 <code>event.stopPropagation()</code>. There are still quite a few more in the codebase.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>You move on to the next <code>event.stopPropagation()</code> 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.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>And just like that, we are again at step 1 of our debugging journey.</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2>Backstory</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>Now to give you more context.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>This week I was migrating our app to React 17. I suspect you might be thinking: "React 17? You do know that <a href="https://reactjs.org/blog/2022/03/29/react-v18.html" target="_blank" rel="noreferrer noopener">React 18</a> 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.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Nevertheless, one of the things that React 17 changes is how <a href="https://reactjs.org/blog/2020/10/20/react-v17.html#changes-to-event-delegation" target="_blank" rel="noreferrer noopener">event delegation</a> works. More specifically, React no longer attaches event handlers to the&nbsp;<code>document</code>&nbsp;object but instead attaches them to the root DOM container.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>My plan for migrating to React 17 was to search our codebase for <code>event.stopPropagation()</code> 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 <code>event.stopPropagation()</code> provides. We usually call it at the top of event handlers without any extra info. When checking the code you know <em>what </em>it is doing (preventing event bubbling) but you don't know <em>why</em>.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>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:</p>
<!-- /wp:paragraph -->

<!-- wp:list -->
<ul><li>They prevent triggering events in components nested many levels higher.</li><li>They solve very specific edge cases.</li><li>They prevent triggering events in 3rd party libraries.</li><li>They exist for no reason.</li></ul>
<!-- /wp:list -->

<!-- wp:paragraph -->
<p>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 <code>event.stopPropagation()</code> without including a comment. I write it automatically when the need arises without giving it a second thought.</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2>Conclusion</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>While not commenting your <code>event.stopPropagation()</code> 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.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>So the next time you are writing <code>event.stopPropagation()</code>, 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.</p>
<!-- /wp:paragraph -->