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

Prescribe a way to encode JSON in the views #657

Closed
jeregrine opened this Issue Mar 5, 2015 · 13 comments

Comments

Projects
None yet
5 participants
@jeregrine
Member

jeregrine commented Mar 5, 2015

Lots of discussion happening around weather you should use defimpl (https://gist.github.com/drewolson/ddcbedbc4780c5066bbd) or if you should just return maps from your view's render function.

@paulcsmith

This comment has been minimized.

Contributor

paulcsmith commented Mar 13, 2015

Just chiming in that I think it would be valuable to have some kind of convention/best practice here. I like @drewolson way of doing this, but I'm not sure what the pros/cons of the different strategies are.

@drewolson

This comment has been minimized.

drewolson commented Mar 13, 2015

@josevalim

This comment has been minimized.

Member

josevalim commented Mar 13, 2015

Nice example @drewolson! Quick question though: shouldn't you pass the options forward to when you call encode instead of passing an empty list?

@drewolson

This comment has been minimized.

drewolson commented Mar 13, 2015

@josevalim that's a really good point, I hadn't thought about it. I'll update the example.

@josevalim josevalim changed the title from Prescribe a way to encode JSON in the views. to Prescribe a way to encode JSON in the views Mar 14, 2015

@josevalim

This comment has been minimized.

Member

josevalim commented Mar 14, 2015

@drewolson btw, have you implemented Poison.Encoder for some default Ecto types like Date, DateTime, Time and Decimal? If so, can you please point me to your implementation so I can add them to phoenix_ecto?

@drewolson

This comment has been minimized.

drewolson commented Mar 14, 2015

@josevalim I've only done something for Ecto.Date, but I hadn't used an encoder. That's a really good idea, I'll move to it. My current implementation looks something like this -- https://gist.github.com/drewolson/04440d19b837dc6b4652

@josevalim

This comment has been minimized.

Member

josevalim commented Mar 14, 2015

@josevalim

This comment has been minimized.

Member

josevalim commented Mar 14, 2015

The big question is if we should serialize decimals as a string or as a float. :(

@hahuang65

This comment has been minimized.

Contributor

hahuang65 commented Mar 17, 2015

@josevalim I do this for Ecto.Date and Ecto.DateTime.
Just uses Ecto's builtin functions to convert to ISO8601.

https://gist.github.com/hahuang65/aeadef311a6f8ec44d8e

@paulcsmith

This comment has been minimized.

Contributor

paulcsmith commented Mar 17, 2015

Note that in the latest version of phoenix_ecto you get datetime JSON encoding for free: https://github.com/phoenixframework/phoenix_ecto/blob/716224086e7e2c1bc613e0eaed15d1a1d9d1c808/lib/phoenix_ecto/json.ex

@hahuang65

This comment has been minimized.

Contributor

hahuang65 commented Mar 17, 2015

I was just gonna link that :)
Jose's link above is outdated. See Paul's link.

@jeregrine

This comment has been minimized.

Member

jeregrine commented Apr 13, 2015

@josevalim we can probably close this one in conjunction with #739 ?

@josevalim

This comment has been minimized.

Member

josevalim commented Apr 13, 2015

render_one and render_many are a huge step but not everything. What is left are the generators + guides but we can tackle it on the side.

@josevalim josevalim closed this Apr 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment