justinwoo / purescript-simple-json

A simple Purescript JSON library that uses types automatically
http://purescript-simple-json.readthedocs.io
MIT License
133 stars 45 forks source link

Maybe type classes #16

Closed dwhitney closed 7 years ago

dwhitney commented 7 years ago

Hi I enjoy using this library. It's really reduced a lot of boilerplate for me, so thanks so much for putting it together!

One thing that I wish it had out of the box is type class instances for Maybe. Using NullOrUndefined feels a bit leaky to me, and wrapping my Maybes in newtypes just to get the functionality I wanted was becoming fairly tedious since I use Maybe so much. I also noticed there some similar sentiment on the latest PureScript Unscripted, so I decided to make this PR.

Things to note:

Hope that you find this helpful. If you'd like any changes, I'm happy to make them!

dwhitney commented 7 years ago

done! travis should be doing its work any moment

On Thu, Sep 21, 2017 at 1:58 PM, Justin Woo notifications@github.com wrote:

@justinwoo commented on this pull request.

In src/Simple/JSON.purs https://github.com/justinwoo/purescript-simple-json/pull/16#discussion_r140315536 :

@@ -143,6 +166,12 @@ instance writeForeignArray :: WriteForeign a => WriteForeign (Array a) where instance writeForeignNullOrUndefined :: WriteForeign a => WriteForeign (NullOrUndefined a) where writeImpl (NullOrUndefined a) = maybe undefined writeImpl a

+instance writeForeignMaybe :: WriteForeign a => WriteForeign (Maybe a) where

  • writeImpl (Just a) = writeImpl a
  • writeImpl Nothing = null
  • +foreign import null :: Foreign

Ah, null isn't exposed by Nullable but really, I'd even prefer toForeign $ toNullable Nothing

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/justinwoo/purescript-simple-json/pull/16#discussion_r140315536, or mute the thread https://github.com/notifications/unsubscribe-auth/AACCoXCc0i_ruMA6N2F0OXEi06emueODks5skqOvgaJpZM4Pexyq .

jacereda commented 7 years ago

Merged, thanks!