Monet crashes TypeScript compiler 2.4+

[Rikmorn]

I've recently updated to Typescript 2.4.1 and noticed that Monet is causing it to crash when compiling, with a out of memory error.

Simply importing the module is enough to force a crash during compilation (no other logic besides the import)

I've tested it on TS@2.5.0-dev.20170629 (which has a fix for a similar issue) and Monet@0.9.0-alpha.4 and it still persists.

The TS team introduced in 2.4 improved generic inference which could be responsible for this.

[emmanueltouzery]

I believe this is this issue:
microsoft/TypeScript#16777

clearly we'll need better error messages from the compiler to fix the issue. It is my understanding that it's fixed in typescript master but I confirm that 20170629 still has the issue. I'm waiting for the next snapshot (not sure how often they do them) and if that one still doesn't fix the issue, I'll reopen that issue.

[emmanueltouzery]

so it seems the bug was not fixed yet. To be clear, the typescript bug is only that it reacts very badly in case of now-invalid typescript source. It should display a proper error message, but instead of that, it fails with out of memory. That doesn't change the fact that typescript 2.4 now rejects monet type definitions, and even after the fix it'll presumably keep rejecting them.

Here is an example of a fix for this issue with the rxjs bindings:
ReactiveX/rxjs#2539

so in that case an interface inheriting changes the type definition compared to the parent interface regarding generics. Indeed the feature that breaks the compilation is the new strict generics check (workaround: enable noStrictGenericChecks).

Now I got monet type definitions to compile with typescript 2.4.1 if I comment some bits of them:

diff --git a/src/monet.d.ts b/src/monet.d.ts
index a5904c9..24ac381 100644
--- a/src/monet.d.ts
+++ b/src/monet.d.ts
@@ -27,7 +27,7 @@ interface Chain<T> {
 /* Typeclass for binding, the core monadic transformation */
 interface Bind<T> extends Chain<T> {
   bind<V>(fn: (val: T) => Bind<V>): Bind<V>     // alias of chain
-  chain<V>(fn: (val: T) => Bind<V>): Bind<V>;
+  // chain<V>(fn: (val: T) => Bind<V>): Bind<V>;
   flatMap<V>(fn: (val: T) => Bind<V>): Bind<V>  // alias of chain
   join<V>(): Bind<V>  // works only if T = Bind<V>
 }
@@ -44,11 +44,11 @@ export interface ITraversable<T> {
 
 interface IMonad<T> extends Functor<T>, Bind<T>, Applicative<T> {
   /* These all are defined in Functor, Bind and Applicative: */
-  bind<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
-  flatMap<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
-  chain<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
-  map<V>(fn: (val: T) => V): IMonad<V>;
-  join<V>(): IMonad<V>; // only if T = IMonad<V>
+  // bind<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
+  // flatMap<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
+  // chain<V>(fn: (val: T) => IMonad<V>): IMonad<V>;
+  // map<V>(fn: (val: T) => V): IMonad<V>;
+  // join<V>(): IMonad<V>; // only if T = IMonad<V>
 
   /* These are monet-Monad-specific: */
   takeLeft(m: IMonad<T>): IMonad<T>;

looking at these, we're supposed to return Chain<V> but we return Bind<V>, and things like that. I did try to make it compile by returning what the parent interface declared but failed (but it gets tedious to do it everywhere).

I mean it's really just guessing until typescript gives us a proper error message but... It might be that this part must be reworked to fully support typescript 2.4. Clearly it will be much easier for us to diagnose when typescript gives a better message though.

[ulfryk]

I'll try to look at it today evening, but I can't promise that I'll push anything forward before 25 Jul (due family vacations).

[emmanueltouzery]

I think this is relevant:
gcanti/fp-ts#138

[Rikmorn]

This doesn't seem like it'll be address soon without rewriting the definitions or ignoring lib check.

[estaub]

Consider reporting this to the Typescript folks as a bug specific to Monet. I see no issue over there referring to Monet. It may be the same as something else, or it may not. At the least, it will bump the priority.

(It turned out the code where I was using Monet is dead, so I just removed it, and thus I have no stake.)

[emmanueltouzery]

Good point, though this comment is from a MS developer, best as I can tell:
microsoft/TypeScript#14628 (comment)

Also, do you depend on fp-ts, fantasyland or monet? Those libraries are known to have problems running out of memory. (See #16029 or #16777)

There are a bunch of bugs related to out of memory in 2.4.1 in the typescript github. There is probably a single root cause (?). But if we open a bug maybe they can help us on the direction to take. Or maybe that'd be better handled through a stackoverflow question.

[emmanueltouzery]

Could be related again:
microsoft/TypeScript#17070 (comment)

[emmanueltouzery]

Another related link, i'll stop now:

immutable-js/immutable-js#1285

[emmanueltouzery]

I may be wrong, but I think the core problem is that typescript doesn't support higher-kinded types. The definitions that we have are fundamentally invalid in typescript and we only got away with them so far because the compiler wasn't strict when checking generic constraints. That's my understanding at least.

I see two paths:

1. we accept that typescript doesn't support them and remove the Functor, Applicative, Chain, Bind, IMonad interfaces -- obviously we do keep all the concrete implementations (list, nel, maybe..). That would fix the problem. We do lose the compiler-enforced uniform interfaces between these concrete types, and the documentation advantages that HKTs offer

2. we try to find a way to get the HKT features despite typescript's limitation. Here is an explanation of a possible solution, which I think the fp-ts library is using

Personally I would probably try to be pragmatic and go for the first solution. KISS and accept that in the end, we are not using haskell. What do you think @ulfryk?

[estaub]

@emmanueltouzery You don't have to lose much of the documentation advantage; I suspect the extends can be left on, with Functor et al having neutered but well-commented definitions.

[emmanueltouzery]

I think this typescript PR will make typescript react better than with an out of memory:

microsoft/TypeScript#17947

Not merged yet though and i'm sure we have to change monet no matter what.

[emmanueltouzery]

Apparently typescript master doesn't oom anymore:
microsoft/TypeScript#16777 (comment)

I'll try it next week when i return from holidays, by then hopefully they'll have released a typescript@next snapshot with the fix.

[ulfryk]

I'll also investigate when new snapshot is available :)

[emmanueltouzery]

The ts developer says "starting tomorrow" and it was yesterday but i'm not sure typescript@next is in fact released daily.

Otherwise it should be:

npm install -g typescript@next
tsc --version

[emmanueltouzery]

ok i installed the tsc snapshot using the method i described (version @next).

I got version 2.6.0-dev.20170826.

and you'll never guess (well maybe you would have, but i for one didn't expect that). it compiles monet master without any errors.

npm test and tsc both run cleanly. I re-ran with 2.4.2 and 2.5.1 and confirmed running tsc in monet's root folder fails for both versions. Not so with the @next version.

so i guess we can say the issue will fix itself with 2.6 (that said, it'll take a while since 2.5 is only at the RC stage). I still think it's still possible that even 2.6 should not compile it, but maybe after all it's actually possible to express this with typescript.. and since it compiles...

[ulfryk]

🎉

[treybrisbane]

I feel like we should still remove the "fake" HKTs, or migrate to an alternative approach if possible.

You said it yourself; the type definitions are technically invalid. I don't know why 2.6 successfully compiles them, but I don't think we should continue to rely on this behaviour when we know we're doing bad things...

For some added context, I use Monet in a production system and this bug is blocking us from upgrading to TS 2.4. If Monet continues to rely on being able to get away with bad things, then it runs the risk of blocking further upgrades in the future, which is something I'd really like to avoid if possible. :)

[ulfryk]

@emmanueltouzery you wrote:

remove typescript binding bits which assume higher-kinded types support: those bits break with typescript

Where can I find information that TS doesn't support higher-kinded types in a way we wrote them in monet ? I'm using them in many places, and the only problem was TS2.4/2.5 while it seems to me that it was some kind of TS bug rather then decision to not support such type constructs…

[emmanueltouzery]

@ulfryk please look at the post from fp-ts on how they tackled the issue that i referred to a couple of times: https://medium.com/@gcanti/higher-kinded-types-in-typescript-static-and-fantasy-land-d41c361d0dbe

A little google also found this page, where a user asks for implementing applicative with typescript:
microsoft/TypeScript#5929

[emmanueltouzery]

also more info there => microsoft/TypeScript#1213

[emmanueltouzery]

note: i am not 100% sure that TS doesn't enable what monet does. i think it doesn't, but i didn't invest much time into this.

[ulfryk]

See #149