Circe: Scala.JS linking errors with 0.12.0-RC4

[japgolly]

Using 0.12.0-RC4 with Scala.JS, everything compiles but then it fails at link-time with lots of linking errors. They all seem to be related to java.time. Some of java.time is implemented for Scala.JS under "org.scala-js" %%% "scalajs-java-time" % "0.2.5" but not all of it, and not enough to prevent these linking errors.

The sad thing is that this will affect all Scala.JS users and not just those who attempt to use java.time decoders. The Decoder object has a bunch of implicit vals and Scala.JS currently doesn't eliminate them as dead-code yet, the workaround for now is to make them all lazy vals instead which does get DCE'd. Or drop back to Circe 0.11.1 but then one loses Scala 2.13 support.

Are you happy to make the out-of-the-box implicits lazy? If so, I'm happy to submit a PR.

Sample errors:

[error] Referring to non-existent method java.time.format.DateTimeFormatter$.ISO$undOFFSET$undTIME()java.time.format.DateTimeFormatter
[error]   called from io.circe.Decoder$$anon$70.parseUnsafe(java.lang.String)java.time.OffsetTime
[error]   called from io.circe.Decoder$$anon$70.parseUnsafe(java.lang.String)java.lang.Object
[error]   called from io.circe.Decoder$JavaTimeDecoder.apply(io.circe.HCursor)scala.util.Either
[error]   called from io.circe.Decoder.tryDecode(io.circe.ACursor)scala.util.Either
[error]   called from io.circe.Decoder$JavaTimeDecoder.tryDecode(io.circe.ACursor)scala.util.Either
[error]   called from io.circe.ACursor.as(io.circe.Decoder)scala.util.Either

[error] Referring to non-existent method java.time.Period$.parse(java.lang.CharSequence)java.time.Period
[error]   called from io.circe.Decoder$$anon$54.parseUnsafe(java.lang.String)java.time.Period
[error]   called from io.circe.Decoder$$anon$54.parseUnsafe(java.lang.String)java.lang.Object
[error]   called from io.circe.Decoder$JavaTimeDecoder.apply(io.circe.HCursor)scala.util.Either
[error]   called from io.circe.Decoder.tryDecode(io.circe.ACursor)scala.util.Either
[error]   called from io.circe.Decoder$JavaTimeDecoder.tryDecode(io.circe.ACursor)scala.util.Either
[error]   called from io.circe.ACursor.as(io.circe.Decoder)scala.util.Either

[travisbrown]

Thanks for the report, @japgolly! There is currently a suggested workaround in these release notes, and I was planning to highlight it more prominently in the 0.12.0 release this week.

I thought I'd tried making all of the java.time-related definitions lazy when I first moved them back into circe-core a few months ago, but to be honest I don't personally use Scala.js and I rely on contributors to make sure the Scala.js experience feels polished (to the extent that it does), so this hasn't been a priority for me. I would happily review a pull request that provides a better fix for the issue.

[japgolly]

Oh sorry about that! I didn't even think to check the release notes. Ok cool, let me have a look at and wrap my head around those different java.time implementations and get back to you :)

[travisbrown]

Yeah, it kind of sucks that we've had so many 0.12.0 pre-releases while waiting for Cats 2.0.0 that this stuff gets buried.

[travisbrown]

To summarize the situation here, Scala.js users have two choices with the current 0.12.0 release candidates and M4:

1. Use scala-java-time, an excellent but (necessarily) enormous Scala.js implementation of java.time.
2. Use not-java-time, my tiny minimal implementation that's just a horrible workaround.

There are three things we could do in 0.12.0:

1. Stick with the current approach, requiring Scala.js users to bring their own java.time implementation.
2. Find a way to fix this issue without requiring a lot of platform-specific code or a separate circe-java8 module. Making the instance vals lazy might be enough—I thought I'd tried that, but as I mention above, polished Scala.js support hasn't been a priority for me.
3. Go back to platform-specific code to keep these instances out of circe-core on Scala.js and reinstate circe-java8, at least for Scala.js (but probably for both platforms, with the JVM one being empty).

I'm personally strongly opposed to 3. and inclined toward 1., but if someone else wants to put together a proposal for 2. I'd love to see it.

[japgolly]

Thanks for that summary @travisbrown! I'm with you about option 3 being strongly undesirable.

Regarding option 1, I've had a more detailed look now. Your commit comments in not-java-time made me laugh and having a look at the implementation I understand your pain! That wouldn't have been fun to have to hack together. And that'd be pretty scary for me as a regular Scala.JS user because you end up trading those linker errors away to runtime errors if you end up actually and/or accidentally relying on it.

My ideal state for this would be the easy variant of option 2. That all the instances remain in the same, core package and things Just Work for Scala.JS users by default, and if they decide to use any java.time stuff then they need an implementation for the subset they use. Trying this out locally, blindly making all the implicit final vals in both Encoder and Decoder lazy works, and allows me to compile and link without any java.time impl on the classpath.

There's also the nice benefit that the resulting filesize is smaller. Here are the filesizes of some fully-optimised JS output:

• 1,537,372 bytes using cquiroz's java.time impl
• 1,244,706 bytes using not-java-time
• 1,191,850 bytes using lazy vals

I'll get a PR together and see what you think :)

[travisbrown]

Fixed in #1246 and #1249.

[japgolly]

Awesome! Sorry I didn't get the PR done in time but glad it all worked out well in the end. Thank you @travisbrown!