no-useless-undefined: undefined removed from jest function matchers

[danut-t]

test('function called with different parameters that could be undefined', () => {
	expect(someFunction).toHaveBeenCalledWith(1, 2, 3);
	expect(someFunction).toHaveBeenCalledWith(1, undefined, 3);
	expect(someFunction).toHaveBeenCalledWith(1, 2, undefined);
});

This is breaking as jest toHaveBeenCalledWith (and other similar matchers) explicitly checks for nth argument to be undefined

Would be better if this rule would accept and array of "acceptable" functions that have undefined?

[silverwind]

I'd say you better disable the rule in that case. Generally, there are always cases where explicit undefined makes a difference. Even some builtin functions sadly have it:

> new Date()
2020-08-25T07:28:34.351Z
> new Date(undefined)
Invalid Date

[papb]

Maybe the rule could be automatically disabled for files in which the 'jest' environment is enabled?

[silverwind]

The issue has nothing to do with jest. Any function that checks the length of its arguments or is implemtend in native code like the Date constructor can be affected and which ones are can probably not be statically determined.

[papb]

I know, but it would be a way to at least lower the amount of false-positives. I agree that false positives would still exist.

[papb]

Wait, actually it seems @fisker already did this in #758

Edit: it was done with an allowlist of methods and toHaveBeenCalledWith was not one of them.

[fisker]

Yap, the simplest way to fix this is add this name to that list.

[fisker]

If this issue is about toHaveBeenCalledWith only, we can add this to the compareFunctionNames list, PR welcome.

If it's about arguments.length, we can continue discussion in #760

[silverwind]

Essentially it's a unsolvable problem but I guess a allowlist of common usages of undefined is a bandaid fix that might ease the pain in some cases. Still I have the rule disabled and would not recommend enabling it 😉.

[fisker]

What do you prefer?

• an option to ignore CallExpression
• a configurable list

Or better solution?

[silverwind]

Neither, I would personally just put a eslint-disable-line comment I guess. But I'm probably not the right person to ask this question.

[BPetronel]

The changes made in #758 were to add the missing cases to allow-list,so adding a configurable list might be useful for new checker aliases or test runners too.