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

Add branch awareness to TypeScript types (strict) #438

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Avaq
Copy link
Member

@Avaq Avaq commented Aug 6, 2020

This is an alternative to #435 that doesn't include auto-expansion of generic types.


A quick recap of auto-expansion:

Unstrict: With auto-expansion (behaviour on master for chain and chainRej)

  1. chain (_ => reject ('b')) (reject ('a')) gets type Future<string, never>: The types align.
  2. chain (_ => reject ('a')) (reject (1)) gets type Future<number | string, never>: The rejection type was expanded.
  3. chain (_ => reject ('a')) (resolve (1)) gets type Future<string, number>: The types were expanded with never, which leaves them intact.

Strict: Without auto-expansion (behaviour on master for everything else)

  1. chain (_ => reject ('b')) (reject ('a')) gets type Future<string, never>: The types align.
  2. chain (_ => reject ('a')) (reject (1)) produces a type error: The types don't align. This is desirable behaviour, because it is more true to the behaviour in Haskell, where the rejection branch cannot change type because the monad instance is created for the unary constructor that is left after providing the first type variable.
  3. chain (_ => reject ('a')) (resolve (1)) produces a type error: TypeScript doesn't want to assign never to another type. For chain and chainRej this was fixed in Improve TypeScript type inference #374 by making those functions unstrict.

In #435, I had given up on the strict approach (following the trend created in #374), because I thought there's no way to get the strict behaviour while retaining the ability to chain (or otherwise combine) futures where the branch types don't technically conflict because the variable on one of the sides was untouched (like in examples 3 above).

The goal of this pull request is to:

  • Reimplement chain and chainRej using the strict approach, getting rid of the problem highlighted by example 2.
  • Add branch awareness to all combinators. I realized this is enough to get rid of problem 3 from the strict approach, because we're no longer asking TypeScript to assign never to anything: Our overloads take care of any occurrence of never with special treatment.

This approach brings new problems, however. In particular, TypeScript loses type information when these functions are called indirectly, such as by pipe. I think this happens because TypeScript cannot infer types from overloaded functions (microsoft/TypeScript#32418 (comment)).

So where this approach works perfectly for statements like and (resolve (42)) (reject ('abc')), it produces some unknown type variables when refactoring that to reject ('abc') .pipe (and (resolve (42))), because and (resolve (42)) creates an overloaded lambda, from which pipe cannot infer any type information.

If anyone can help with this new problem, it'll be much appreciated!

@codecov

This comment has been minimized.

@Avaq
Copy link
Member Author

Avaq commented Jan 7, 2021

A solution that was found for #457 has the potential of unblocking this PR. In particular, it is possible to overload functions in such a way that TypeScript chooses the correct overload based on the "direction" that function arguments are supplied in:

Fluture/index.d.ts

Lines 144 to 148 in 8eef14a

/** Map over the resolution value of the given Future or ConcurrentFuture. See https://github.com/fluture-js/Fluture#map */
export const map: {
<B, F extends Functor<unknown>>(f: Functor<unknown> extends F ? never : (a: F extends Functor<infer A> ? A : never) => B): (source: F) => Unfunctor<F, B>
<A, B>(f: (a: A) => B): <F extends Functor<A>>(f: F) => Unfunctor<F, B>
}

I believe that this allows me to fix the issue mentioned at the bottom of my PR comment. 🎉

@Avaq Avaq changed the base branch from master to avaq/map-ts January 8, 2021 13:46
@Avaq
Copy link
Member Author

Avaq commented Jan 8, 2021

I rebased this on #457 and pushed a commit that shows typings for alt that are adjusted to usage with pipe and encode branching. This approach seems promising. :)

Base automatically changed from avaq/map-ts to master March 13, 2021 11:25
A certain Future is a Future whose branch is certain. In TypeScript,
this is represented by having 'never' as the type for one of the
branches.

This change also applies to ConcurrentFutures.
An uncertain Future is one that might reject or resolve.
@Avaq Avaq force-pushed the avaq/tsd-strict branch 2 times, most recently from 3b7922b to 7072818 Compare April 24, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant