Recently Double Agents Dale Karp and Josh Justice contributed a new lint rule to eslint-plugin-testing-library, a linting plugin related to React Testing Library (RTL). In a previous post Dale described the new rule and the problem it solves. In this post Josh will talk through the process of identifying the need, building the solution, and getting the contribution accepted.

Neither Dale nor I (Josh) had written a custom ESLint rule before, let alone contributed to a broadly-adopted library. But over the course of a month of pairing once a week we were able to get our rule written and accepted. Hearing about how it went could be valuable to equip you with:

• Tips about how to contribute to open-source software (OSS) in general and to an ESLint plugin rule in particular.
• Suggestions for how to interact with OSS maintainers.
• The confidence that you can successfully make your first major OSS contribution. It worked for us!

Identifying the need

The way I first came up with the idea for this rule was by assumption: I thought it already existed! I had opened a PR, and another Double Agent who was reviewing it saw that I had written:

expect(screen.queryByText('floof')).toBeVisible();

They pointed out that query queries are not recommended for positive checks like .toBeVisible(), and recommended using getByText() instead. (For more detail, see the part 1 blog post.)

I agreed, and I was surprised our existing linting configuration hadn’t caught this issue. We were already using eslint-plugin-testing-library, and I relied on its prefer-presence-queries rule to catch similar mistakes. This rule performs the following checks:

• For matchers that assert presence (.toBeInTheDocument(), .toBeTruthy(), and .toBeDefined()) it ensures that get queries are used.
• For matchers that assert absence (.toBeNull(), .toBeFalsy(), and presence queries negated with a .not) it ensures that query queries are used.

In my mind, .toBeVisible() was a presence query; in fact, it was the best presence query because of its extra assertions that an element is visually present for the user. Nonetheless, .toBeVisible() isn’t one of the matchers that prefer-presence-queries checks—that’s how my code made it past the linter and required a human to find the issue.

Making the proposal

I was surprised that .toBeVisible() wasn’t being checked, and it seemed like a good idea to propose that it should be. As a first step, I opened a GitHub Issue on the eslint-plugin-testing-library GitHub repository to start a conversation.

The plugin has a PR issue template, and one of the questions it asks is “Do you want to submit a pull request to change the rule?” with options “No,” “Yes,” and “Yes, but need help.” This encouraged me that help would be available, so I decided to answer “Yes, but need help.”

Within a few days the maintainer, Mario Beltrán, replied to the issue, agreeing it was a good idea and recommending a small change that would let the existing prefer-presence-queries rule catch this case. I gave that suggestion a quick try, but once I did we discovered a problem: there is an important difference between how.toBeVisible() behaves and how the other matchers behave.

As I mentioned above, for presence checks like .toBeInTheDocument(), get should be used when they are positive checks, but when they are negated query should be used instead. But this isn’t the way that .toBeVisible() works: it requires get for both positive and negative checks. This is because it isn’t actually checking for whether or not an element is present in the document; it assumes the element is present, and checks whether that element is visible or invisible.

Once we discovered this difference, the maintainer had a new recommendation: create a new rule prefer-query-matchers that is very similar to prefer-presence-queries but doesn’t have the logic to switch between get and query for positive and negative versions of the check. Instead, for a given matcher, allow configuring whether it should always be used with get or with query. This would allow us to specify that .toBeVisible() should always appear with get.

Getting help

Now the scope of the change had increased: I had a whole new lint rule to write! Now I felt the need to ask for help, especially because of my unfamiliarity with TypeScript and ESLint.

At the same time, I felt confident that with the help of another developer we would be able to get the rule written. From my initial review of the code it seemed pretty approachable. Also, I had a little experience writing a custom rule for a linting library in a different programming language, so I was familiar with the basic concepts. Plus, I had the active support of the maintainer.

At Test Double, agents have four hours a week of “growth time” available for us to invest in ourselves and/or open source. I wanted to invite another agent to put some of their growth time toward working on this with me—but I wanted to make sure I wasn’t asking too much of them. Following the suggestion of fellow agent Jamie Phelps, I wrote up a description of a “short-term mentorship” where we would work on this contribution together. This proposal included:

• Ways this new lint rule would help the community and support our professional growth and careers.
• A specific, limited time frame (two hours per week for four weeks), so they weren’t obligated to spend time on this indefinitely.
• Clarity that the only thing required was to show up for these pairing sessions: no “homework” was needed.
• A commitment from me that if we didn’t finish in this time frame, I would keep working on it until it was merged. There was no expectation of extra time from them, and they would still get full credit.

I posted this proposal in our company Slack, and within a day, Dale reached out and expressed interest. As he described in his earlier blog post, on his client project at the time he had run across the same issue as I had, so he was eager to get this rule built.

The work

As we started our first pairing session, we decided to follow the common “driver/navigator” style of pairing, rotating every 5 minutes. This format was especially helpful while working on code that we were not very familiar with. For five minutes one of us would brainstorm and decide what we should try, then they would get a break while the other took a turn brainstorming.

We first stepped through the code for the similar prefer-presence-queries rule to try to understand how it worked. This was helpful to figure out just enough to get our bearings.

Next, we had to start writing code. eslint-plugin-testing-library includes a test suite for its lint rules, so we considered writing the tests first. However, that test suite is somewhat indirect so we felt like we would have a smoother time learning by starting with trying to write the production code that we thought would work. This went fairly quickly: we were able to follow the example of the existing rule and make changes where the new rule’s needs differed.

With this knowledge under our belt, we had to circle back and figure out the tests so we could add coverage for our new rule. We eventually stumbled across the ESLint documentation about Custom Rules, which included documentation about the testing approach that the plugin used. We wished we had looked for that from the start!

For the first time in software development history, the estimate was exactly right: after four weeks of two-hour pairing sessions, our code was complete!

As we were cleaning up our commits, we wanted to make sure both of us got visible credit for the work. To accomplish this, we followed GitHub’s instructions on how to create a commit with multiple authors.

Getting it done

With this, we put up a pull request. The good news is, the maintainer posted in response to the PR quickly! However, his response was “I’ll try to review it by the end of the month” (3 weeks away). Now, we totally understood an OSS maintainer needing some time before they could do this work. But we were still nervous: would our hard work on this feature get merged or get stuck in limbo?

In the meantime, the automated CI tasks that ran on our PR reported a few failures. We went ahead and fixed those up so that the PR would be in as ready a state as possible when the maintainer was ready to review.

Sure enough, after three weeks the maintainer came back around and did a review. They had just a few requests for us that were quick to handle.

We were grateful that the maintainer was kind in his feedback all throughout the process. Maintaining a popular OSS project is hard enough work without the additional obligation of always crafting your words carefully. But when a maintainer is able to offer that kind of positive support, it can be really encouraging for new contributors like us.

Once our PR was approved and merged, we were excited to see that the project’s practice was to cut a new release as soon as a new rule is merged. So the day our PR was merged, eslint-plugin-testing-library version 5.11.0 was released with our rule, prefer-query-matchers!

Takeaways

Here’s what we learned through this contribution process:

• When a company gives its software developers time to contribute to open source, this helps them level up their skills, give back to the community, and have a great time. If your employer isn’t allocating time for open source contributions like this, look for a specific need you could contribute and then propose it to your manager. You might get approval to do so, and you might even open the door for others to do the same!
• The level of support you get from an OSS project’s maintainer makes a huge difference in how the process goes. Reach out to them initially and find out if they are available, interested in your contribution, and able to help you out. We want to give a big shout-out to Mario Beltrán for his help on our contribution!
• When you’re taking on a new challenge, it can be easier to get past roadblocks if you can pair up with someone with complementary skills (like Dale’s TypeScript knowledge plus my familiarity with ASTs). When you’re stuck, the other person might know the answer or have different ideas to try.
• When you invite a coworker to volunteer their time to help you with something, put thought and structure into the proposal. This can help people see the commitment required and the benefits they’ll get. If you’re not their boss you can’t make them do anything; instead, paint a clear picture so they can make an informed decision.

If you’ve never contributed to an OSS project before, I hope this story increases your confidence that you can do it. Look for a need and for someone to bring along with you, and take the leap!

Join the conversation about this post on our N.E.A.T community

Not a N.E.A.T. community member yet? More info.