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

Scala.js options overwritten on Scala 3 #102

Closed
keynmol opened this issue Jul 28, 2022 · 8 comments
Closed

Scala.js options overwritten on Scala 3 #102

keynmol opened this issue Jul 28, 2022 · 8 comments

Comments

@keynmol
Copy link

keynmol commented Jul 28, 2022

On Scala 3, -scalajs option is required to generate IR files. Without it, the compiler will complain, and the linking will succeed without actually doing anything.

In the app you will likely notice it immediately, in the libraries - not so much.

Desired behaviour: -scalajs option, if present, is preserved

───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
        File: ./Test.scala
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1    @main def hello = println("yo")
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
        File: ./project/plugins.sbt
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1    addSbtPlugin("io.github.davidgregory084" % "sbt-tpolecat"              % "0.4.1")
   2    addSbtPlugin("org.scala-js"              % "sbt-scalajs"               % "1.10.1")
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
        File: ./build.sbt
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1    scalaVersion := "3.1.3"
   2   
   3    enablePlugins(ScalaJSPlugin)
───────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/v/f/m/d/T/tmp.B2Uwm0IW > sbt run
[info] welcome to sbt 1.6.2 (GraalVM Community Java 17.0.3)
[info] loading global plugins from /Users/velvetbaldmime/.sbt/1.0/plugins
...
[warn] The `-scalajs` flag was missing from `compile / scalacOptions`, but it is required to produce Scala.js IR.
[warn] Linking, running and/or testing will probably go wrong.
[warn] The most likely cause is that you used `scalacOptions := ...` instead of using `++=`.
armanbilge added a commit to armanbilge/scala-js that referenced this issue Jul 28, 2022
While any Scala.js-focused project would pick up on a warning during local development, unfortunately for many projects/maintainers Scala.js is a cross-build that primarily depends on CI to be tested. If it does not error fatally the problem is unlikely to be noticed.

Context: typelevel/sbt-tpolecat#102, a popular plugin used in many cross-building projects.
@objektwerks
Copy link

I just stumbled across this issue via this project ( github.com/objektwerks/scalajs/blob/master/build.sbt ). To avoid getting this error during sbt clean test ( java.lang.AssertionError: assertion failed: asTerm called on not-a-Term val ), I had to add scalacOptions ++= Seq("-scalajs") to the build.sbt. Apparently sbt-tpolecat was removing the -scalajs option added by the Scalajs plugin ( addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.10.1") ).

@DavidGregory084
Copy link
Member

Unfortunately the obvious fix to this - appending the existing scalacOptions.value to the resulting scalacOptions after this plugin is applied - doesn't resolve the issue, or at least it causes more issues.
I've started a discussion in sbt (sbt/sbt#6986) about how to resolve this so hopefully we will get some good advice there.

@objektwerks
Copy link

Beyond simple tasks, Sbt is like an extreme sport. So be careful. :)

@davesmith00000
Copy link
Contributor

Just bumping this issue, it only occurred for me when I tried to go from sbt-tpolecat version 0.3.1 to 0.4.1. Same output as originally shown:

[info] Running scalafix on 1 Scala sources (incremental)
[warn] The `-scalajs` flag was missing from `compile / scalacOptions`, but it is required to produce Scala.js IR.
[warn] Linking, running and/or testing will probably go wrong.
[warn] The most likely cause is that you used `scalacOptions := ...` instead of using `++=`.

@DavidGregory084
Copy link
Member

Sorry for the silence on this one folks - I'm going mad trying to find a way to solve this without breaking any of the previous behaviours of the library and while still accomodating the ways that folks set the various keys of the library. The combination of +=, ++= and sbt scope delegation is very hard to navigate.

@BalmungSan
Copy link

Sorry for adding more salt to the wound, but I just found that sbt-tpolecat also causes problems with metals / semanticdb; where this plugin ends up removing the required scalac flags for it to work properly.

Apparently, the issue is related to the exclusions rule.
I didn't test with main sources, but if I do this:

Test / tpolecatExcludeOptions ++= ScalacOptions.defaultConsoleExclude

Then metals is broken on the test files.

sjrd added a commit to sjrd/sbt-tpolecat that referenced this issue Oct 28, 2022
We now maintain a set of `ScalacOptions` "managed" by sbt-tpolecat.
When computing `scalacOptions`, we do not overwrite them, but
instead we get the previous `scalacOptions.value` and we modify it.

We only remove options that are managed by sbt-tpolecat. And we
only add options that are not already there (because they were
already added in an upper delegate scope).

By default, we automatically compute the set of managed options as
all the options that are "ever" added once by sbt-tpolecat in the
delegate chain.
@sjrd
Copy link
Contributor

sjrd commented Oct 28, 2022

PR available: #126

raquo added a commit to raquo/scala-dom-types that referenced this issue Jan 3, 2023
DavidGregory084 added a commit that referenced this issue Jan 6, 2023
Fix #102: Do not overwrite other plugins' `scalacOptions`.
@keynmol
Copy link
Author

keynmol commented Jan 6, 2023

I can confirm that reproducer no longer works with a locally published version of the plugin 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants