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

Proposal for HKT restructuring #1005

Closed
mikearnaldi opened this issue Dec 15, 2021 · 11 comments
Closed

Proposal for HKT restructuring #1005

mikearnaldi opened this issue Dec 15, 2021 · 11 comments
Assignees
Labels
breaking Breaking change enhancement New feature or request

Comments

@mikearnaldi
Copy link
Member

Since the time we implemented HKTs we found a new encoding that doesn't require having a central registry of URIs and that should have far better performances while allowing for more flexibility.

The encoding is implemented in:

https://github.com/mikearnaldi/hkt-new

I propose we change the current encoding to reflect the new implementation, as part of the change we will also need to remove the flexibility of choosing the variance of a type parameter namely in the current encoding we are using a config param to specify how a type parameter should be mixed, the problem with that approach is that it may generate inconsistencies, namely we do implementations on a reduced base using HKT/HKT2 and we have a generic overload that considers all the params with their respective rules but there is no guarantee that the base impl would actually work with that set of rules, namely I could implement a base using R as contravariant and having R as covariant in the target type. Also we actually never used any other setting other than S invariant, R contravariant, etc.

@isthatcentered
Copy link
Contributor

isthatcentered commented Dec 15, 2021

If I may add my newcomer two cents,

I re-implemented it, to understand how you managed to make it work and it is a pleasure to work with ! (especially coming from fp-ts's way of doing HKTs)

The only "confusing" thing was the separate declaration of {Option|Either|...}F for the R, E, Amapping. Maybe the naming convention could be {Option|Either|...}Mapping or something to get a better feel of what this does... But that's a bit of a nitpick😅 (and nothing a few lines of documentation wouldn't make easy).

I'm not sure choosing the variance is required (besides the complexity it might add), I tend to expect R to be contravariant, E & A to be covariant and would probably be confused otherwise.

@mikearnaldi
Copy link
Member Author

it could be OptionHKT or OptionRepr or similar, at the end it is a generic representation of a type

@mikearnaldi
Copy link
Member Author

(overall I agree, it is refreshing how simpler it is)

@mikearnaldi
Copy link
Member Author

I'm not sure choosing the variance is required (besides the complexity it might add), I tend to expect R to be contravariant, E & A to be covariant and would probably be confused otherwise.

My guess is it may be restored as a config param over the typeclass that doesn't get propagated into Kind at all (or it may)

Arguably it isn't too useful

@IMax153
Copy link
Member

IMax153 commented Dec 16, 2021

I also agree - I really enjoy the new encoding! Excellent work on this @mikearnaldi 👏

With respect to variance, maybe it would be helpful to see a use case in user-land where variance may help avoid pitfalls?

The reason I ask is because I also tend to agree with @isthatcentered in how I view R, E, and A, but those newer to Effect may not. Perhaps having a small example of how inclusion of variance as a config param would benefit newer users of Effect and also facilitate this dicussion?

@mikearnaldi
Copy link
Member Author

mikearnaldi commented Dec 16, 2021

As I was mentioning there really is no reasonable place where to use variance configured in any non standard setting. Typescript users, strict mode folks, rightfully expect inputs to be contravariant and outputs to be covariant (the language automatically defines variance based on usage).
The only place where I have ever used the config param to change a default is when I want something to be invariant, like a chain that doesn't allow widening of errors, while this is a valid use case it is not a necessity in fact you can annotate the return types in places where you don't want widening of things.

To better clarify the current hkt does not impose R to be contravariant, it makes no assumption of R at all but:

  1. when R is used in a typeclass it is used contravariant (as input)
  2. when R is used in a data type it is defined as contravariant

So while allowing invariance is a valid use case, making R covariant will break typeclasses that uses it.

The initial thinking was 'let's call it R but let's keep it flexible as it may be Env for a type but output for another' while this is true it is bringing in an added level of complexity for a use case that really has nothing practical

@mikearnaldi mikearnaldi added breaking Breaking change enhancement New feature or request labels Dec 19, 2021
@mikearnaldi
Copy link
Member Author

A side effect of this would be that Deno will be working (without tracing and plugins) #754 given the main issues was usage of module augmentation

@isthatcentered isthatcentered linked a pull request Jan 22, 2022 that will close this issue
@shroomist
Copy link

since #1162 was suppressed, is there any rough insight/plan around this? @mikearnaldi

@mikearnaldi
Copy link
Member Author

@shroomist yes we are working on a re-unification with fp-ts that has adopted this encoding for v3 which is under heavy development by @gcanti, you can follow https://github.com/gcanti/fp-ts/tree/3.0.0-new-hkt for updates. As soon as that is ready Effect will depend on fp-ts and provide fp-ts compliant instances (which is great news for the ecosystem)

@shroomist
Copy link

oh nice, great news, I think it's a good decision to get the Effect back together with fp-ts thanks for sharing.

@SHND
Copy link

SHND commented Oct 4, 2023

I was wondering are we still expecting to re-unify effect-ts with fp-ts? It seems that there wasn't much change on https://github.com/gcanti/fp-ts/tree/3.0.0-new-hkt since last year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants