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

Duplicate definition in Codec for shared types #3

Open
alltonp opened this issue Feb 12, 2018 · 8 comments
Open

Duplicate definition in Codec for shared types #3

alltonp opened this issue Feb 12, 2018 · 8 comments

Comments

@alltonp
Copy link

alltonp commented Feb 12, 2018

Hi,

I've been using scala-elm-types for a while and it's been working very nicey, it's awesome not having to handcode encoders and decoders :)

However, I've hit a snag when case classes share the same type, for example:

case class Ball(name: String)

sealed trait ClientToServer
case class Ping(message: String, ball: Ball) extends ClientToServer

sealed trait ServerToClient
case class Pong(message: String, ball: Ball) extends ServerToClient

The Ball type (and its encoder/decoder) are generated twice (see errors below)

Is there anyway to prevent this?

Many thanks,
Paulos


-- DUPLICATE DEFINITION -------------------------------------------- ./Codec.elm

Naming multiple top-level values `decodeBall` makes things ambiguous. When you
say `decodeBall` which one do you want?

48| decodeBall =
    ^^^^^^^^^^
Find all the top-level values named `decodeBall` and do some renaming. Make sure
the names are distinct!

-- DUPLICATE DEFINITION -------------------------------------------- ./Codec.elm

Naming multiple top-level values `encodeBall` makes things ambiguous. When you
say `encodeBall` which one do you want?

64| encodeBall obj = Encode.object
    ^^^^^^^^^^
Find all the top-level values named `encodeBall` and do some renaming. Make sure
the names are distinct!

-- DUPLICATE DEFINITION -------------------------------------------- ./Codec.elm

Naming multiple top-level values `Ball` makes things ambiguous. When you say
`Ball` which one do you want?

9| type alias Ball = { name : String }
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Find all the top-level values named `Ball` and do some renaming. Make sure the
names are distinct!

-- DUPLICATE DEFINITION -------------------------------------------- ./Codec.elm

There are multiple types named `Ball` in this module.

9| type alias Ball = { name : String }
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Search through this module, find all the types named `Ball`, and give each of
them a unique name.
@alltonp
Copy link
Author

alltonp commented Feb 12, 2018

This is the complete generated Codec.elm ...

module Codec exposing (..)
import Date exposing (Date)
import Json.Decode.Extra exposing(..)
import Json.Decode as Decode exposing ( field )
import Json.Encode as Encode
import Date.Extra exposing (toUtcIsoString)
type ClientToServer = ClientToServerPing Ping
type alias Ping = { message : String, ball : Ball }
type alias Ball = { name : String }
decodeClientToServer : Decode.Decoder ClientToServer
decodeClientToServer = Decode.oneOf
  [ (field "Ping" <| Decode.map ClientToServerPing decodePing)
  ]
decodePing : Decode.Decoder Ping
decodePing =
  Decode.succeed Ping |: (field "message" <| Decode.string) |: (field "ball" <| decodeBall)
decodeBall : Decode.Decoder Ball
decodeBall =
  Decode.succeed Ball |: (field "name" <| Decode.string)

encodeClientToServer: ClientToServer -> Encode.Value
encodeClientToServer obj =
  let
    (typefield, inner) = case obj of
      ClientToServerPing obj2 -> ("Ping", encodePing obj2)
  in
    Encode.object [(typefield, inner)]
encodePing : Ping -> Encode.Value
encodePing obj = Encode.object
  [ ("message", Encode.string obj.message)
  , ("ball", encodeBall obj.ball)
  ]
encodeBall : Ball -> Encode.Value
encodeBall obj = Encode.object
  [ ("name", Encode.string obj.name)
  ]
type ServerToClient = ServerToClientPong Pong
type alias Pong = { message : String, ball : Ball }
type alias Ball = { name : String }
decodeServerToClient : Decode.Decoder ServerToClient
decodeServerToClient = Decode.oneOf
  [ (field "Pong" <| Decode.map ServerToClientPong decodePong)
  ]
decodePong : Decode.Decoder Pong
decodePong =
  Decode.succeed Pong |: (field "message" <| Decode.string) |: (field "ball" <| decodeBall)
decodeBall : Decode.Decoder Ball
decodeBall =
  Decode.succeed Ball |: (field "name" <| Decode.string)

encodeServerToClient: ServerToClient -> Encode.Value
encodeServerToClient obj =
  let
    (typefield, inner) = case obj of
      ServerToClientPong obj2 -> ("Pong", encodePong obj2)
  in
    Encode.object [(typefield, inner)]
encodePong : Pong -> Encode.Value
encodePong obj = Encode.object
  [ ("message", Encode.string obj.message)
  , ("ball", encodeBall obj.ball)
  ]
encodeBall : Ball -> Encode.Value
encodeBall obj = Encode.object
  [ ("name", Encode.string obj.name)
  ]

@reactormonk
Copy link
Owner

Could you give me the full Scala code you're using? With the main etc.?

@reactormonk
Copy link
Owner

Even better: add a PR with a test case in https://github.com/reactormonk/scala-elm-types/blob/master/test/src/test/scala/elmtype/CompileTest.scala - to run that locally, you'll need the elm make in your $PATH.

@alltonp
Copy link
Author

alltonp commented Feb 12, 2018

Thank you for the prompt response.

Here is a complete scala example that exposes the problem:

object Main extends App {
  import shapeless.{::, HNil}
  import elmtype.ElmTypeShapeless._
  import elmtype._

  case class Ball(name: String)

  sealed trait ClientToServer
  case class Ping(message: String, ball: Ball) extends ClientToServer

  sealed trait ServerToClient
  case class Pong(message: String, ball: Ball) extends ServerToClient

  val types = ToElmTypes[ClientToServer :: ServerToClient :: HNil]().apply

  ElmTypeMain(types).main(List("-").toArray)
}

On running, in System.out you can see that the type alias for Ball (and it's decoder/encoder) is generated twice.

ElmTypeMain.main emits a chunk of elm for each individual type and as both ClientToServer and ServerToClient reference Ball it gets generated twice.

So I guess possible solutions might be:
(1) collect all the known types and do the generation in one go or
(2) remember which types are been emitted as we go and exclude any dupes

I did try to make a test case, but didn't get massively far because CompileTest.compileExternal is currently geared around testing one type at time, so might need a bit of work.

@reactormonk
Copy link
Owner

reactormonk commented Feb 13, 2018

I did try to make a test case, but didn't get massively far because CompileTest.compileExternal is currently geared around testing one type at time, so might need a bit of work.

Ok, that would explain why I didn't test for that case. Sounds like toElmTypes needs to be a bit more involved. Let me take a look.

@reactormonk
Copy link
Owner

Considering it again, you should be able to use case class Messages(m: ClientToServer, s: ServerToClient) and then ToElmTypes[Messages :: HNil]() and ignore Messages.

@alltonp
Copy link
Author

alltonp commented Feb 13, 2018

ah ha, yes that does indeed work, very elegant :)

it's a bit messy having the one extra unused type, encoder and decoder but thats probably preferable to adding a lot of complexity to the code/tests

thank you

@reactormonk
Copy link
Owner

I'll still think about a better way to do it, but the quickfix should last you that long.

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