Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change deepEqual (actual: ValueType, expected: ValueType) to (any, any) #2580

Closed
mesqueeb opened this issue Sep 14, 2020 · 6 comments · Fixed by #2704
Closed

change deepEqual (actual: ValueType, expected: ValueType) to (any, any) #2580

mesqueeb opened this issue Sep 14, 2020 · 6 comments · Fixed by #2704

Comments

@mesqueeb
Copy link
Contributor

ava/index.d.ts

Line 125 in f328a69

<ValueType = any>(actual: ValueType, expected: ValueType, message?: string): void;

Because I run my Ava tests in ts-node, Ava has type checking on assertion functions.
Mostly it's OK, but the type checking done by deepEqual always gets in the way.

There are so many cases where the the two objects I'm comparing are practically the same data-wise, but Type-wise the type of one is not the same as the other.

Just one example:
image

Or another:
image

You can see the types being passed are clearly the same, but somehow DeepEqualAssertion is throwing errors:
image

I believe TypeScript works best if it doesn't get in the way, and one way to solve a lot of issues I always face with deepEqual is to just simply change the function's signature to:

// instead of:
<ValueType = any>(actual: ValueType, expected: ValueType, message?: string): void; 

// do this:
(actual: any, expected: any, message?: string): void; 

I don't see any harm in this and I believe that we don't need the currently strict type checks there are in place on deepEqual. Why don't we let deepEqual be only about checking the data, not the types. That's my humble request. 🙃

@mesqueeb mesqueeb changed the title change (actual: ValueType, expected: ValueType) to (any, any) change deepEqual (actual: ValueType, expected: ValueType) to (any, any) Sep 14, 2020
@sindresorhus
Copy link
Member

Makes sense to me. However, I think it should be object to ensure people don't accidentally don't try to compare primitives.

@tymfear
Copy link
Contributor

tymfear commented Sep 14, 2020

For me this makes code analysis less reliable. It makes sense that you should compare objects with the same types, otherwise they are not equal anyway.

@novemberborn
Copy link
Member

This might be similar to what @papb was bringing up in #2575. I'm reluctant though, the expected value should be structurally the same as the actual value, and it's nice to get auto-completion and type checking for it.

What I tend to do when I get stuck with the type inference in tests is cast to any. I think that strikes the right balance.

@papb
Copy link
Contributor

papb commented Sep 26, 2020

Hi @novemberborn, I totally agree with you. I wouldn't like to lose autocompletion and type safety on this. When I want to assert that two things are deep equal, if TypeScript can know ahead of time that they don't even have the same time, I prefer it to fail at compile time than at runtime.

@papb
Copy link
Contributor

papb commented Sep 26, 2020

Hi @mesqueeb, can you post a minimal example that represents your situation in which the types "are the same" but TypeScript thinks they are not? I couldn't understand only from your screenshots. To me, if two objects don't have the same type, then clearly t.deepEqual will fail, so I don't see this as "TypeScript getting in the way". Instead, TypeScript is helping you letting you know ahead of time that the test will fail.

@novemberborn
Copy link
Member

For #2456 I'm changing this to:

<Actual, Expected extends Actual>(actual: Actual, expected: Expected, message?: string): actual is Expected;

@novemberborn novemberborn self-assigned this Mar 7, 2021
novemberborn added a commit that referenced this issue Mar 7, 2021
Update the `deepEqual`, `like`, `false`, `true` and `is` assertions so
they can be used as type guards. Fixes #2456.

Change the `deepEqual`, `like` and `is` assertions so the actual and
expected parameters can be typed separately. Fixes #2580.
novemberborn added a commit that referenced this issue Mar 7, 2021
Update the `deepEqual`, `like`, `false`, `true` and `is` assertions so
they can be used as type guards. Fixes #2456.

Change the `deepEqual`, `like` and `is` assertions so the actual and
expected parameters can be typed separately. Fixes #2580.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants