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

Cross build for ScalaJS and Scala Native #251

Closed
wants to merge 8 commits into from

Conversation

jodersky
Copy link

@jodersky jodersky commented Mar 8, 2018

This is an attempt to enable building versions of spray for ScalaJS and Scala Native.

In my opinion, spray is among the simplest and most fun to use JSON libraries for scala. It would benefit spray's adoption tremendously if it were available on all platforms targeted by Scala; even Scala Native.

Changes

The changes proposed in this pull request mostly refactor the build configuration to use sbt-crossproject to build spray for Scala, ScalaJS and Scala Native. Although I tried to keep source compatibility as much as possible, some traits needed to be re-composed because ScalaJS and Scala Native versions do not implement reflective product formats, meaning that there are no more jsonFormatX() methods on these platforms. (The overloaded method jsonFormat(<constructor>, <field_names>*) is still available and may be used as a workaround). In more detail, the changes are:

  • Code has been reorganized to an sbt-crossproject (Cross.Full) configuration. Shared code lives in shared/src and some JVM specific code is in jvm/src.

  • Methods that require Java reflection have been moved out of ProductFormats into ReflectiveProductFormats and are only available in the JVM version.

  • The mixins of DefaultJsonProtocol have been mostly grouped into a new trait SharedJsonProtocol which contains all platform-independent formats (i.e. all formats but reflective ones). DefaultJsonProtocol extends SharedJsonProtocol with ReflectiveProductFormats on the JVM.

  • Drop Scala 2.10 (not supported anymore by the latest versions of the test frameworks)

Known issues

There are no published versions of specs2 or scalacheck for Scala Native. Therefore, this platform version currently has no tests.

These changes are not binary compatible with spray 1.3.4.


I know that this pull request is quite large; however I tried to regroup commits into logical units of change. Let me know if you'd like any more explanations on individual changes.

The main motivation for this pull request was an experiment on spray format derivation using Magnolia (that I'm using in a Scala-ScalaJs-ScalaNative combo project). Hopefully, if Scala gets official support for some form typeclass derivation, we won't need reflective formats anymore and spray-json can have a 100% uniform api on all platforms. In the mean-time, IMO it is still worthwhile to publish spray for other platforms, even if reflective formats aren't available on all.

@jodersky jodersky force-pushed the cross-build branch 3 times, most recently from d38d339 to 23f5855 Compare March 8, 2018 05:20
@jodersky
Copy link
Author

jodersky commented Mar 8, 2018

NB: the symlinks to boilerplate templates can be removed once support for multiple directories is implemented in sbt-boilerplate. There's a pull request pending: sbt/sbt-boilerplate#26

@jrudolph
Copy link
Member

jrudolph commented Mar 8, 2018

Thanks, @jodersky. I agree with the motivation that spray-json should be available on all platforms.

What we won't do is publish binary incompatible versions for the JVM. So, we will have to find another way of separating code files to remove the jsonFormatX ones for the other platforms.

I find the magnolia based derivation quite appealing so maybe we could provide that everywhere and start a migration path in that direction? Imo non-implicit derivation could be fine as a replacement for the jsonFormatX methods for now.

WDYT?

To manage expectations, we haven't had much time lately to maintain spray-json, so it might be a while until we are able to review, merge and release anything.

@jodersky
Copy link
Author

jodersky commented Mar 8, 2018

Thanks for getting back so quickly. I absolutely understand your position and in fact the approach I used in an internal project did keep binary compatibility, at the expense of some code duplication however. ProductFormats and ProductFormatsInstances were duplicated without jsonFormatXs on the other platforms (no SharedJsonProtocol or ReflectiveProductFormats, as described above, were required). If the trade-off is acceptable, I can take that approach here too.

In regards to using Magnolia for format derivation, are you suggesting to include derived formats in spray-json proper? I'm not sure how robust Magnolia is yet, and furthermore I'm reluctant to add a dependency to spray.
My recommendation would be to begin by adding build support for the other platforms, leaving out any reflective or derived formats just yet. Once we have multi-platform support in place, we can decide how we want to approach format derivation. In the mean-time, we can just point people to the external spray format derivation library (it could of course also be moved/forked and tweaked into the spray organization).

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

Successfully merging this pull request may close these issues.

2 participants