fix(filter, reject) update overload orderings

test/filter.test.ts

@@ -0,0 +1,92 @@
+import { expectError, expectType } from 'tsd';
+
+import { isNil, isNotNil, filter, gt, pipe, prop } from '../es';
+
+type Foobar = { foo?: string; };
+type Values = { value: number };
+const gt5 = gt(5);
+
+// filter(() => val is p)(list)
+// curried filter(isNotNil) with no type annotation defaults to `{}`, the the full function's return type ends up as {}[]
+// the return type here is {}[] due to the "collapsing generic issue", read more about that here: https://github.com/ramda/types/discussions/54
+expectType<{}[]>(filter(isNotNil)([] as (string | undefined)[]));
+// you can fix by setting the generic on isNotNil, the type gets narrows to be just `string` (intended purpose)
+expectType<string[]>(filter(isNotNil<string | undefined>)([] as (string | undefined)[]));
+// deeper types can't be narrowed by default with isNotNil
+expectType<Foobar[]>(filter((x: Foobar) => isNotNil(x.foo))([] as Foobar[]));
+// type annotations required for function composition with other ramda funcs
+expectType<Foobar[]>(
+  filter(
+    pipe(prop('foo')<Foobar>, isNotNil)
+    //                   ^ this give typescript a "hint" that `prop('foo')(foobar: Foobar)`, which trickles out to both `pipe` an `isNotNil` here
+  )([] as Foobar[])
+);
+// to narrow deeply, set custom return type on arrow function
+expectType<Required<Foobar>[]>(filter((x: Foobar): x is Required<Foobar> => isNotNil(x.foo))([] as Foobar[]));
+// can do isNil too
+expectType<undefined[]>(filter(isNil<string | undefined>)([] as (string | undefined)[]));
+// combining with `pipe` requires same annotations
+expectType<string[]>(pipe(filter(isNotNil<string | undefined>))([] as (string | undefined)[]));
+// when using a function like gt5 which doesn't have a generic, passing filter to pipe has no negative consequences
+expectType<number[]>(pipe(filter(gt5))([] as number[]));
+
+// for when predicate doesn't type narrow
+
+
+// filter(() => boolean)(list)
+expectType<number[]>(filter(gt5)([] as number[]));
+// can't use and untyped arrow function
+// @ts-expect-error
+filter(x => gt5(x.value))([] as Values[]); // prop `value` does not exist on `unknown`, remove ts-expect-error to display error (test fails without)
+// fix by setting the generic on `filter`
+expectType<Values[]>(filter<Values>(x => gt5(x.value))([] as Values[]));
+// or typing the arrow func
+expectType<Values[]>(filter((x: Values) => gt5(x.value))([] as Values[]));
+// using an inline `isNotNil` function that does not type narrow returns `number | undefined`, and not `number` like the tests above
+expectType<(number | undefined)[]>(filter((x: number | undefined) => x != null)([] as (number | undefined)[]));
+// combining with `pipe` requires same annotations
+expectType<Values[]>(pipe(filter<Values>(x => gt5(x.value)))([] as Values[]));
+expectType<Values[]>(pipe(filter((x: Values) => gt5(x.value)))([] as Values[]));
+// when using a function like gt5 which doesn't have a generic, passing filter to pipe has no negative consequences
+expectType<number[]>(pipe(filter(gt5))([] as number[]));
+
+
+// filter(() => narrow)(dist)
+type Dict = Record<string, string | undefined>;
+
+// same is as the first test, returns `Record<string, {}>`
+expectType<Record<string, {}>>(filter(isNotNil)({} as Dict));
+// can fix with a type annotation
+expectType<Record<string, string>>(filter(isNotNil<string | undefined>)({} as Dict));
+
+// notice that `dict` is not inherently supported by `filter` when used with `pipe`
+// this is because when a function is passed a function, it takes last overload, which only supports `list`
+// this is a limitation of typescript, not ramda
+expectError(pipe(filter(isNotNil<string | undefined>))({} as Dict));
+// you can get around this by using an arrow function
+expectType<Record<string, string>>(pipe((dict: Dict) => filter(isNotNil, dict))({} as Dict));
+
+// filter(() => val is p, list)
+// full signature can directly match list/dict type, and apply it to the generic on isNotNil
+expectType<string[]>(filter(isNotNil, [] as (string | undefined)[]));
+// this also means it can infer `x` for arrow functions
+expectType<Foobar[]>(filter(x => isNotNil(x.foo), [] as Foobar[]));
+// as well as for function composition with other ramda funcs
+expectType<Foobar[]>(
+  filter(pipe(prop('foo'), isNotNil), [] as Foobar[])
+);
+// still need to set custom return type to narrow deeply, but you don't need to set the type on `x`
+expectType<Required<Foobar>[]>(filter((x): x is Required<Foobar> => isNotNil(x.foo), [] as Foobar[]));
+// is Nil works un-annotated as well
+expectType<undefined[]>(filter(isNil, [] as (string | undefined)[]));
+
+// filter(() => boolean, list)
+expectType<number[]>(filter(gt5, [] as number[]));
+// arrow function `x` arg get's type inferred correctly
+filter(x => gt5(x.value), [] as Values[]);
+// using an inline `isNotNil` function that does not type narrow returns `number | undefined`
+expectType<(number | undefined)[]>(filter(x => x != null, [] as (number | undefined)[]));
+
+// filter(() => narrow, dist)
+// no need for type annotations when using full signature
+expectType<Record<string, string>>(filter(isNotNil, {} as Dict));

test/reject.test.ts

@@ -0,0 +1,94 @@
+import { expectError, expectType } from 'tsd';
+
+import { isNil, isNotNil, reject, gt, pipe, prop } from '../es';
+
+type Foobar = { foo?: string; };
+type Values = { value: number };
+const gt5 = gt(5);
+
+// reject(() => val is p)(list)
+// unlike `filter(isNotNil)`, `reject(isNil)` this works because `isNil`, and mostly by accident
+// `reject(isNil)` return `(null | undefined) & T`, and `T` collapses to `{}`,
+// but this combined with `Exclude<string | undefined, (null | undefined) & {}>` does yield the desired result of `string[]`
+expectType<string[]>(reject(isNil)([] as (string | undefined)[]));
+// deeper types can't be narrowed by default with isNil
+expectType<Foobar[]>(reject((x: Foobar) => isNil(x.foo))([] as Foobar[]));
+// type annotations required for function composition with other ramda funcs
+expectType<Foobar[]>(
+  reject(
+    pipe(prop('foo')<Foobar>, isNil)
+    //                   ^ this give typescript a "hint" that `prop('foo')(foobar: Foobar)`, which trickles out to both `pipe` an `isNil` here
+  )([] as Foobar[])
+);
+
+// You'd expect this to work same as `filter(pred)s`, but `Exclude<Foobar, Required<Foobar>>` is tyring to remove `foo: string` from `foo?: string`
+// `foo` doesn't exist, only `foo?`, so `Exclude<Foobar, Required<Foobar>>` just ends up being `Foobar`. Typescript is weird sometimes
+expectType<Foobar[]>(reject((x: Foobar): x is Required<Foobar> => !isNil(x.foo))([] as Foobar[]));
+// can do isNotNil too
+expectType<undefined[]>(reject(isNotNil<string | undefined>)([] as (string | undefined)[]));
+// combining with `pipe` requires same annotations
+expectType<string[]>(pipe(reject(isNil<string | undefined>))([] as (string | undefined)[]));
+// when using a function like gt5 which doesn't have a generic, passing reject to pipe has no negative consequences
+expectType<number[]>(pipe(reject(gt5))([] as number[]));
+
+// for when predicate doesn't type narrow
+
+
+// reject(() => boolean)(list)
+expectType<number[]>(reject(gt5)([] as number[]));
+// can't use and untyped arrow function
+// @ts-expect-error
+reject(x => gt5(x.value))([] as Values[]); // prop `value` does not exist on `unknown`, remove ts-expect-error to display error (test fails without)
+// fix by setting the generic on `reject`
+expectType<Values[]>(reject<Values>(x => gt5(x.value))([] as Values[]));
+// or typing the arrow func
+expectType<Values[]>(reject((x: Values) => gt5(x.value))([] as Values[]));
+// using an inline `isNil` function that does not type narrow returns `number | undefined`, and not `number` like the tests above
+expectType<(number | undefined)[]>(reject((x: number | undefined) => x != null)([] as (number | undefined)[]));
+// combining with `pipe` requires same annotations
+expectType<Values[]>(pipe(reject<Values>(x => gt5(x.value)))([] as Values[]));
+expectType<Values[]>(pipe(reject((x: Values) => gt5(x.value)))([] as Values[]));
+// when using a function like gt5 which doesn't have a generic, passing reject to pipe has no negative consequences
+expectType<number[]>(pipe(reject(gt5))([] as number[]));
+
+
+// reject(() => narrow)(dist)
+type Dict = Record<string, string | undefined>;
+
+// works for the same reason as the first test
+expectType<Record<string, string>>(reject(isNil)({} as Dict));
+// can fix with a type annotation
+expectType<Record<string, string>>(reject(isNil<string | undefined>)({} as Dict));
+
+// notice that `dict` is not inherently supported by `reject` when used with `pipe`
+// this is because when a function is passed a function, it takes last overload, which only supports `list`
+// this is a limitation of typescript, not ramda
+expectError(pipe(reject(isNil<string | undefined>))({} as Dict));
+// you can get around this by using an arrow function
+expectType<Record<string, string>>(pipe((dict: Dict) => reject(isNil, dict))({} as Dict));
+
+// reject(() => val is p, list)
+// full signature can directly match list/dict type, and apply it to the generic on isNil
+expectType<string[]>(reject(isNil, [] as (string | undefined)[]));
+// this also means it can infer `x` for arrow functions
+expectType<Foobar[]>(reject(x => isNil(x.foo), [] as Foobar[]));
+// as well as for function composition with other ramda funcs
+expectType<Foobar[]>(
+  reject(pipe(prop('foo'), isNil), [] as Foobar[])
+);
+// still need to set custom return type to narrow deeply, but you don't need to set the type on `x`
+// see about why this returns `Foobar[]` while `filter()` returns `Required<Foobar>[]`
+expectType<Foobar[]>(reject((x): x is Required<Foobar> => !isNil(x.foo), [] as Foobar[]));
+// isNotNil works un-annotated as well
+expectType<undefined[]>(reject(isNotNil, [] as (string | undefined)[]));
+
+// reject(() => boolean, list)
+expectType<number[]>(reject(gt5, [] as number[]));
+// arrow function `x` arg get's type inferred correctly
+reject(x => gt5(x.value), [] as Values[]);
+// using an inline `isNil` function that does not type narrow returns `number | undefined`
+expectType<(number | undefined)[]>(reject(x => x != null, [] as (number | undefined)[]));
+
+// reject(() => narrow, dist)
+// no need for type annotations when using full signature
+expectType<Record<string, string>>(reject(isNil, {} as Dict));

types/filter.d.ts

@@ -1,12 +1,24 @@
+// filter(() => narrow)
 export function filter<A, P extends A>(
   pred: (val: A) => val is P,
 ): {
+  // if we put `Record<string, A>` first, it will actually pic up `A[]` as well
+  // so it needs to go first
   <B extends A>(list: readonly B[]): P[];
   <B extends A>(dict: Record<string, B>): Record<string, P>;
+  // but we also want `A[]` to be the default when doing `pipe(filter(fn))`, so it also needs to be last
+  <B extends A>(list: readonly B[]): P[];
 };
+
+// filter(() => boolean)
 export function filter<T>(
   pred: (value: T) => boolean,
 ): <P extends T, C extends readonly P[] | Record<string, P>>(collection: C) => C;
+
+// filter(() => narrow, list) - readonly T[] falls into Record<string T> for some reason, so list needs to come first
 export function filter<T, P extends T>(pred: (val: T) => val is P, list: readonly T[]): P[];
+// filter(() => narrow, dist)
 export function filter<T, P extends T>(pred: (val: T) => val is P, dict: Record<string, T>): Record<string, P>;
+// filter(() => boolean, list | dist) - this case is not expected to be picked up directly
+// it is here so operations like `flip(filter)` or `addIndex(filter)` get retained correctly type-wise (or best they can)
 export function filter<T, C extends readonly T[] | Record<string, T>>(pred: (value: T) => boolean, collection: C): C;

types/reject.d.ts

@@ -1,18 +1,24 @@
+// reject(() => narrow)
 export function reject<A, P extends A>(
   pred: (val: A) => val is P,
 ): {
-  <B extends A>(list: readonly B[]): Array<Exclude<B, P>>;
+  // if we put `Record<string, A>` first, it will actually pic up `A[]` as well
+  // so it needs to go first
+  <B extends A>(list: readonly B[]): Exclude<B, P>[];
   <B extends A>(dict: Record<string, B>): Record<string, Exclude<B, P>>;
+  // but we also want `A[]` to be the default when doing `pipe(reject(fn))`, so it also needs to be last
+  <B extends A>(list: readonly B[]): Exclude<B, P>[];
 };
+
+// reject(() => boolean)
 export function reject<T>(
   pred: (value: T) => boolean,
 ): <P extends T, C extends readonly P[] | Record<string, P>>(collection: C) => C;
-export function reject<A, B extends A, P extends A>(
-  pred: (val: A) => val is P,
-  list: readonly B[],
-): Array<Exclude<B, P>>;
-export function reject<A, B extends A, P extends A>(
-  pred: (val: A) => val is P,
-  dict: Record<string, B>,
-): Record<string, Exclude<B, P>>;
+
+// reject(() => narrow, list) - readonly T[] falls into Record<string T> for some reason, so list needs to come first
+export function reject<T, P extends T>(pred: (val: T) => val is P, list: readonly T[]): Exclude<T, P>[];
+// reject(() => narrow, dist)
+export function reject<T, P extends T>(pred: (val: T) => val is P, dict: Record<string, T>): Record<string, Exclude<T, P>>;
+// reject(() => boolean, list | dist) - this case is not expected to be picked up directly
+// it is here so operations like `flip(reject)` or `addIndex(reject)` get retained correctly type-wise (or best they can)
 export function reject<T, C extends readonly T[] | Record<string, T>>(pred: (value: T) => boolean, collection: C): C;