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

Erlang R17 has maps #49

Closed
edescourtis opened this issue Feb 5, 2014 · 13 comments
Closed

Erlang R17 has maps #49

edescourtis opened this issue Feb 5, 2014 · 13 comments

Comments

@edescourtis
Copy link

Please provide some way to convert json objects into Erlang maps directly.

@talentdeficit
Copy link
Owner

i'll have something in the next week or two

the implementation is pretty straight forward i'm just trying to figure out an interface.

what i'm leaning towards now is:

$ jsx:to_list(<<"{}">>).
[{}]
$ jsx:to_term(<<"{}">>).
#{}
$ jsx:decode(<<"{}">>).
#{}
$ jsx:decode(<<"{}">>, [{objects, proplist}]).
[{}]
$ jsx:decode(<<"{}">>, [{objects, map}]).
#{}

@ransomr
Copy link

ransomr commented Feb 5, 2014

Please consider not changing the default output of jsx:decode, as it will break compatibility. For example I use jsx:decode in the erlcloud DynamoDB client. If someone were to upgrade to a new version of jsx that changed the output from a proplist to a map it would completely break erlcloud DynamoDB client.

@talentdeficit
Copy link
Owner

the other option is adding parse/stringify methods that work on maps (altho stringify will likely just be an alias for encode. i can't forsee any issues with that?)

@edescourtis
Copy link
Author

I agree with this. However having to type extra stuff every time might not
be a good idea. Instead you should provide an updated interface along with
the old one.

Consider something like jsx_ng:decode(<<"{}">>) -> #{} and keeping the old
interface as is.

That way backward compatibility can be preserved.

On Wed, Feb 5, 2014 at 10:27 AM, Ransom Richardson <notifications@github.com

wrote:

Please consider not changing the default output of jsx:decode, as it will
break compatibility. For example I use jsx:decode in the erlcloud DynamoDB
client. If someone were to upgrade to a new version of jsx that changed the
output from a proplist to a map it would completely break erlcloud DynamoDB
client.

Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-34188404
.

@ransomr
Copy link

ransomr commented Feb 5, 2014

I'm not sure I fully understand the parse/stringify proposal. What is important for compat is that the output of all existing methods doesn't change and that all arguments that are currently supported continue to be supported.

So I'm not aware of any good options other than leaving existing interface and adding a new one. Any of jsx:decode2, jsx2:decode, jsx_ng:decode, or if you prefer shorter jsx:dec, should work. Or you could do as you propose and have an {objects, map} option as long as the default stays proplist, but since map will be the preferred output going forward, would be too bad to have to add that every call.

@talentdeficit
Copy link
Owner

decode would remain exactly as it is now (barring being slightly more permissive in the input it accepts. output should remain unchanged for anything it currently accepts).

parse would work as decode except instead of returning proplists where appropriate it would return maps. so:

$ jsx:decode(<<"{}">>).
[{}]
$ jsx:parse(<<"{}">>).
#{}

@ransomr
Copy link

ransomr commented Feb 5, 2014

Perfect, sorry I misunderstood. parse would work fine.

@edescourtis
Copy link
Author

It would work but might confuse people 2 or 3 years down the road.

On Wed, Feb 5, 2014 at 5:31 PM, Ransom Richardson
notifications@github.comwrote:

Perfect, sorry I misunderstood. parse would work fine.

Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-34263503
.

talentdeficit pushed a commit that referenced this issue Feb 11, 2014
encoding only for now until interface issues are resolved. addresses #49
@talentdeficit
Copy link
Owner

encoding support is now live in the maps branch

still thinking about how best to deal with the interface issues with supporting maps and proplists at the same time

@talentdeficit
Copy link
Owner

for 2.0 i've decided to allow transparent encoding of maps to json but to pass on supporting decoding to maps for now. i'm still not convinced they are a good match for json. i will probably put up an application that extends jsx and replaces proplists with maps for those that disagree, but i doubt json -> maps will be coming to jsx core anytime soon

@edescourtis
Copy link
Author

@talentdeficit Acceptable compromise. Good work!

@talentdeficit
Copy link
Owner

see https://github.com/talentdeficit/jsxn for a wrapper around jsx that decodes to maps

@edescourtis
Copy link
Author

@talentdeficit You are amazing! Thanks!

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

3 participants