New rule proposal, prefer mockReturnValue over mockImplementation

[geoffswift]

We are seeing examples likes this

// BAD
example.mockImplementation(() => value);

Rather than just

// GOOD
example.mockReturnValue(value);

This seems to be a sound change to make when

• value is a literal primitive e.g. true
• A const primitive value

In more complex values using mockImplementation may be approprate, e.g. the result is deferred or closures a var etc

// OK
example.mockImplementation(() => new Date());

// NOT THE SAME
example.mockReturnValue(new Date());

[G-Rath]

I'm not sure about this - while it seems straightforward in theory, in practice I'm not sure it could ever be safe:

const myMockedThing = jest.fn<number, unknown[]>();

function addFive(): number {
  return myMockedThing() + 5;
}

let value: number;

beforeEach(() => {
  myMockedThing.mockImplementation(() => value);
  // myMockedThing.mockReturnValue(value);
});

it.each([
  [1, 6],
  [3, 8],
  [4, 9]
])('correctly adds 5 to %i', (input, expected) => {
  value = input;

  expect(addFive()).toBe(expected);
});

The only time we could safely cover this would be for the literal case, which you should be able to cover with no-restricted-syntax as literals have their own AST node type

[geoffswift]

If the rule only supported literals that'd be good. Ideal is if we can have it detect a reference of a literal via a const. I can fix up all the literal cases easily enough with a Perl one-liner, but not so much the const variety.

[G-Rath]

oh duh yea course, we should be able to check that to know about mutability 🤦

so yeah some kind of prefer-shorthand-mocks rule could be useful - I think there might also be a possible thing we could do with mockResolvedValue as well (e.g. we could detect async () => value) 🤔

[geoffswift]

Good point about async ()=> value

I've even seen people write this:

example.mockImplementation(() => Promise.resolve(1));

Instead of just:

example.mockResolvedValue(1);

[G-Rath]

Note that there is a slight difference in those which will be important to call out: the first creates an actual native Promise while the second results in a spec-compliant Promise but that is not an instance of the native Promise class, meaning that instanceof Promise will fail.

That's generally very rare as while not outright bad, instanceof especially for globals tends to be easily break, but it does happen from time to time.