How I Fixed a Bug in Angular

Photo by Caspar Camille Rubin on Unsplash

Hello, everyone! Today I’m going to tell you how I’ve contributed to the Angular community. You will learn about the contribution process to a large open-source project and I’ll shed light on the code review. Let’s get started!

Issue

In my previous job, we had a long-lived SPA application, which the user may keep open for a very long time. And at some point, we noticed that some Observable of HTTP request was never completed.

Sad Observable, that is never completed

After doing some small amount of research, we discovered that Angular doesn’t respond to outer XMLHttpRequest.abort() in the case when the browser cancels HTTP requests itself. That situation occurs when the user turns off the device to sleep mode or presses Ctrl+S to save a web page. We even found the appropriate issue. And someone had already made a pull request (PR) to fix this bug. We wrote a workaround with XhrFactory inside our project, expecting that the fix would be released.

But when I checked the fix status six months later, I found out that the issue was still open. And the PR was canceled since the author didn’t update it. Then I decided: “This is my chance to try open-source developing.”

Contribute process

First of all, I read through Angular’s contributing guidelines, which you can find in the COUNTIBUTING.md file. Here is a list of what you need to do so that the Angular team will consider your PR:

1. Search GitHub for an open or closed PR that is related to your submission.
2. Fork the project.
3. Create a new branch based on “master”.
4. Write your code and at least one test to it following Code Style.
5. Run the full Angular test suite.
6. Commit your changes with the right message.
7. Sign Contributor License Agreement (CLA).
8. Create a PR to the “master” branch of Angular.

I guess the first three points are clear. We’re going to start with the fourth point. This one was easier for me, because there was the canceled pull request, which needed improving. Check it out.

We have a problem with the requests to the backend. So, we need to check the http module. Quickly find the right package. As we know, the issue is related to xhr and abort, therefore search inside the http module where XMLHttpRequest is created and how the abort event is handled. Find the xhr.ts file and understand that we’ve hit the mark.

Structure of http module

Look for an event listener for abort. Bingo! This event isn’t handled.

We just need to add an event listener for abort, then an Observable will be completed with an error if the HTTP request fails. And don’t forget to write a test for this. There is the onError function, which we could use, inside xhr.ts already. It takes only two rows.

Two rows of changes

I have another idea that it is possible to handle HTTP abort such as HttpEvent instead of an error. Then our fix would be more complicated. But in my opinion, a situation when forces beyond your control have decided your request is no longer going to continue is rather an error.

The code, which fixes the bug, is written. After that we need to implement unit tests on these changes for one simple purpose — if the functionality of the http module is changed, everything will work. Every file in the src directory has its own file with .spec.ts extension for tests in the test directory. The algorithm is very easy — find a similar test and change it. I also had to extend one mock.

The test

The tests have been written. Now we need to run all the project tests. It may take quite a while. Be prepared.

While we are writing tests we can check a separate module, it will be faster. For example:

yarn bazel test packages/common/http/test

All the tests are run. It’s time to commit our changes. Before this, I recommend doing yarn lint, if you don’t apply clang automatic formatter. Code style is the base thing.

And we can shift to our commit. Angular team has strict rules for commit message naming. So, we carefully read through the whole section about this in contributing guidelines.

Now we’re publishing our branch. And we are at the finish line. We need to sign CLA with Google. It allows a contributor to keep their rights on code and Google will have legal rights to use that code. Signing CLA is required only one time and it spreads to all Google’s projects.

Make PR and wait, when the Angular team will review it.

Code review

After doing some research, I was prepared to wait for a long time. But my pull request was reviewed after 13 minutes! It was absolutely amazing to get such fast feedback.

Andrew Kushnir approved my PR and said that my changes looked good to him. He also added Alex Rickabaugh as a reviewer, because Alex had more context. He approved after 30 minutes and wrote that he had had some concerns originally, but he had convinced himself that this PR was good to go.

However, that wasn’t the end. It turned out that there was another similar pull request that fixed the issue with xhr timeout, and the code was identical to mine. Therefore, we would have merge conflicts when that PR would be landed. Andrew asked me to rebase my PR and resolve merge conflicts, when the time came. I did my best and pushed my changes with the--force-with-lease flag. You can read more about this flag here.

The next day Andrew Kushnir added the “merge” label for my pull request and gave me thanks for my contribution. And a few hours later, my changes were in Angular.

After some days CHANGE_LOG.md was updated that the bug had been fixed and would be released in version 12.

Summary

It was an extremely interesting experience. It was curious to see the structure of the framework on which I’ve written for a long time, run tests, and find out what the processes look like in the Angular team. As it turned out, making your own changes to the open-source framework is not difficult at all.

I hope you have found this useful. Thank you for reading.

More content at plainenglish.io