updates for 0.12, remove foreign-generics #21
Conversation
|
Thanks! I really would like to use foreign-generics, was the hold-up just with some record stuff? Do you think we should make an interim release with this change and then update to foreign-generics later? |
|
I would like to use this library without foreign-generics at all though. Those who want to use foreign-generics could later opt in when it gets updated. |
|
How would they opt in later if this library doesn't use foreign-generic anymore? I'm skeptical of removing the |
|
They can always recover the constraint version by binding show and ForeignGeneric.decode. I would never want to use Decode in my code base which purposefully doesn't use Decode for anything. Decoding the Foreign value I get back is exactly what this PR does, where no assumptions are made but for the user to provide a function to Either. I think there is absolutely no type class that would serve everyone, and nobody really should expect one to work. This library should not force a specific decoding solution to its users. Libraries forcing very specific unrelated typeclasses is what makes them impossible to actually use with any of my other tools, and, for example, is why I don't use the purescript-express package. I wouldn't be that opposed to having a fork of this library called "node-postgres-core" and having this one maintain some Foreign-Generics constraints if the README at least pointed people at not using Decode for everything. |
|
One option that converges with the |
|
I really don't want this library to do any actual decoding on my behalf, I want to pass in all the decoding I do with concrete functions. |
|
Could we get rid of the |
src/Database/Postgres.purs
Outdated
| rows <- fromEffFnAff $ runQuery_ sql client | ||
| either liftError pure (runExcept (sequence $ decode <$> rows)) | ||
| query_ :: forall b a | ||
| . (b -> String) -> (Foreign -> Either b a) -> Query a -> Client -> Aff (Array a) |
There was a problem hiding this comment.
There is an extra space before the Query a here and also in the next few type defs.
|
All right, updated to use Error |
|
One final nitpick, the commit still says |
removes dependency on foreign-generics and allows the user to supply their own Foreign -> Either Error a function, e.g. Simple-JSON for record alias decoding.
|
okay, i updated it |
|
Thanks! |
Foreign-Generics won't be updated for a while, but the usages here seem to be leftover from pre-11.6 days. I updated this to have the user supply their own decoding function, and updated the tests to use Simple-JSON.