Also, I am wondering if introducing the new type names was really necessary. As anyway Safe, Base and Raw modules are supposed to be removed in yojson 2. Moving to Safe.t today doesn’t help with the future.
So I think that all library should stick to using json instead of t for now. Locally disable the deprecation warning where necessary. And stay compatible with most of the existing yojson and code out there.
Just to clarify, you’re suggesting that there shouldn’t be an upper bound on Yojson for old versions of ppx_deriving_yojson using the deprecated types right ?
I personally agree and I think deprecation warnings aren’t really well handled by the current tooling. If I knew how much of a mess it would’ve been I wouldn’t have deprecated the type but unfortunately it’s there now. I’m deeply sorry for the inconvenience.
Would a new release of ppx_deriving_yojson solve the issue for you ? I think there’s one underway, I’ll try to speed this up as much as I can but ppx_deriving_yojson maintainers have a lot on their plate and I’m on a holiday trip so that doesn’t help.
Exactly. Removing the < 1.6 limit that was added recently or turning it into < 2.0 instead.
I don’t really have an issue right now, it’s possible to work in the current situation. So I don’t feel an urgent need for a release. And I don’t want to disturb your holidays. The lack of contributors to ppx_deriving_yojson is indeed a problem.
My opinion is that https://github.com/ocaml-ppx/ppx_deriving_yojson/pull/90 should not have been merged, ppx_deriving_yojson should instead add annotations around the code it generates to locally disable the deprecation warning. But maybe I am missing something and it’s not possible.
Well, if more people were helping you maintaining the project, maybe the potential issues would have been spotted earlier. You are not to blame here, it’s already nice that you work on the project.
Imho ppx_deriving_yojson needs redesing anyway (we need proper error messages at least instead of Error: type), so yojson redesign would become a good motivation for this.
As the one who added the type, let me explain my reasoning:
I wanted to have show, equal and compare.
And since the convention is to use these names only if the type is called t and I can never remember the arbitrary name of the type in Yojson.Safe I thought it would be a good idea to have this type be called t. In places where I use Yojson I keep on stumbling over these all the time so that’s why when Yojson moved into community-maintenance I decided to submit fixes for that are tripping me up personally.
Why didn’t I wait for unification of Base, Safe etc? Because at the time it was not on the table. I wanted to do a thing now and make it possible to adapt in my code soon, without having to redesign the library. You see the way it was added was backward-compatible apart from the deprecation. Ironically I’m still stuck on an older version of Yojson, since I also depend on ppx_deriving_yojson whose latest release is not compatible with the new t-type containing Yojson.
Overall I still stand by this, though marking the safe type deprecated has unintentionally caused more issues than intended. Possibly @Khady is right and ppx_deriving_yojson should be changed to instead silence the warning. Or yojson 1.6.1 released which removes the deprecation (and reverts the ppx_deriving_yojson change) to be added at a later point in time. I think both approaches are pretty simple to do.
In particular I have relatively little incentive to fix that module split (apart from pulling out cppo of this whole thing) and forking Yojson to some other name makes it worse by having me to decide between the Yojson that has these annoying issues but is in use or Yojson2 that does not but also is not used by anyone. In the end I will probably end up with both in my dependency tree, which would be annoying.
Back when I was working on the ppx_deriving_yojson I started looking at developing yet another json library. The result was jsonxt, a modular library supporting various levels of standard compliance with similar levels of performance to yojson (around 95% with ocaml 4.05).
Unfortunately, real life intervened and I haven’t had the time to polish it up and make it into a releasable package. However, it’s pretty close and if people are interested I would be happy to look at bashing it into shape and releasing it or release it to the community to work on it collectively.
On a slightly different topic. I think we should re-factor ppx_deriving_yojson into a library independent core and json library dependent part. Then we can create ppx_deriving_jsonm etc without developing the whole ppx deriver. From what I have seen this should be pretty staight forward. What do people think?
Hi,
I’ve actually created a library[1] which does exactly that; a ppx for general type extraction + drivers which contains the specific serialization code. Its mostly feature complete (only inline records and gadt’s are not supported yet).
Currently, the library provides drivers for json (yojson, ezjsonm), yaml, msgpack and xml serialization and allows for users to easily create their own.
It supports the same features as ppx_deriving_yojson (incl @key, @name and @default attribute handling) + a few more type constructs (e.g nonrec types), so it can be used as a plugin replacement for ppx_deriving_json [2].
/Anders
[1] GitHub - andersfugmann/ppx_protocol_conv: Pluggable serialization and deserialization of ocaml data strucures based on type_conv
[2] The generated json output only differs on ADT type constructors without arguments which are serialized to strings, whereas ppx_deriving_yojson serializes these to singleton lists.
It would be nice if we could settle for a single JSON type in the whole opam universe and release it as a slim package called json or json0 (similar to sexplib0) containing just the type or maybe some basic functions. But it seems impossible to me, because of tension between performance and correctness: to use int/float or bignum/rational types. The json spec does not limit precision, so to be safe you need to handle arbitrary precision integers and decimals. And even though Zarith is more-or-less a community standard, not every target would want to take that as a dependency (e.g. JavaScript, microcontrollers).
(I would prefer Zarith and literals being part of the language…)
It does not limit precision but RFC8259 suggests that if you care about interoperability you should stick to those numbers that can be represented by an IEEE754 64-bit floating point number (e.g. if you care about your data to be read back correctly by JavaScript’s JSON.parse, which you should).
People that need integers larger than 2**53-1 in JSON represent them as strings (which always makes me smile).
I think one feature may be missing, the ability to ignore keys in incoming objects which are not in the target record ([@@deriving yojson {strict = false}]). That could save an application from breaking after a server upgrade in case on has not closely monitored the protocol development and anticipated a new key.
But I like the design of ppx_porotocol_conv, esp. the abstraction from the underlying format.
Actually, the provided ppx_protocol_conv drivers does not require exact fields in input, so they they have the same behavior as {strict=false} I will update the documentation.
Thanks.
/Anders
Seeing that the latest version of Yojson on OPAM is still 1.7.0, I assume that this topic is still relevant?
If you’re willing to make API-breaking changes, might I suggest matching the JSON type defined in the Ezjsonm library? I noticed that some other libraries use Ezjsom’s JSON type, in particular yaml and mustache. I am currently writing my own library, and I’m using Ezjsonm’s JSON type simply because I intend to use this library alongside yaml and mustache. But, a lot of other projects use Yojson, and if the entire OCaml ecosystem could agree on a standard JSON type, that would greatly improve interoperability among the different libraries.
Thank you for all your work on Yojson!
If anything, it’s ezjsonm that should use yojson types, isn’t it? Yojson is much, much more widely used.
I have to agree with @c-cube here, I think it would have much less of an impact on the opam universe if ezjsonm were to align than the other way around. Otherwise I’m totally in favor of unifying the types indeed!
I suppose you are talking about this (yojson) and this (ezjsonm).
The problem is that the Yojson representation is an interpretation of what JSON is, not a representation. There are no integers in JSON, only floats. And if you are going to operate on geometrical data like geojson you can’t believe how annoying these Ints and Floats are going to be (not even talking about conversion costs).
I think it would make sense to make Basic.t a representation of JSON whereas Safe.t could be a convenient (for those for whom integers are convenient) interpretation of it. There could even be conversion functions between those.
Though I would suggest we concentrate on releasing 2.0.0 with the current (mostly minor and hopefully not affecting too many people) breaking changes and then rethink the Basic/Safe/Raw variants for 3.0.0 (along with some maintainability updates).
Though I would suggest we concentrate on releasing 2.0.0 with the current (mostly minor and hopefully not affecting too many people) breaking changes and then rethink the
Basic/Safe/Rawvariants for 3.0.0 (along with some maintainability updates).
Why not bite the pill and do the breaking changes now, though? 
It’s easier to migrate only twice, I think. Besides, I imagine that a
lot of people use Safe.t or use Basic.t through code-generators and
ppxes?
While I see the appeal of doing a big-bang release instead of a constant stream of breakage I can see the ideas in the RFCs at least somewhat controversial.
The current master does exactly what the current release can do, just the types are different, so for users it it is at worst a simple search & replace. But removing `Tuple would make functionality that people use just go away completely with no trivial way to upgrade. So I would like to make sure that this is a change that we can do safely without splitting the ecosystem into old-Yojson and new-Yojson or delaying the release forever to find just the perfect set of changes.
ezjsonm will not change, due to the reasons that @dbuenzli outlined. The ejzsonm value types are the simplest types that represent a parsed version of the underlying jsonm representation. This approach means that you can easily drop down to an incremental parse with jsonm if you can’t store the full value in memory with ezjsonm.
However, I’d be happy to merge in conversion functions from Yojson into Ezjsonm if appropriate, since they are just polymorphic variants (so no dependency is needed from ezjsonm to yojson directly).