The issue has been solved

Rule proposal: No deepEqual for primitives values #158

jfmengels posted onGitHub

I see a lot of of uses of t.deepEqual that compares to a promitive value when a simple t.is(a, b) or t.true(a === b) would work.

t.deepEqual(value, 'foo');

I'm not sure if there is a big perf hit or something, but I think it's better to use a simpler form of assertion whenever possible. I personally like to use t.is() or t.true(===) instead (frankly, most of my assertions now are t.true())

Invalid

t.deepEqual(expression, 'foo');
t.deepEqual(expression, 1);
t.deepEqual(expression, `foo${bar}`);
t.deepEqual(expression, null);
t.deepEqual(expression, undefined);
t.notDeepEqual(expression, undefined);
t.deepEqual(1, expression); // ?

Valid

t.deepEqual(expression, otherExpression);
t.deepEqual(expression, {});
t.deepEqual(expression, []);
t.notDeepEqual(expression, []);

As for the rule name, I think this only targets deepEqual and notDeepEqual, so I propose no-simple-deep-equal or something equivalent (but would love a better name for it).

Sounds good. Using t.is()/t.not() instead also makes the intent clearer.

I couldn't think of a better name. Only thing that comes to mind is no-incorrect-deep-equal.

Could this be auto-fixable?

posted by sindresorhus over 8 years ago

that sounds like a really easy fix, just change the name of the method) to is/not.

Rule name open to bikeshed for those interested :)

posted by jfmengels over 8 years ago

I wanted to add my 2p. with regards of t.true. It's is bad because wrong value does not get reported:

Having said that, it would be nice if t.deepEqual was automatically replaced with t.is and backwards — it is very important to consider the real user's workflow.

Namely. Users have gajillion of unit tests, it's already tedious to visually grep them. Now, if linter could do optimisation switching deepEqual to is and backwards, AUTOMATICALLY for me, I'd appreciate that. If it will just nag me, I refuse to enable such rule.

Like Sindre said, definitely, let's do auto-fixable. And very important, both ways. Detect comparison to "object-type", switch to deepEqual. Detect compared-to value being number or Bool, switch to t.is.

What do you think about backwards (is -> deepEqual) scenario?

posted by revelt about 6 years ago

It's is bad because wrong value does not get reported:

Your example doesn't use variables though, I think the values would be displayed, though to your point not as nicely as with t.is().

What do you think about backwards (is -> deepEqual) scenario?

👍

posted by novemberborn about 6 years ago

@issuehunt has funded $60.00 to this issue.

Submit pull request via IssueHunt to receive this reward.
Want to contribute? Chip in to this issue via IssueHunt.
Checkout the IssueHunt Issue Explorer to see more funded issues.
Need help from developers? Add your repository on IssueHunt to raise funds.

posted by IssueHuntBot almost 6 years ago

@sindresorhus has rewarded $54.00 to @mrhen. See it on IssueHunt

:moneybag: Total deposit: $60.00
:tada: Repository reward(0%): $0.00
:wrench: Service fee(10%): $6.00

posted by issuehunt-app[bot] over 5 years ago

Comparing regular expressions with is() makes my tests fail.

const ext2regexp = require('ext-to-regexp');

const jsx = ext2regexp('js', 'jsx');

t.deepEqual(jsx, /\.(js|jsx)$/); // Pass

t.is(jsx, /\.(js|jsx)$/);
// Fail: Values are deeply equal to each other, but they are not the same

posted by Airkro over 5 years ago

@Airkro but it's expected, jsx like you defined is one object, the regex you're comparing it with is another object. Just like ["z"] !== ["z"], same thing, is is strict === comparison.

posted by revelt over 5 years ago

@revelt I know, but this rule keep telling me that I should use is().

posted by Airkro over 5 years ago

@Airkro Open a new issue about this. I guess we can add support for detecting t.deepEqual(jsx, /\.(js|jsx)$/); and t.is(jsx, /\.(js|jsx)$/); in the https://github.com/avajs/eslint-plugin-ava/blob/master/docs/rules/prefer-t-regex.md rule.

posted by sindresorhus over 5 years ago

We shouldn't recommend t.is() when a regular expression is passed to t.deepEqual() though. That'd be a bug. @Airkro could you open a new issue for this?

posted by novemberborn over 5 years ago

#270 sure

posted by Airkro over 5 years ago

Fund this Issue

$60.00

Rewarded