-
Notifications
You must be signed in to change notification settings - Fork 9
default noEmptyComposite to true #499
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
Comments
Hi, @raylu. I understand your point of view, but it comes down to backwards compatibility. When I cut a 2.x revision I can revisit defaults. I agree that the inability to roundtrip is counterintuitive, so noEmptyComposite will probably default to true. I'm less convinced about changing AQF, though. That can have a big affect on the size of encoded text. Unfortunately, I don't really have data about how users change defaults to base these sorts of decisions on. |
are you worried about people depending on the behavior of serializing existing empty arrays as at the least, I think this should be mentioned on the README. I saw the thing about (I'm curious why you're against defaulting AQF to on. isn't the entire point of JsonURL to work for URLs? are people passing them around without them hitting the address bar at some point? or did I misunderstand what AQF is for?) |
Hello, I just started making a POC with this lib. As I'm sometimes passing empty arrays in filters, I agree that the bijection of JSON -> JSONUrl -> JSON is more than mandatory to get rid of surprises. I will check the |
A fair point and good suggestion. Thanks!
Enabling It will also increase the size of stringified JSON→URL text. One user in the past has noted that size was a contributing factor when choosing JSON→URL over alternatives (#97 (comment)).
No, you did not misunderstand. That is what AQF is for, but not everyone uses JSON→URL in a browser address bar. It works well for idempotent server-side HTTP API calls too. |
as #102 shows, the fact that
JsonURL
doesn't round-trip empty arrays is pretty surprising. I can't really think of a situation where you would want to save the:
byte versus being able to round-trip the correct type(I also wouldn't mind if AQF were the default since that's the main usage of this library, I think, but I don't know if that might have some backwards compatability problems)
The text was updated successfully, but these errors were encountered: