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

mdoc.js is brittle to Scala.js updates #601

Closed
armanbilge opened this issue Jan 5, 2022 · 12 comments · Fixed by #629
Closed

mdoc.js is brittle to Scala.js updates #601

armanbilge opened this issue Jan 5, 2022 · 12 comments · Fixed by #629

Comments

@armanbilge
Copy link
Contributor

armanbilge commented Jan 5, 2022

Using mdoc.js is fantastic :) However, there is some friction surrounding updates. Here are some thoughts/notes.

  1. Don't put the Scala.js compiler and linker on the same classpath. See Update Scala.js to 1.8.0 #597 (comment) for details. It seems that this is what created problems with the updates to Scala.js 1.6, 1.7, and 1.8.
  2. mdoc.js breaks if the user project uses a newer Scala.js than what mdoc was published on. I wonder if sbt-mdoc can read the scalaJSVersion from sbt-scalajs in the user's build, and bring in the right version of its dependencies. Otherwise, publishing for every Scala.js (minor) release is necessary, or users will have to apply a workaround (see 3 below).
  3. mdoc.js is published with only the binary suffix e.g. _2.13, however it includes a dependency to the Scala.js compiler with the full suffix e.g. _2.13.6. This created errors when I attempted to manually add the latest Scala.js linker/compiler dependencies in my 2.13.7 project, as a workaround for problem (2).
[error] Modules were resolved with conflicting cross-version suffixes in ProjectRef(uri("file:/home/runner/work/http4s-dom/http4s-dom/"), "docs"):
[error]    org.scala-js:scalajs-compiler _2.13.7, _2.13.6
@olafurpg
Copy link
Member

Using class loaders would solve the issue. If you load the compiler in one class loader, and the linker in another one, they won't conflict.

This sounds like a good starting point to address this issue without infrastructural changes.

mdoc.js breaks if the user project uses a newer Scala.js than what mdoc was published on. I wonder if sbt-mdoc can read the scalaJSVersion from sbt-scalajs in the user's build, and bring in the right version of its dependencies.

This also sounds like a good approach to mitigate problems. The sbt plugin can add any custom sbt settings/tasks similar to how it already updates libraryDependencies

libraryDependencies ++= {

mdoc.js is published with only the binary suffix e.g. _2.13, however it includes a dependency to the Scala.js compiler with the full suffix e.g. _2.13.6. This created errors when I attempted to manually add the latest Scala.js linker/compiler dependencies in my 2.13.7 project, as a workaround for problem (2).

We've gone back and forth on whether mdoc should include the full Scala version in the binary name. Because mdoc only touches a small part of the public scala-compiler API (trigger compilation, read diagnostics) that rarely breaks we concluded it was better to not go that route because 1) it would dramatically complicate the mdoc build and 2) require users to bump up their mdoc version every time they upgrade Scala.

@armanbilge
Copy link
Contributor Author

Thanks for the response.

We've gone back and forth on whether mdoc should include the full Scala version in the binary name.

I agree with the trade-offs here, except we are already in this sort of not-so-nice situation :) e.g. consider the situation in outwatch/outwatch#573 (comment) which requires lots of manual intervention and magic versions.

Although I suspect solving problem (2) well would make problem (3) go away.

@olafurpg
Copy link
Member

Although I suspect solving problem (2) well would make problem (3) go away

I agree. Full cross-compilation is helpful when you depend directly on source compatible but binary incompatible APIs. That’s not the case here because my understanding is that the APIs we touch from Scala.js are binary compatible.

@armanbilge
Copy link
Contributor Author

I need to study exactly what triggers the Modules were resolved with conflicting cross-version suffixes warning in sbt. One sort of tricky solution could be to "erase" the sjs compiler dependencies from the published mdoc.js pom with the expectation that they'll be injected by the user (most likely automatically via the sbt plugin). Then, even if the scala versions are conflicting there'd be no way for sbt to detect this.

@olafurpg
Copy link
Member

Are there any sbt settings we could inject with the plugin to resolve the cross-version conflicts?

@olafurpg
Copy link
Member

One sort of tricky solution could be to "erase" the sjs compiler dependencies from the published mdoc.js pom with the expectation that they'll be injected by the user

i would prefer to erase the dependency from the sbt plugin side. Many people use mdoc outside of sbt

@armanbilge
Copy link
Contributor Author

Are there any sbt settings we could inject with the plugin to resolve the cross-version conflicts?

I think we can do what we need with an exclude.

libraryDependencies ++= {
val isJS = mdocJS.value.isDefined
if (mdocAutoDependency.value) {
val suffix = if (isJS) "-js" else ""
List("org.scalameta" %% s"mdoc$suffix" % BuildInfo.version)
} else {

@armanbilge
Copy link
Contributor Author

To be able to access the user's Scala.js version in the mdoc plugin, we'd need to add a dependency to the Scala.js plugin. @olafurpg would that be ok?

https://github.com/scala-js/scala-js/blob/12285ab2aa365463f70a8e339bc32606fd4bc167/sbt-plugin/src/main/scala/org/scalajs/sbtplugin/ScalaJSPlugin.scala#L34

@keynmol
Copy link
Collaborator

keynmol commented Feb 20, 2022

I am currently working on this.

@olafurpg
Copy link
Member

To be able to access the user's Scala.js version in the mdoc plugin, we'd need to add a dependency to the Scala.js plugin. @olafurpg would that be ok?

We should avoid the direct scala-js dependency from sbt-mdoc if we can avoid it. You can often avoid the direct dependency by either using sbt reflection (TaskKey[String]("jsVersion").value) or Java reflection

classloadedSetting(
ref,
"org.scalajs.linker.interface.StandardConfig",
"scalaJSLinkerConfig"
).value.map { config =>

@keynmol
Copy link
Collaborator

keynmol commented Feb 21, 2022

@olafurpg I'm doing this and more in #629 :)

@armanbilge
Copy link
Contributor Author

Yes it's in good hands. Although @keynmol I think the sbt reflection thing might be better if possible.

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 a pull request may close this issue.

3 participants