snapframework / snap

Top-level package for the official Snap Framework libraries, includes the snaplets API as well as infrastructure for sessions, auth, and templates.
http://snapframework.com/
BSD 3-Clause "New" or "Revised" License
455 stars 68 forks source link

aeson-1.0.0.0 support #180

Closed bergmark closed 8 years ago

bergmark commented 8 years ago

There are two compilation error against the newest aeson, they can be fixed in different ways.

Which one should we go for?

/Users/adam/repos/3rd/snap/src/Snap/Snaplet/Auth/Backends/JsonFile.hs:303:7: error:
    • Overlapping instances for ToJSON UserIdCache
        arising from a use of ‘.=’
      Matching instances:
        instance (ToJSON v, ToJSONKey k) => ToJSON (Map k v)
          -- Defined in ‘aeson-1.0.0.0:Data.Aeson.Types.ToJSON’
        instance ToJSON UserIdCache
          -- Defined at src/Snap/Snaplet/Auth/Backends/JsonFile.hs:92:10
    • In the expression: "uidCache" .= uidCache uc
      In the first argument of ‘object’, namely
        ‘["uidCache" .= uidCache uc, "loginCache" .= loginCache uc,
          "tokenCache" .= tokenCache uc, "uidCounter" .= uidCounter uc]’
      In the expression:
        object
          ["uidCache" .= uidCache uc, "loginCache" .= loginCache uc,
           "tokenCache" .= tokenCache uc, "uidCounter" .= uidCounter uc]

/Users/adam/repos/3rd/snap/src/Snap/Snaplet/Auth/Backends/JsonFile.hs:314:11: error:
    • Overlapping instances for FromJSON UserIdCache
        arising from a use of ‘.:’
      Matching instances:
        instance (FromJSONKey k, Ord k, FromJSON v) => FromJSON (Map k v)
          -- Defined in ‘aeson-1.0.0.0:Data.Aeson.Types.FromJSON’
        instance FromJSON UserIdCache
          -- Defined at src/Snap/Snaplet/Auth/Backends/JsonFile.hs:95:10
    • In the second argument of ‘(<$>)’, namely ‘v .: "uidCache"’
      In the first argument of ‘(<*>)’, namely
        ‘UserCache <$> v .: "uidCache"’
      In the first argument of ‘(<*>)’, namely
        ‘UserCache <$> v .: "uidCache" <*> v .: "loginCache"’
mightybyte commented 8 years ago

Hmm, I don't know much about the OVERLAPS pragmas. Maybe we should go for the conditional?

bergmark commented 8 years ago

It's pretty simple (but perhaps my explanation makes it sound hard 😞 ); Marking an instance as OVERLAPPING allows it to overlap other instances, marking it as OVERLAPPABLE allows for other instances to overlap it. If A should overlap B only one of these are needed, but having both doesn't hurt. Finally OVERLAPS is OVERLAPPING+OVERLAPPABLE which is the same as having the instance in a module with -XOverlappingInstances enabled.

But I agree that the conditional is better, overlapping instances should be avoided when possible IMO.

I'll send a PR.