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

new type deriving broken #115

Open
turboMaCk opened this issue Jun 2, 2021 · 8 comments
Open

new type deriving broken #115

turboMaCk opened this issue Jun 2, 2021 · 8 comments

Comments

@turboMaCk
Copy link
Member

turboMaCk commented Jun 2, 2021

Deriving instances for newtype is broken in cases like:

newtype TopicKey = TopicKey
        { unTopicKey :: Text
        } deriving stock (Show, Eq, Generic)
          deriving (FromJSON, ToJSON, Elm) via ElmStreet TopicKey

Json instances expect the value to be wrapped in tagged object while elm decoders are generated for json string values. Decoding therefore fails (at runtime).

Two other ways to derive the value work

newtype TopicKey = TopicKey
        { unTopicKey :: Text
        } deriving stock (Show, Eq, Generic)
          deriving newtype (FromJSON, ToJSON)
          deriving (Elm) via ElmStreet TopicKey

This generates instances where both expect wrapping object to be present.

newtype TopicKey = TopicKey
        { unTopicKey :: Text
        } deriving stock (Show, Eq, Generic)
          deriving (FromJSON, ToJSON) via Text
          deriving (Elm) via ElmStreet TopicKey

Derives the instance where json values are not boxed in json representation.

I think the later option is better in most cases but there is a disadvantage for types which might start as a newtype and later be changed to full data record. This often happens as hlint complains about data with single field. With 2nd approach the JSON representation in not compatible between newtype and data so changing types from newtype to data is breaking change for the api.

Related Issues

@dtaskoff
Copy link

dtaskoff commented Jul 6, 2022

up

@turboMaCk
Copy link
Member Author

@dtaskoff in meantime can you use one of the work-around mentioned above?

@dtaskoff
Copy link

dtaskoff commented Jul 7, 2022

Sure, it's just that I changed a type from data to a newtype, and spent some time wondering why it didn't work. It's even written in the documentation already (to use deriving newtype (FromJSON, ToJSON), and deriving anyclass Elm), so it's only a matter of convenience.

@turboMaCk
Copy link
Member Author

cool. There is another related but slightly different issue which is that elm-street likes to generate record aliases for new types on elm side. But semantically it would be better to better to generate regular type since elm compiler can unbox these in compile time much like Haskell does for new types. It can't do it for records though because records are real records in elm, not just field accessors like in Haskell.

@dtaskoff
Copy link

dtaskoff commented Jul 7, 2022

I'm not sure that it's a good idea to translate Haskell's newtypes into Elm's type aliases, since in Elm they are really just aliases, so type alias X = Int, and type alias Y = Int are indistinguishable.

@turboMaCk
Copy link
Member Author

turboMaCk commented Jul 7, 2022

it should not be alias. elm equivalent of newtype Foo a = Foo { unFoo :: a } is type Foo a = Foo a. There are 2 differences in type systems in play here:

  1. haskell requires newtype for runtime unboxed values. Elm implicitly unboxes all values it can for type.
  2. haskell records are just types with field accessors. Elm records are aliases to record type which has row polymorphism (can't be unboxed).

so generating types elm can unbox would not be less type safe on elm side. It would be more type safe because it would generate distinct type rather than alias so newtype Foo = Foo { fooVal :: Int } and newtype Bar = Bar { barVal :: Int } would not be the same alias on elm side as they are now.

@dtaskoff
Copy link

dtaskoff commented Jul 7, 2022

My bad, I misread type for type alias in your previous comment.

@turboMaCk
Copy link
Member Author

turboMaCk commented Jul 7, 2022

no worries. this terminology is super confusing between Elm and Haskell..

  • data vs type
  • type vs type alias

that's a recipe for misunderstanding.


I will try to bring this issue up internally at Holmusk but expect this issue will take some time to resolve. There is a lot of code in the wild which need to keep supporting without introducing major breakages.

@turboMaCk turboMaCk changed the title new type derivation broken new type deriving broken Jul 27, 2023
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

2 participants