New `filter` overload causes type errors in 2.11.0

OliverJAsh commented 3 years ago

🐛 Bug report

Current Behavior

Expected behavior

Reproducible example

We have defined a reject operator that is implemented using filter:

import * as A from 'fp-ts/Array';

type MyGenericUnion<T> = { tag: 'A'; value: T } | { tag: 'B'; value: T };

declare const fn: <T>(a: MyGenericUnion<T>) => a is { tag: 'B'; value: T };

A.filter(fn);

const reject = <A>(fn: (a: A) => boolean) => A.filter((value: A) => fn(value) === false);

/*
Argument of type '<T>(a: MyGenericUnion<T>) => a is { tag: "B"; value: T; }' is not assignable to parameter of type '(a: unknown) => boolean'.
  Types of parameters 'a' and 'a' are incompatible.
    Type 'unknown' is not assignable to type 'MyGenericUnion<unknown>'.
      Type 'unknown' is not assignable to type '{ tag: "B"; value: unknown; }'.
*/
reject(fn);

The error only happens in 2.11.0—it worked fine in 2.10. This seems to be because of the overload that was added to filter:

 export declare const filter: {
   <A, B extends A>(refinement: Refinement<A, B>): (as: Array<A>) => Array<B>
+  <A>(predicate: Predicate<A>): <B extends A>(bs: Array<B>) => Array<B>
   <A>(predicate: Predicate<A>): (as: Array<A>) => Array<A>
 }

Do you know why this overload was added?

I filed a TS bug but I think it's working as intended on their end: https://github.com/microsoft/TypeScript/issues/45184

Additional context

Your environment

Which versions of fp-ts are affected by this issue? Did this work in previous versions of fp-ts?

Software	Version(s)
fp-ts
TypeScript

OliverJAsh commented 3 years ago

If we change the overload order so the B extends A overload is last, the issue disappears, however to understand whether that's a viable solution I would first need to understand why that overload was added.

val-o commented 3 years ago

Hi @OliverJAsh! I think it was added after I submitted this issue

val-o commented 3 years ago

Here is a code snippet explaining the issue. This wouldn't compile prior to 2.11

interface Registered {
  type: 'Registered';
  username: string;
}
interface Unregistered {
  type: 'Unregistered';
}
type User = Registered | Unregistered;

const getName = (u: Registered) => u.username;
declare const isRegistered: (u: User) => u is Registered;
declare const someFn: (u: User) => boolean;
declare const users: User[];
const namesOfRegisterdUsers = pipe(
  users,
  A.filter(isRegistered),
  A.filter(someFn),
  A.map(getName)
);

What's interesting is that if I change the overload from:

<A>(predicate: Predicate<A>): <B extends A>(bs: Array<B>) => Array<B>

to:

  <A>(predicate: Predicate<A>): <A>(bs: Array<A>) => Array<A>

It works as expected on TS3.4

OliverJAsh commented 3 years ago

Interesting, thanks for sharing that.

It seems the issue described in https://github.com/gcanti/fp-ts/issues/1484 happens because of a limitation with the way type inference works inside of TypeScript. I reduced the test case a bit more:

https://www.typescriptlang.org/play?ts=4.3.0-beta#code/JYOwLgpgTgZghgYwgAgEoQObAM6ShAE2QG8AoZZMATwAcIAuZAcnS12kKYG5zkBXbNBBwAtg2S4ooDDwC+pUqDzwkyAKoh8bPIRK9qdRgCINWnDoJGANKXmkDKNYKjIAvGkzmORAD7rNnuz4BDykBBAIADZw+MgIAPYguBLxYgBiIIwAFHyMTtAAlG4AfMgARvHxkRBwIKHhUTEoCUlgyDDAkXiMZBQAPACCxVk0wcAIcJDZcIwDRa6lFVU1IAXZ8LNQUHBUg8XzpQNbO3u2Cg3RsS3JZkGEjKxewQDaALqk1223FmmdFg+BCxvNztP7QLLYVIQDIFLLfbwFIA

Using pipe doesn't help:

https://www.typescriptlang.org/play?ts=4.3.0-beta#code/KYDwDg9gTgLgBAE2AYwDYEMrDgMwK4B2yMAlhAXGCWMADwCCANHAEIB8AFOgFxxNzoARry696ASjgBeNq3G8WAKEUkCMYFBzpk2AErAA5iQDO6rAjgBvRXDgwAnjV4ByfUdMbgCZwG4bcPGMNAnQAW2BeUyhVAz8AX2VVMy0dOABVAix3My8rfwcnOAAiDKyTHIQixkUExQLsNKCoaTg3cs8LAB90zMN28z9FJDRMbGRyUzhjCHCAMQIRPF5GjUkZOEEICFRgdAJB4YwsOHGCSZwSVDNea1sGTjBzEmR0dREePjXZTe3dgnk4BwtGIoFB0PZ7l8+KDwfcaspDqMThN4GUPOZeG10V4ANoAXUUp0maIqs0uFUxfWxCHxLSoNA4JI6zAuVw0HGmc3+4iAA

I'm not sure we should add filter overloads to workaround this, especially if it creates problems elsewhere (such as the issue described here).

Alternative solutions to https://github.com/gcanti/fp-ts/issues/1484 (instead of adding an overload):

The first workaround makes the most sense to me personally.

gcanti / fp-ts