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

Encoding of NULL value #130

Closed
asieira opened this issue May 20, 2016 · 8 comments
Closed

Encoding of NULL value #130

asieira opened this issue May 20, 2016 · 8 comments

Comments

@asieira
Copy link

asieira commented May 20, 2016

This is unexpected:


> cat(jsonlite::toJSON(NULL, null = "null"))
{}

Shouldn't the output on this case be the string null instead? It would certainly be more consistent with the behavior of fromJSON:

> str(jsonlite::fromJSON("null"))
 NULL
@asieira
Copy link
Author

asieira commented May 21, 2016

I just found https://github.com/jeroenooms/jsonlite/blob/master/R/toJSON.R#L21-L24 and realized this is a deliberate attempt to avoid generating "invalid" JSON.

However, it turns out that if we are strict, only arrays and objects are valid JSON. No single value (null, string, numeric, etc) is valid JSON. So for consistency if we don't allow null as an output, we also shouldn't allow these:

> jsonlite::toJSON("a", auto_unbox = T)
"a" 
> jsonlite::toJSON(1, auto_unbox = T)
1 

I don't see any reason to make an exception just for null, and strongly suggest that edge case check is removed from the toJSON code.

@jeroen
Copy link
Owner

jeroen commented May 21, 2016

Well the idea was that at least with the default settings toJSON should always generate valid json output.

@asieira
Copy link
Author

asieira commented May 23, 2016

But if that was the case, then no JSON values (as opposed to arrays or objects) should be generated either, such as strings or numbers, right?

Also, I believe that allowing just values to be generated is the standard behavior of all JSON libraries I've seen on other languages. For example, in Python:

| => python
Python 2.7.11 (default, Dec  7 2015, 10:35:55)
[GCC 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.1.76)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import json
>>> json.dumps("foo bar")
'"foo bar"'
>>> json.dumps(None)
'null'

My humble suggestion would be that jsonlite follow this same behavior, and treat NULL the same way it treats string and numeric values.

@jeroen
Copy link
Owner

jeroen commented May 23, 2016

But if that was the case, then no JSON values (as opposed to arrays or objects) should be generated either, such as strings or numbers, right?

Exactly, the default arguments in toJSON never do this either. It only happens if you explicitly set auto_unbox = TRUE.

> toJSON(pi)
[3.1416]

Also, I believe that allowing just values to be generated is the standard behavior of all JSON libraries I've seen on other languages.

I don't disagree, but I'm reluctant to make breaking changes at this point. I have a hard time remembering why we added this special case but there probably was a good reason to avoid null outputs. I need to sleep on this for a bit :)

@asieira
Copy link
Author

asieira commented May 23, 2016

Of course, breaking changes are painful and I totally understand your reluctance. :)

For the record, I would be perfectly happy with having toJSON(NULL, auto_unbox = TRUE, null = "null") generate null and everything else stay as it is.

@asieira
Copy link
Author

asieira commented May 23, 2016

Thanks again for the awesome package and let me know if there's anything I can do to help.

@jeroen
Copy link
Owner

jeroen commented Jun 14, 2016

OK it is now changed: debfe97. Hopefully nobody was relying on this edge case.

@jeroen jeroen closed this as completed Jun 14, 2016
@jeroen
Copy link
Owner

jeroen commented Jun 15, 2016

This is now on CRAN in jsonlite 0.9.22. Just want to emphasize that we usually never change behavior of toJSON / fromJSON. Just for this tiny special edge case I make an exception.

Also note that a trick for users that want to use null="null" and auto_unbox = TRUE but still always get valid json, is to wrap the object in I().

> toJSON(I(123), null = "null", auto_unbox = T)
[123]
> toJSON(I(NULL), null = "null", auto_unbox = T)
[]

This is what e.g. shiny does, so they were unaffected by this edge case in the first place.

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

2 participants