purescript / purescript-typelevel-prelude

Types and kinds for basic type-level programming
BSD 3-Clause "New" or "Revised" License
63 stars 21 forks source link

Type.Row exports both a type and a class called Cons #43

Closed hdgarrood closed 5 years ago

hdgarrood commented 5 years ago

At the moment, Type.Row re-exports both Prim.Row.Cons and Prim.RowList.Cons. Unfortunately, this will be disallowed at some point in the future (see https://github.com/purescript/purescript/issues/3502 and https://github.com/purescript/purescript/issues/1888), so presumably we will need to remove one of them.

Perhaps we can export a type synonym along the lines of

type ConsRL = Prim.RowList.Cons

so that at least you don't need to separately import Prim.RowList if you were previously using Prim.RowList.Cons via Type.Row.

hdgarrood commented 5 years ago

I've come across one problem with this: a type synonym for Prim.RowList.Cons isn't going to be particularly useful, because 99% of the time, you're going to want to use it in instances, where type synonyms are disallowed. Perhaps we should lift the restriction that type synonyms may not appear in instances: https://github.com/purescript/purescript/issues/3391

LiamGoodacre commented 5 years ago

How about splitting Type.Row into Type.Row and Type.RowList, in the same way that we have Prim.Row and Prim.RowList? Then they are exported from different modules.

hdgarrood commented 5 years ago

:+1: that sounds sensible to me too.

joneshf commented 5 years ago

There is another way that doesn't require a new type synonym or force any breaking changes. The module only actually exports Prim.RowList.Cons. From the perspective of anyone consuming Type.Row, it has only ever exported Prim.RowList.Cons, no matter what the source code says. Those consumers include Type.Prelude and Type.Row.Homogeneous. If you attempt to import Prim.Row.Cons from Type.Row, you get an UnknownImport error:

module Foo where

import Type.Row (class Cons)
[1/2 UnknownImport] src/Foo.purs:3:18

  3  import Type.Row (class Cons)
                      ^^^^^^^^^^

  Cannot import type class Cons from module Type.Row
  It either does not exist or the module does not export it.

If https://github.com/purescript/purescript/issues/3502 is fixed and released it will cause the current version to error, but a non-breaking change can be something like:

diff --git a/src/Type/Row.purs b/src/Type/Row.purs
index 1f9ede7..caba108 100644
--- a/src/Type/Row.purs
+++ b/src/Type/Row.purs
@@ -12,7 +12,8 @@ module Type.Row
   , type (+)
   ) where

-import Prim.Row (class Lacks, class Nub, class Cons, class Union)
+import Prim.Row (class Lacks, class Nub, class Union)
+import Prim.Row as Row
 import Prim.RowList (kind RowList, Cons, Nil, class RowToList)
 import Type.Equality (class TypeEquals)
 import Type.Data.Symbol as Symbol
@@ -33,7 +34,7 @@ instance listToRowNil

 instance listToCons
   :: ( ListToRow tail tailRow
-     , Cons label ty tailRow row )
+     , Row.Cons label ty tailRow row )
   => ListToRow (Cons label ty tail) row

 -- | Remove all occurences of a given label from a RowList

That change could happen today. It doesn't need to wait for the compiler to disallow things.

If a module restructure is still wanted, that's fine, but it's not necessary to address this potential future issue. Please strongly consider making a non-breaking fix for this issue if https://github.com/purescript/purescript/issues/3502 disallows the code.

hdgarrood commented 5 years ago

That's a really good point. I agree, let's do that.

garyb commented 5 years ago

That's a great find! I'm all for changing the non-breaking way, since functionally it'll be no different.

hdgarrood commented 5 years ago

@joneshf would you like to send a PR?

hdgarrood commented 5 years ago

I am going to close this because this has been addressed by #45, but if anyone feels strongly in favour of adding a Type.RowList module we can reopen this.