fix(propEq): improve propEq typings

[Nemo108]

• remove unnecessary constraint from propEq value: the function should receive any type values of object, no matter what parameter it received;
• add additional types to use with placeholder

[Harris-Miller]

remove unnecessary constraint from propEq value: the function should receive any type values of object, no matter what parameter it received;

I'm afraid I have to disagree with this. It is not unnecessary at all. It is important for type safety. With the current implementation, you get this level of safety

import { propEq } from 'ramda';

const doesFooEq123 = propEq(123, 'foo');
//    ^? (obj: Record<'foo', number>) => boolean

// now consider how this gives a type error
'123' === 123; // TypeError: This comparison appears to be unintentional because the types 'string' and 'number' have no overlap.

// With the current typing, you get a similar error
doesFooEq123({ foo: '123' }); // Type 'string' is not assignable to type 'number'.

That error at the end would not happen with your updated, and that's not something that we should loosen

Playground: https://tsplay.dev/wgBX9W

add additional types to use with placeholder

The added currying support with Placeholder is fine, however there are a few situation that don't make sense to me, I'll leave inline comments for those

[Harris-Miller]

I looked at your example and added an additional test:

type Obj = { key: 'A' | 'B' };

function doesEqKey(val: string, obj: Obj) {
    return obj.key === val;
}

typescript here gives no issue. The reason why I found is because at last one side as assignable to the other. However, do understand that val would not be assignable to obj.key

obj.key = val; // Type 'string' is not assignable to type '"A" | "B"'.

So for assoc, we would want to keep the way it is typed, same as propEq is now. But you are correct that propEq does need to be loosened a bit.

However, any is still not appropriate here, because we don't want to mix types.

function doesEqKey(val: number, obj: Obj) {
    return obj.key === val; // This comparison appears to be unintentional because the types 'string' and 'number' have no overlap 
}

We do have a solution for what you are looking for you. In types/util/tools.d.ts use WidenLiterals<T>

import { WidenLiterals } from './util/tools';

export function propEq<K extends keyof U, U>(val: WidenLiterals<U[K]>, name: K, obj: U): boolean;

What this will do is collapse down val from 'A' | 'B' to be string. This should properly mimic the behavior for both doesEqKey functions I have above. Particularly that any string can be compared to 'A' | 'B', but not number or other types that don't overlap

[Nemo108]

Thank you for the tip about WidenLiterals type. It did help me and fixed some of the false negative results that I had.

Unfortunately, there are a lot of other kinds of errors that I wasn't able to fix. I created another draft PR here, where you can find more tests and the version of propEq typings that utilizes WidenLiterals, but as you can see, most of the tests are failing. Moreover, other tests are failing too, like anyPass and allPass.

const isOld = propEq(212, 'age');
const isAllergicToGarlic = propEq(true, 'garlic_allergy');
const isAllergicToSun = propEq(true, 'sun_allergy');
const isFast = propEq(null, 'fast');
const isAfraid = propEq(undefined, 'fear');

const isVampire = anyPass([
  isOld,
  isAllergicToGarlic,
  isAllergicToSun,
  isFast,
  isAfraid
]);

expectType<boolean>(
  isVampire({} as {
    age: number,
    garlic_allergy: boolean,
    sun_allergy: boolean,
    fast: boolean | null,
    fear: boolean | undefined
  })
);

I don't see the reason, why this test should fail, but as you can see, it does, because 'fast' and 'fear' are expected to be of 'null' and 'undefined' types respectively.

These errors are popping out all over the place and cause pain to work with propEq.

[Harris-Miller]

@Nemo108 I checked and rechecked what we need to do and why. Both your MRs at a high-level 3 things

• Better typing for val when we don't yet know the type for obj
• Full currying
• Handling when obj is an array

That is a lot to check, verify, and agree on in a single MR. I would like to suggest we split these into 3 MRs, and focus on one part at a time.

I made the first MR that just focuses on the first case: https://github.com/ramda/types/pull/74

Please let me know what you think of my analysis to the problem and my proposed solution

[Harris-Miller]

Given the happenings in both https://github.com/ramda/types/pull/99 and https://github.com/ramda/types/pull/73. I went back to this MR to look at the changes made that we left off on. Given the now known limitations about trying to do the other improvements (particularly with arguments, possibly returning never there, and cross function type support), what is in this MR currently is the best option I think to move forward with in the short-term.

The code changes do not solve all the issues called out, but it's better than nothing. I'm going to give it a thorough re-test and see if we can try and get this merged after https://github.com/ramda/types/pull/99