🥰 Hello! Long time no see! I am currently on maternity leave with my second child and writing code is basically impossible to do right now, especially with the COVID situation preventing us from taking advantage of childcare.
However, I did have a small amount of free time recently and I decided to spend it fixing an issue reported on an open source tool I maintain. And then, as a companion to my Debugging in JavaScript for Beginners: How I approach things article I wrote, I thought I’d write this little real-life walkthrough of a bug I fixed.
I wrote this one-handed while simultaneously patting a baby's bottom rhythmically in an attempt to get her to sleep, so please forgive any quality issues 😳
The library
I don’t do a ton of side-coding outside my job but I do have a small library that converts content from the Draft.js rich text editor to markdown and vice versa.
The issue
Someone opened an issue on the GitHub repo reporting that a link at an end of a block of text was not being converted correctly.
The issue was first reported in May -
When people put effort into reporting issues or opening pull requests I like to try to acknowledge them somehow, even if it might take me awhile to follow up in any concrete way -
The approach
When I actually went to dig in to this issue, my first step was to try to re-create it. My repo has a gh-pages branch that is just a demo of the code in master running (you can see it here)
My attempt to re-create failed. I tried a simple block of text with a link at the end and it converted correctly:
I tried a few different things (inside a blockquote perhaps?) but no luck re-creating. I returned to the issue and left a comment asking for more details, as well as sharing what I had tried.
In case you are wondering: how did I manage to get the “raw drafts object” from the page I was testing on?
I have defined a few tools on the window so that I can quickly access them from the console. That, combined with the React Developer Tools extension:
Allowed me to grab the data using my browser's console tool.
Here's the code-snippet where I define some useful methods on window so I can quickly use them -
Here's me making sure that the right component is selected so I can access it with $r in the console:
And here I am getting the editor state and using the convertToRaw method so that I can copy and paste it into the GitHub issue:
OK so that's all great but I still don't know how to re-create the bug. So now I just wait to see if I get a reply. If I had tons of time I could be more proactive and keep trying different things, but as noted: I have no time 🤭
Luckily, a few days go by and I get this:
Here is where being a more experienced developer has some advantages, and I apologize to beginners that I don't have some super magical advice here except that, "Huh I have seen bugs like this before."
I actually remember an old boss and mentor (hi Jason Webster) sending me JavaScript has a Unicode problem many years ago and it has been... a very useful article in my career. I guess I deal with a lot of strings at my work? haha.
It's a great article and I totally recommend you read the whole thing, but for our purposes: Basically the issue is that sometimes if you have a string in JavaScript that contains certain unicode characters, it doesn't "count" them the way you might expect. From the article:
>> '💩'.length // U+1F4A9 PILE OF POO
2
As the article explains, a way around this issue is to use Array.from
>> Array.from('💩').length // U+1F4A9 PILE OF POO
1
So somewhere in my code, I theorized, I was falling into this .length trap.
OK so now we're onto something!
Solving the bug
My issue reporter handily included an example of a failing case, so I decide to write a new test with this failing case. That way, when I run the tests I will first see it fail, and once I have fixed it, I will see it pass. A really gratifying way of knowing that you have fixed the bug!
And here's the test failing:
Next up: Opening draft-to-markdown.js, which is where all the conversion code exists. I can tell by the appearance of the failed test that the closing tag of the final link is what is failing. It just dies off after the opening tag and the link text.
Error: Expected '🙋 [link](https://link.com) [link' to equal '🙋 [link](https://link.com) [link](https://link.com)'
So I look through the code to see where links are closed. I actually haven't worked on this project in ages so I kind of forget how it all fits together. I see this chunk of code:
var EntityItems = {
'LINK': {
open: function open(entity) {
return '[';
},
close: function close(entity) {
return "](".concat(entity.data.url || entity.data.href, ")");
}
}
};
"Hmm ok," I think, "So there's the snippet that handles link open/close."
So I do a file search for EntityItems[ to see every spot where it is being used and see if I can find a place that looks like a likely culprit.
Again, being an experienced developer probably helps here, but this stood out to me as I went through the search results:
// Close any remaining entity tags
block.entityRanges.forEach(function (range, rangeIndex) {
if (range.offset + range.length === block.text.length) {
var entity = rawDraftObject.entityMap[range.key];
if (customEntityItems[entity.type] || EntityItems[entity.type]) {
markdownString += (customEntityItems[entity.type] || EntityItems[entity.type]).close(entity);
}
}
});
// Close any remaining inline tags (if an inline tag ends at the very last char, we won't catch it inside the loop)
The comment is helpful too:
Close any remaining entity tags
and the one beneath, which is actually for a different bit of code but still provides context for the part I was working on:
// Close any remaining inline tags (if an inline tag ends at the very last char, we won't catch it inside the loop)
HMM this is code specifically for the last item, and it references EntityItems and .close AND I see a spot where text.length is being used directly. Could this be it???
I make a small change, can you spot it?
// Close any remaining entity tags
block.entityRanges.forEach(function (range, rangeIndex) {
if (range.offset + range.length === Array.from(block.text).length) {
var entity = rawDraftObject.entityMap[range.key];
if (customEntityItems[entity.type] || EntityItems[entity.type]) {
markdownString += (customEntityItems[entity.type] || EntityItems[entity.type]).close(entity);
}
}
});
OK let's run my test again:
NAILED IT.
I was lucky, this bug fix went so amazingly smoothly. Trust me, they are not all like that.
My only final step was to write some nice commits explaining the cause of the bug and open a pull request to fix it:
You can see the pull request right here
THAT IS IT!
Thank you for reading! I have to run now, if you notice any major errors or typos or whatever, please comment and I'll try to fix. Like I said, I threw this together under a bit of pressure, so there's bound to be a few things 💕
Top comments (2)
Your well-written explanation will help me learn how to write useful tests regardless of programming language, and for that I can't thank you enough. My inability to get this right, if done at all, has bit me uncountably numerous times.
Thank you for taking the time to write this up 😄