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

Change Lens type from Tuple to a record/union #58

Open
LAJW opened this issue Jan 24, 2021 · 5 comments
Open

Change Lens type from Tuple to a record/union #58

LAJW opened this issue Jan 24, 2021 · 5 comments

Comments

@LAJW
Copy link

LAJW commented Jan 24, 2021

A great package, I'm enjoying it thoroughly, with the exception of compile errors.

It quite often happens the type checker substitutes the type alias for the actual type (espcially in the case of an inferred type and especially in the case of an error) and produces errors more difficult to parse than they would be if the lens was stored not as a tuple but rather a record:

expected: Lens<MyRecord, int>
vs
expected: (MyRecord -> int) * (int -> MyRecord -> MyRecord)

Which needless to say gets worse with the complication of either of the types involved.

Would this be something you'd be willing to consider changing or is it too compatibility breaking to be worth thinking about?

@varon
Copy link
Contributor

varon commented Jan 26, 2021

Lenses are defined as tuples to allow for libraries to implement them without a dependency on aether.

It also means that different lens libraries can interop.

To get clean types, create an alias for Lens<'a, 'b> = ('a -> b) * ('b -> 'a -> 'a) and a simple helper function:

let lens get set: Lens<_,_> = (get,set)

@LAJW
Copy link
Author

LAJW commented Jan 27, 2021

@varon and I do that (using Aether's Lens alias). The problem with aliases though is they too often get expanded in erroneous situations.

@varon
Copy link
Contributor

varon commented Jan 27, 2021

It may also depend on your tooling, but there's not all that much you can do aside from specifying signatures.

@LAJW
Copy link
Author

LAJW commented Jan 27, 2021

One thing that could be done is making Aether work with a record type. I rolled my own subset of lense methods and errors were consistently better, but I thought it would be nice if it was integrated into the library.

Unless not having to include aether to use lenses is a valuable enough of a feature...

@stijnmoreels
Copy link

To get clean types, create an alias for Lens<'a, 'b> = ('a -> b) * ('b -> 'a -> 'a) and a simple helper function:

let lens get set: Lens<_,_> = (get,set)

Can we maybe consider including these helper functions in the library? Especially when dealing with complex lenses, it can get confused if your tuple has the right kind of types.

These ones, could already save us a lot of trouble:

module Lens =
  let create getter setter : Lens<'a, 'b> = getter, setter
module Prism =
  let create getter setter : Prism<'a, 'b> = getter, setter
module Isomorphism =
  let create left right : Isomorphism<'a, 'b> = left, right
module Epimorphism =
  let create left right : Epimorphism<'a, 'b> = left, right

Happy to contribute.

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

No branches or pull requests

3 participants