Skip to content

@testing-library/react-hooks specific rules #371

Open
@mpeyper

Description

@mpeyper

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    new ruleNew rule to be included in the pluginpinnedPinned for different reasons. Issues with this label won't be flagged as stale by stalebot

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions