purescript-contrib / purescript-profunctor-lenses

Pure profunctor lenses
MIT License
144 stars 52 forks source link

Add _Newtyped #98

Closed LiamGoodacre closed 5 years ago

LiamGoodacre commented 5 years ago

I've seen multiple people run into an issue with using _Newtype and it failing because it can't work out one of the two types. This PR adds a version of this function that only requires a single Newtype instance, which should improve the experience people have with this.

Any better suggestions on a name? My two thoughts were _Newtyped and _Newtype'.

garyb commented 5 years ago

I don't really have any particular opinion on the naming of this or anything. Just curious as to what kind of usage causes this to be a problem? I've used _Newtype quite a bit and never had a problem with it, but perhaps it's always been under a type annotation or something.

LiamGoodacre commented 5 years ago

Here's an example repl session:

> import Prelude
> import Data.Newtype
> import Data.Lens
> import Data.Lens.Iso.Newtype
> newtype X = X Int
> derive instance newtypeX :: Newtype X _
> X 42 ^. _Newtyped
42
> X 42 ^. _Newtype
Error found:
in module $PSCI
at <internal> line 0, column 0 - line 0, column 0
  The inferred type
    forall t3 t5. Newtype t3 t5 => Int
  has type variables which are not mentioned in the body of the type. Consider adding a type annotation.

In this scenario, as we are only using one half of the Iso, it can't resolve the Newtype constraint for the half we don't use.

Though now I'm wondering if we should instead have a simple function that works for any optic, instead of special casing _Newtype. So, you could call simple _Newtype, which unifies s with t and a with b in Optic p s t a b.

LiamGoodacre commented 5 years ago

Closing in favour of https://github.com/purescript-contrib/purescript-profunctor-lenses/pull/99