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.

Sign up for GitHub

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

Jump to bottom

Don't use CrossVersion.for3Use2_13 in Scala.js #1054

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't use CrossVersion.for3Use2_13 in Scala.js #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/src/main/scala/com.olegych.scastie.api/ScalaTarget.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,7 @@ object ScalaTarget {
else scalaJsVersion.split('.').head)
)

def renderSbt(lib: ScalaDependency): String =
s"${renderSbtCross(lib)} cross CrossVersion.for3Use2_13"
def renderSbt(lib: ScalaDependency): String = renderSbtCross(lib)

def sbtConfig: String = {
s"""|$sbtConfigScalaVersion
Expand Down
2 changes: 1 addition & 1 deletion project/SbtShared.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ object SbtShared {
val sbt = latest212
val jvm = latest213
val cross = List(latest210, latest211, latest212, latest213, old3, js, sbt, jvm).distinct
val crossJS = List(latest212, latest213, js).distinct
val crossJS = List(latest212, latest213, stableLTS, js).distinct
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be old3. The reason is that Scastie can run all Scala versions and 3.0.0 is the one that we can use in all newer versions. If we use stableLTS, we won't be able to use versions below it.

Copy link
Contributor Author

@JD557 JD557 Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. I missed that (I thought you could only pick LTS and Next).

I do get an error compiling apiJS3 when I change that:

[error] error while loading $throws$package$,
[error] class file scala/runtime/$throws$package.class is broken, reading aborted with class dotty.tools.tasty.UnpickleException
[error] TASTy signature has wrong version.
[error]  expected: {majorVersion: 28, minorVersion: 0}
[error]  found   : {majorVersion: 28, minorVersion: 1}
[error]
[error] This TASTy file was produced by a more recent, forwards incompatible release.
[error] To read this TASTy file, please upgrade your tooling.
[error] The TASTy file was produced by Scala 3.1.3-bin-nonbootstrapped

I'll see if I can find out what's going on.

Copy link
Collaborator

@rochala rochala Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I know the reason. So this is caused because the Scala version is lower than some library on classpath.

So the fix has to be something different. The exact cause why we need this cross compilation is for our runtime macros that we use in worksheet to display value decorations. The error you see comes from play json dependency.

If I may suggest something:
Leave the versions 2.12, 2.13 and js.
Then we should only handle the runtime-scala dependency to always cross compile:

"org.scastie" %%% "runtime-scala" % "1.0.0-SNAPSHOT" cross CrossVersion.for3Use2_13

and handle the others in the way you've changed it to work.

Copy link
Contributor Author

@JD557 JD557 Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that initially, but that causes another problem: runtime-scala includes scalajs-dom, so in this case it would pull the 2.13 version

So any Scala 3 library that includes scalajs-dom then fails. :(

Maybe one alternative would be to use "org.scastie" %%% "runtime-scala" % "1.0.0-SNAPSHOT" cross CrossVersion.for3Use2_13 if the scala target is lower than 3.3.0 and "org.scastie" %%% "runtime-scala" % "1.0.0-SNAPSHOT" otherwise.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a fix for that in my Scala-CLI PR. I dropped play json and it should work. Until then I think there is nothing we can do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I wasn't aware of #997, that looks pretty exciting.

I'll eagerly wait for that, then 🙏

}

object ScalaJSVersions {
Expand Down
Loading