play-json
play-json copied to clipboard
playframework
Integrating with ScalaJSON AST
The efforts from both SLIP-28 and the scalaplatform has finally been materialized, ScalaJSON AST is now released on the scalaplatfrom with its first milestone release (see https://github.com/mdedetrich/scalajson for more information). Its expected that a final release will be made in a few days pending that there aren't many major issues.
This ticket is meant to deal with any potential migration issues, also note that there already exists an integration at https://github.com/mdedetrich/playframework/tree/scalaJsonAstIntegration (although this is with an older version of Play 2.5).
There are different ways to integrate the AST, one would be with (mainly) source compatibility which is the reason behind https://github.com/mdedetrich/playframework/blob/b0934ce452d82312f37cd62af28524c6840a4c45/framework/src/play-json/src/main/scala/play/api/libs/json/package.scala#L47-L58. Another alternative would be to just deal with the fact that JsValue is now just JValue (same goes for all other json types). In any case I suspect that this is going to be a big breaking change that will require a major release (play-json version 2.7.x to coincide with play 2.7 release).
I can also help with the integration once I know what the goals are
My thought is that for 2.7.0 we could provide an integration layer to convert between JsValue and scalajson JValue. Maybe for 3.0.0 we could switch to scalajson as the only AST, though I haven't fully considered the implications of that on backwards compatibility. Note that play-json has a separate release cycle and versioning scheme from Play Framework itself, though Play does depend on play-json.
Honestly, I would prefer if scalajson allowed us to extend traits, and only provided a default implementation. So Play JSON JsObject would also be a scalajson JObject. This would make it possible to provide integration with minimum breakage. There are a lot of older libraries out there using the play-json AST, and while the AST is very similar at a basic level to the scalajson one, we provide a lot of functionality on top that would have to be migrated in some way. I understand your desire to provide certain performance and behavior guarantees in scalajson, but existing JSON libraries already provide those guarantees. If we could simply extend an interface, our users could get started using scalajson right away.
For the most part I agree with the reasoning here so I'm not going to re-state it: https://github.com/spray/spray-json/issues/232#issuecomment-312220600
My current thought is that initially we should provide a conversion module and if scalajson gets wide adoption we could consider supporting it more directly at a low level, with the proper type aliases, etc., though that will be a considerable migration effort for users. We have had a number of complaints about backwards compatibility in play-json and Play in general from our users, so we are trying to move in the direction of less breakage rather than more.
I doubt we would ever use the unsafe API for parsing since I don't really see the benefits there.
If you see the PR I created before, its actually possible to integrate ScalaJSON with very minimal source breakage. Although the PR I linked before is for an old version of Play, I think only a couple of methods actually changed which broke source compatibility (mainly dealing with number conversions).
As @Ichoran mentioned, the design of the AST makes it a bit harder for frameworks to integrate but provides a much better net result for end users. Unfortunately a lot of this pain is due to the fact that we didn't have a JSON AST to start off with, so every framework decided to implement their own (which only differs in trivial cases). You would have the exact same problem with a standard String
So completely understand that a full conversion may take a while, there are different ways you can approach this. Some JSON framework libraries like Circe decided to only make a conversion module (it makes sense for their case because the internal Circe JSON AST is completely different to ScalaJSON and also they are going to decouple it in the future). However since the PlayJSON AST is very similar, the ideal route would be to do what was done in the PR (which I can recreate for the latest PlayJSON AST) which would maintain almost complete source compatibility.
You can then do stuff like add @deprecated to JsObject to tell end users to use JObject. Also understand if you don't find a use case for the unsafe AST, this is expected.
I am not opposed to eventually replacing the AST if there is a general consensus in the community that this is the right AST to use. As of right now I prefer to take the more conservative approach of doing the conversion. That solves the problem of being able to interoperate with other JSON libs, which as I understand it is the core problem scalajson is trying to solve. I'm a bit worried we are rushing things here so I would like to take the time before forcing changes onto our users.
@gmethvin If I produced a PR which integrated the AST without performance regressions/maintaining source compatibility would that help sway your mind? The stance is also confusing because its somewhat different to what was mentioned from the play authors some time ago.
The community consensus is not going to happen automatically, its a chicken and egg problem because Scala (unlike almost every other mainstream language barring Java) didn't provide a common way of parsing/working with JSON. In Java's case they provided a good strong implementation early enough that they didn't end up with 7 or so competing JSON AST. You can have a look at Haskell's String issue to see that unless a hard stance is made, community adoption is not going to happen "naturally"
If I produced a PR which integrated the AST without performance regressions/maintaining source compatibility would that help sway your mind? The stance is also confusing because its somewhat different to what was mentioned from the play authors some time ago.
I don't think our position has really changed. We are interested in integrating with a common JSON API, either through an integration API or through a different AST. We are simply applying the same caution we would for any new library. It seems like there are still some design decisions being made and work being done, and I'm not willing to commit to an API that's still changing and that other JSON library authors aren't sold on yet.
The community consensus is not going to happen automatically, its a chicken and egg problem because Scala (unlike almost every other mainstream language barring Java) didn't provide a common way of parsing/working with JSON. In Java's case they provided a good strong implementation early enough that they didn't end up with 7 or so competing JSON AST. You can have a look at Haskell's String issue to see that unless a hard stance is made, community adoption is not going to happen "naturally"
In Java's case, the only JSON standard I know is JSR-353, which happened fairly recently. Jackson and GSON are both very popular JSON libraries for Java (and Scala) and neither of them implement JSR-353 directly as far as I know. If anything the case with Java is an example of what happens when you try to standardize something too late.
I don't think our position has really changed. We are interested in integrating with a common JSON API, either through an integration API or through a different AST. We are simply applying the same caution we would for any new library. It seems like there are still some design decisions being made and work being done, and I'm not willing to commit to an API that's still changing and that other JSON library authors aren't sold on yet.
Understood, I don't have a disagreement with this although I am not sure what you mean when you say community consensus
If you mean the general Scala community in general using the scalaJSON then this isn't going to happen anytime soon (this is the chicken vs egg problem).
If you mean the other authors of JSON libraries/frameworks broadly agreeing with the design of scalaJSON, then there isn't any disagreement here (and actually this was the state because there weren't objections to the design until it was stated it was going to be released). Some things however (like making the AST a trait) are not going to happen because its not solving the actual problem that scalaJSON is meant to solve (I also don't thing reactive-streams is an apt comparison, one is a protocol and one is an AST with a strict definition)
In any case, I suspect that these design decisions will be resolved quite soon, the only exception being https://github.com/mdedetrich/scalajson/pull/19, although I am not sure how big of a deal this is.
In Java's case, the only JSON standard I know is JSR-353, which happened fairly recently. Jackson and GSON are both very popular JSON libraries for Java (and Scala) and neither of them implement JSR-353 directly as far as I know. If anything the case with Java is an example of what happens when you try to standardize something too late.
Java is still doing a good job in this regard (i.e. look what happened with JodaTime which turned into java.date._).
Also what happened with Java is quite different to what happened in Scala. In Java we had 2 major JSON implementations (Jackson and GSON) and all of the libraries/frameworks broadly used those implementations.
In Scala we had a massive NIH syndrome and every framework/library just implemented their own JSON library AST. In some cases there were legitimate reasons, i.e. fragility with lifecycle of applications/binary compatibility meant that frameworks such as play had to use their own AST because otherwise they would be tied to the release cycle of some other AST (and this other AST may not have supported older versions of Scala, or Scala.js). Java just has a much better compatibility story than Scala in this regard.
Remember one of the goals of the ScalaJSON AST is to prevent stuff like this happening https://github.com/tminglei/slick-pg#built-in-supported-typemappers , i.e. any library that wants to work with JSON has to maintain release against 6 or 7 JSON frameworks. The idea is that eventually slick-pg would only release against scalaJSON, which would inturn work with other json libraries like play-json or spray-json
This is also why I advise against just providing a conversion module unless there is a strong reason why. In Circe this is justified (internal AST is completely different, they are going to decouple AST's in the future) however with play-json (as demonstrated in this branch here https://github.com/mdedetrich/playframework/tree/scalaJsonAstIntegration), the AST's are almost the same. I understand its important to maintain compatibility for users, but as shown I don't see this being a problem unless you have an issue with another dependency (and there shouldn't be an issue because its basically built for everything barring scala-native).
If you have design/performance issues with how it would work within Play you are more then welcome to submit a PR (its an open source project) or create an issue for it, however something like "lets make the AST an interface" is pointless because all it does is allow a library framework to say "we now implement/extend this trait" without providing any real benefit for interopt/users (i.e. this is overengineering with no real benefit)