testing-library / testing-library/eslint-plugin-testing-library

@testing-library/react-hooks specific rules

Open
#371 9 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

new rule pinned
Dominant language
TypeScript
Stars
1.1k
Forks
168
Avg merge
15h 44m
Merged PRs (30d)
14

Description

I'd like to propose a new rule for a testing-library that I do not believe is currently covered in this plugin. I'm not sure if it would be part of testing-library/react rules or if a new set of rules (testing-library/react-hooks) should be introduced. Reading this, it's very possible that people will be using both @testing-library/react and @testing-library/react-hooks in the same project and want to have it in the same eslint config, so not sure how it would be best structured.

There is a common mistake people make when using @testing-library/react-hooks that should be pretty simple to lint for. The issue looks like this:

const { result } = renderHook(() => useCounter());

const { value, increment } = result.current;

expect(value).toBe(0);

act(() => {
  increment();
})

expect(value).toBe(1);

Assuming useCounter works as intended, this test would fail because they have destructured result.current.value, effectively locking its value to whatever it was when initially rendered.

(To be clear, people generally aren't testing the value before the act they are just asking why it's wrong at the final expect, but I wanted to be clear about where it goes wrong)

The test should be written as:

const { result } = renderHook(() => useCounter());

expect(result.current.value).toBe(0);

act(() => {
  result.current.increment();
})

expect(result.current.value).toBe(1);

This allows result.current.value to be updated when the internal component renders.

This logic is true for increment as well as this test would also fail because increment is using a stale closure for the update:

const { result } = renderHook(() => useCounter());

const { increment } = result.current;

expect(result.current.value).toBe(0);

act(() => {
  increment();
})

expect(result.current.value).toBe(1);

act(() => {
  increment();
})

expect(result.current.value).toBe(2);

There would be very few cases where destructuring any value from result.current is needed. The only one that I can think of is if you wanted to capture the initial value to test that the new value was different (e.g. a randomly generated value), but in my experience, this is not very common. However, if the test does not perform any updates, the test could still pass, therefore making the destructuring technically valid, but in those cases, inlining result.current would also work.

Let me know what you think, if it's possible and if I can help in any way towards implementing it. I've never worked on an eslint plugin before, but I'm willing to learn.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the existing React rules and the shareable-configuration documentation mentioned in the issue, then determine how @testing-library/react-hooks should be represented. Define the rule’s behavior around destructuring from result.current and cover the updated and non-updated hook examples with tests. Done means the intended stale-value mistake is reported without unnecessarily rejecting valid cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
eslint, javascript, react, typescript
Domain
testing-qa, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.