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

HKT abstraction no more typecheck with latest typescript dev version. #244

Closed
sledorze opened this issue Oct 6, 2017 · 30 comments
Closed
Milestone

Comments

@sledorze
Copy link
Collaborator

sledorze commented Oct 6, 2017

Try

array.traverse(validation)

Using latest typescript version (next) after 2.6.0-dev.20171006
(fp-ts is 0.5.2)

typeof ".../fp-ts/lib/Validation"' is not assignable to parameter of type 'Applicative<"Validation">'.
  Types of property 'ap' are incompatible.
    Type '<L, A, B>(fab: Validation<L, (a: A) => B>, fa: Validation<L, A>) => Validation<L, B>' is not assignable to type '<A, B>(fab: HKT<"Validation", (a: A) => B>, fa: HKT<"Validation", A>) => HKT<"Validation", B>'.
      Types of parameters 'fab' and 'fab' are incompatible.
        Type 'HKT<"Validation", (a: A) => B>' is not assignable to type 'Validation<{}, (a: A) => B>'.
          Type 'HKT<"Validation", (a: A) => B>' is not assignable to type 'Success<{}, (a: A) => B>'.
            Property 'value' is missing in type 'HKT<"Validation", (a: A) => B>'.

it may be related to that recent improvement:

microsoft/TypeScript@884c72e

@gcanti
Copy link
Owner

gcanti commented Oct 8, 2017

@sledorze Yeah, the build is broken if strictFunctionTypes = true. Working on this, I'm afraid there will be a few breaking changes though

@gcanti gcanti added this to the 0.6 milestone Oct 8, 2017
@gcanti
Copy link
Owner

gcanti commented Oct 10, 2017

In case anyone is curious and want to help, there's a next branch containing my WIP.

The best solution I found so far is declaring the typeclass members as bivariant.

This is because I need to persuade the compiler that, for example, Option<A> and HKT<'Option', A> are actually the same type.

@sledorze
Copy link
Collaborator Author

@gcanti there's a lot of modifications, I've tried yet to understand how the last version of typescript detects variance.
Can you pin point at how you enforce bivariance? (is it types on both positions?)

@gcanti
Copy link
Owner

gcanti commented Oct 11, 2017

Can you pin point at how you enforce bivariance?

Sure. This is a miniature of fp-ts

export interface HKT<URI, A> {
  _URI: URI
  _A: A
}

export interface Functor<F> {
  readonly URI: F
  map: <A, B>(f: (a: A) => B, fa: HKT<F, A>) => HKT<F, B>
}

type Option<A> = Some<A> | None<A>

const URI = 'Option'

type URI = typeof URI

class None<A> {
  _A: A
  _URI: URI
  _tag = 'None'
}
class Some<A> {
  _A: A
  _URI: URI
  _tag = 'Some'
}

declare const map: <A, B>(f: (a: A) => B, fa: Option<A>) => Option<B>

const functor: Functor<URI> = { URI, map }
// ^ error: Type 'HKT<"Option", A>' is not assignable to type 'Option<A>'

TypeScript 2.6 with strictFunctionTypes = true (correctly) raises an error because map is not a subtype of Functor.map:

  • (a: A) => B is a subtype of (a: A) => B? YES
  • HKT<'Option', A> is a subtype of Option<A>? NO
  • Option<B> is a subtype of HKT<'Option', B>? YES

This is due to fa being in contravariant position

map: <A, B>(f: (a: A) => B, fa: HKT<F, A>) => HKT<F, B>

Now by just changing the Functor definition to

export interface Functor<F> {
  readonly URI: F
-  map: <A, B>(f: (a: A) => B, fa: HKT<F, A>) => HKT<F, B>  
+  map<A, B>(f: (a: A) => B, fa: HKT<F, A>): HKT<F, B>
}

we can make map bivariant

The stricter checking applies to all function types, except those originating in method or construcor declarations

from microsoft/TypeScript#18654 (comment)

@raveclassic
Copy link
Collaborator

raveclassic commented Oct 11, 2017

Relying on declaration type (method or function) is quite a crutch but I hope if variance assertions (in and out) are implemented as said in microsoft/TypeScript#18654 (comment) we can avoid this mess.

@gcanti
Copy link
Owner

gcanti commented Oct 11, 2017

@raveclassic not sure if it will make any difference, they are just assertions and we still must persuade the compiler that Option<A> and HKT<'Option', A> are actually the same type, right?

@raveclassic
Copy link
Collaborator

I think (I may be wrong) the problem is in additional fields on Option instance like (_tag, methods etc) and that now type parameter A is checked contravariantly because it's an input. If we change delcaration to "method-style", then it's checked bivariantly and almost everything's ok. Almost - because in (a: A) => B A is also checked bivariantly and that's a problem:

const a: Option<number | undefined> = some(undefined);
const double = (n: number): number => n * 2;
a.map(double); //no error because of bivariance

If we are able to do smth like the following, then all the problems go away:

interface Functor<out F> {
  map<in A, out A2 extends A, out B>(f: (a: A) => B, fa: HKT<F, A2>): HKT<F, B>
}

Correct me if I'm wrong, all this type theory is pretty new for me.

@gcanti
Copy link
Owner

gcanti commented Oct 11, 2017

Correct me if I'm wrong

Honestly I don't know, It's hard to say without an actual implementation to play with, we'll see.

In the meantime I would like to find at least one solution to the current problem: the build is broken. Currently we are already paying for bivariance (thanks for the example above) so, even if I'm not completely happy with my solution, is not worse than what we get today.

Anyone with a better idea?

@raveclassic
Copy link
Collaborator

It's strage... I checked latest fp-ts@next branch and callback args are checked correctly!

const a: Array<number | undefined> = [undefined]
const double = (n: number): number => n * 2
a.map(double) //error! hooray

const o: Option<number | undefined> = some(undefined)
o.map(double) //error! hooray

// Argument of type '(n: number) => number' is not assignable to parameter of type '(a: number | undefined) => number'.
//   Types of parameters 'n' and 'a' are incompatible.
//     Type 'number | undefined' is not assignable to type 'number'.
//       Type 'undefined' is not assignable to type 'number'.

Everything seems to be fine :)

@sledorze
Copy link
Collaborator Author

Maybe a bivariance annotation constraint would help..

@gcanti
Copy link
Owner

gcanti commented Oct 12, 2017

@raveclassic Nice.

The static version too

import { Option, some, option } from 'fp-ts/lib/Option'

const o: Option<number | undefined> = some(undefined)
const double = (n: number): number => n * 2
const y = option.map(double, o) // error

@raveclassic
Copy link
Collaborator

raveclassic commented Oct 12, 2017

So, summing up, changing declaration to "method style" seems to solve both problems. We could even include the fix in 0.5.x as there are no breaking changes.

@gcanti
Copy link
Owner

gcanti commented Oct 12, 2017

breaking changes so far

  • Alt
  • Bimap
  • Contravariant
  • Monoidal
  • Profunctor
  • Semigroupoid

and some methods here and there

@raveclassic
Copy link
Collaborator

raveclassic commented Oct 12, 2017

Ahh, looks like all "static" methods accepting a HKT instance (as a last argument) should be uncurried because if they return a function then its argument is checked contravariantly. Makes sense.

@sledorze
Copy link
Collaborator Author

@raveclassic yes that's what I've also seen so far.
I'm fine with what I've seen from this branch (not having any idea of a better solution).
I guess this requires a lot of librairies upgrade..

@gcanti
Copy link
Owner

gcanti commented Oct 13, 2017

Some stats (checked against the current next branch)

  • io-ts: 0 fixes, 0 breaking changes
  • monocle-ts: 0 fixes, 0 breaking changes
  • fp-ts-routing: 0 fixes, 0 breaking changes
  • parser-ts: 6 fixes, 1 breaking change (Alt instance)

I released a fp-ts@next version (fp-ts@0.6.0-dev.20171013) if you want to quickly try it out in your codebases

@sledorze
Copy link
Collaborator Author

@gcanti unfortunately this leads to that kind of issues as I now have several version of fp-ts (though others libs - any idea how to workaround that?):

Type 'Left<ValidationError[], B>' is not assignable to type 'Left<ValidationError[], B>'. Two different types with this name exist, but they are unrelated.

@gcanti
Copy link
Owner

gcanti commented Oct 13, 2017

@sledorze Which ones? We could install them from github. For a quick and dirty test I'm just manually removing the duplicates.

@sledorze
Copy link
Collaborator Author

@gcanti mainly io-ts not sure about monocle-ts or newtype-ts

@gcanti
Copy link
Owner

gcanti commented Oct 13, 2017

@sledorze if you run npm list fp-ts you can see all duplicates

@sledorze
Copy link
Collaborator Author

@gcanti only problem for me is missing partitionMap but only in one place, so no real problem to workaround it.. I can upgrade.

@gcanti
Copy link
Owner

gcanti commented Oct 14, 2017

@sledorze Great, thanks for the feedback

@gcanti
Copy link
Owner

gcanti commented Oct 17, 2017

New version: 0.6.0-dev.20171016

@gcanti
Copy link
Owner

gcanti commented Oct 21, 2017

only problem for me is missing partitionMap

@sledorze Array.partitionMap?

@sledorze
Copy link
Collaborator Author

@gcanti sorry for the delay, yes typescript Array.partitionMap

@gcanti
Copy link
Owner

gcanti commented Oct 24, 2017

Just published

  • fp-ts@next
  • io-ts@next
  • monocle-ts@next
  • newtype-ts@next

Let me know if

  • there's some issue
  • you need other @next libraries

@sledorze
Copy link
Collaborator Author

@gcanti ok big thanks! Will check in the 'next' days

@sledorze
Copy link
Collaborator Author

@gcanti the transition was painless, I've fully update our whole code base :)

@gcanti
Copy link
Owner

gcanti commented Oct 26, 2017

@sledorze great! Thanks for the feedback. I'll work on #258 in the 'next' few days before releasing the official version

@gcanti
Copy link
Owner

gcanti commented Nov 1, 2017

Official next version released, thank you all for your help, everybody.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants