-
Notifications
You must be signed in to change notification settings - Fork 21
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
Type.Row exports both a type and a class called Cons #43
Comments
I've come across one problem with this: a type synonym for |
How about splitting |
👍 that sounds sensible to me too. |
There is another way that doesn't require a new type synonym or force any breaking changes. The module only actually exports module Foo where
import Type.Row (class Cons)
If purescript/purescript#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 purescript/purescript#3502 disallows the code. |
That's a really good point. I agree, let's do that. |
That's a great find! I'm all for changing the non-breaking way, since functionally it'll be no different. |
@joneshf would you like to send a PR? |
As mentioned in the previous commit (09bfea484ff6e0d77c028f11314763c794df8c47), we don't want to allow re-exporting a class and a type that have the same name. The changes here make that a reality. The majority of the tests are breaking because we're dependent on `purescript-typelevel-prelude` and it violates the changes here. See: purescript/purescript-typelevel-prelude#43 for more information.
I am going to close this because this has been addressed by #45, but if anyone feels strongly in favour of adding a |
Refs #43, fixes #46. RowList-related types and classes have been removed from Type.Row and moved into Type.RowList, in order to prepare for an upcoming compiler change whereby a module will no longer be allowed to re-export both a type and a class of the same name; see purescript/purescript#3502
* Add failing test for conflicting re-exports (#3502) Within a module, we cannot define a class and a data type with the same name. The issue here is that we can define a class and a data type with the same name in different modules, then re-export them both from one module. We don't want to allow this behavior. We add a failing test to codify that this should not work. * Disallow re-exporting same class/type name (#3502) As mentioned in the previous commit (09bfea484ff6e0d77c028f11314763c794df8c47), we don't want to allow re-exporting a class and a type that have the same name. The changes here make that a reality. The majority of the tests are breaking because we're dependent on `purescript-typelevel-prelude` and it violates the changes here. See: purescript/purescript-typelevel-prelude#43 for more information. * Print the type of the conflicting name (#3502) Before, we only had conflicting exports for the same types of things. Now that we disallow exports of different types, we'll want to be a bit more explict in what the problem is. Errors we'd get before looked like: ``` Export for type B.X conflicts with A.X ``` While that does convey what the problem is, we can do a bit better. Errors now look like: ``` Export for type B.X conflicts with type class A.X ``` * Update dependencies (#3502) The changes to `purescript-typelevel-prelude` allow us to pass tests. The transitive dependency on `purescript-prelude` required an update.
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 purescript/purescript#3502 and purescript/purescript#1888), so presumably we will need to remove one of them.
Perhaps we can export a type synonym along the lines of
so that at least you don't need to separately import Prim.RowList if you were previously using Prim.RowList.Cons via Type.Row.
The text was updated successfully, but these errors were encountered: