Allow controlling linker re-use on Scala.js #456

keynmol · 2021-02-06T11:19:34Z

This allows users to work around #454 (which is just scala-js/scala-js#4416 wearing a fake mustache) at the cost of some speed penalty

How did I test this?

pre-req: Publish this branch locally

Via the reproduction script https://github.com/keynmol/mdoc-scalajs-problem/blob/master/repro.sh:

js-batch-mode=true

❯ MDOC_VERSION_OVERRIDE='2.2.16+9-c9f6ccba+20210206-2044-SNAPSHOT' MDOC_BATCH_MODE=true ./repro.sh
/tmp/tmp.mglK52yk6v
Using the following mdoc.properties:
js-classpath=/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.3.0/scalajs-library_2.13-1.3.0.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.3/scala-library-2.13.3.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-dom_sjs1_2.13/1.1.0/scalajs-dom_sjs1_2.13-1.1.0.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.1/scala-library-2.13.1.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.0.0/scalajs-library_2.13-1.0.0.jar
js-scalac-options=-Xplugin:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-compiler_2.13.3/1.3.0/scalajs-compiler_2.13.3-1.3.0.jar
js-batch-mode=true
info: Compiling 3 files to /tmp/tmp.9QxaoKSvcu
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Compiled in 4.84s (0 errors)

js-batch-mode=false (default)

❯ MDOC_VERSION_OVERRIDE='2.2.16+9-c9f6ccba+20210206-2044-SNAPSHOT' MDOC_BATCH_MODE=false ./repro.sh
/tmp/tmp.lSDzmvB2cH
Using the following mdoc.properties:
js-classpath=/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.3.0/scalajs-library_2.13-1.3.0.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.3/scala-library-2.13.3.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-dom_sjs1_2.13/1.1.0/scalajs-dom_sjs1_2.13-1.1.0.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.1/scala-library-2.13.1.jar:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.0.0/scalajs-library_2.13-1.0.0.jar
js-scalac-options=-Xplugin:/home/velvetbaldmime/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-compiler_2.13.3/1.3.0/scalajs-compiler_2.13.3-1.3.0.jar
js-batch-mode=false
info: Compiling 3 files to /tmp/tmp.UeNPg3lZB8
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: JSC_UNDEFINED_VARIABLE. variable $c_s_util_Success is undeclared at virtualfile:a.md line 11 : 7
info: Closure: 1 error(s), 0 warning(s)
error: mdoc:js exception
mdoc.internal.markdown.ModifierException: mdoc:js exception
Caused by: org.scalajs.linker.interface.LinkingException: There were errors when applying the Google Closure Compiler
        at org.scalajs.linker.backend.closure.ClosureLinkerBackend.compile(ClosureLinkerBackend.scala:136)
        at org.scalajs.linker.backend.closure.ClosureLinkerBackend.$anonfun$emit$5(ClosureLinkerBackend.scala:105)
        at org.scalajs.logging.Logger.time(Logger.scala:42)
        at org.scalajs.logging.Logger.time$(Logger.scala:40)
        at mdoc.modifiers.JsModifier$$anon$1.time(JsModifier.scala:47)
        at org.scalajs.linker.backend.closure.ClosureLinkerBackend.$anonfun$emit$3(ClosureLinkerBackend.scala:94)
        at scala.Option.map(Option.scala:242)
        at org.scalajs.linker.backend.closure.ClosureLinkerBackend.emit(ClosureLinkerBackend.scala:88)
        at org.scalajs.linker.standard.StandardLinkerImpl.$anonfun$link$2(StandardLinkerImpl.scala:47)
        at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:434)
        at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
        at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
info: Compiled in 3.8s (1 error)

Tested on https://github.com/raquo/Laminar master branch, by using locally published mdoc version:

diff --git a/build.sbt b/build.sbt
index 32ef275..e270850 100644
--- a/build.sbt
+++ b/build.sbt
@@ -43,7 +43,8 @@ lazy val website = project
     mdocJSLibraries := webpack.in(websiteJS, Compile, fullOptJS).value,
     skip in publish := true,
     mdocVariables := Map(
-      "js-mount-node" -> "containerNode"
+      "js-mount-node" -> "containerNode",
+      "js-batch-mode" -> "true"
       //  // Use these as @VERSION@ in mdoc-processed .md files
       //  "LAMINAR_VERSION" -> version.value.replace("-SNAPSHOT", ""), // This can return incorrect version too easily
       //  "SCALA_VERSION" -> scalaVersion.value
diff --git a/project/plugins.sbt b/project/plugins.sbt
index a8522ca..6498675 100644
--- a/project/plugins.sbt
+++ b/project/plugins.sbt
@@ -11,4 +11,4 @@ addSbtPlugin("com.github.gseitz" % "sbt-release" % "1.0.8")

 addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "3.8.1")

-addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.2.9" )
+addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.2.16+9-c9f6ccba+20210206-2044-SNAPSHOT" )

sbt:Laminar> website/docusaurusCreateSite
[info] running mdoc.Main
info: Compiling 9 files to /home/velvetbaldmime/projects/Laminar/website/target/mdoc
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Closure: 0 error(s), 0 warning(s)
info: Compiled in 24s (0 errors)
yarn install v1.22.10
.....

keynmol · 2021-02-06T11:59:56Z

@olafurpg I also reproduced it in tests here: #455

Let me know if you want to combine the two to make sure the fix doesn't get accidentally broken in the future

olafurpg

LGTM 👍

It might be good to document the existence of this option on the website.

@keynmol i really appreciate your contributions to mdoc. I just sent you a collaborator invitation so that you can merge PRs and cut new releases. I may be slow to respond sometimes so don’t hesitate to merge and release without waiting for me.

olafurpg · 2021-02-06T19:41:30Z

mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala

@@ -165,7 +165,9 @@ class JsModifier extends mdoc.PreModifier {
    } else {
      val output = MemOutputFile.apply()

-      val linking = linker.link(virtualIrFiles ++ sjsir, Nil, LinkerOutput.apply(output), sjsLogger)
+      val currentLinker = if (config.reuseLinker) linker else newLinker()


Does the linker have some shutdown/cleanup method that we should call when we initialize a new linker?

I'm not seeing any in the interface, but there's something like ClearableLinker that seems to just manage an internal _linker state and setting it to null when .clear() is called

I'll check with Scala.js folks on whether I should use that, or just set batch mode on the linker which should achieve the same results without this if condition

just sent you a collaborator invitation

Thank you, appreciate it :)

321db8a should be better - clearable linker + batch mode means no manual management of linker lifecycle and the same results

olafurpg

The last commit looks great!

keynmol · 2021-02-06T23:45:34Z

Gonna merge this and test Laminar with the snapshot version before releasing a new tag 👍

Allow controlling linker re-use on Scala.js

c9f6ccb

keynmol force-pushed the linker-reuse-flag branch from 6f02c30 to c9f6ccb Compare February 6, 2021 11:22

keynmol mentioned this pull request Feb 6, 2021

Use fast opt in mdoc to circumvent a bug in Scala.js incremental linker raquo/Laminar#85

Closed

olafurpg approved these changes Feb 6, 2021

View reviewed changes

Use clearable linker and batch mode

321db8a

olafurpg approved these changes Feb 6, 2021

View reviewed changes

keynmol merged commit 977d6c8 into scalameta:master Feb 6, 2021

keynmol deleted the linker-reuse-flag branch February 6, 2021 23:46

keynmol mentioned this pull request Feb 6, 2021

Update mdoc to 2.2.17, use batch mode workaround raquo/Laminar#86

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow controlling linker re-use on Scala.js #456

Allow controlling linker re-use on Scala.js #456

keynmol commented Feb 6, 2021 •

edited

Loading

keynmol commented Feb 6, 2021

olafurpg left a comment

olafurpg Feb 6, 2021

keynmol Feb 6, 2021

keynmol Feb 6, 2021

olafurpg left a comment

keynmol commented Feb 6, 2021

Allow controlling linker re-use on Scala.js #456

Allow controlling linker re-use on Scala.js #456

Conversation

keynmol commented Feb 6, 2021 • edited Loading

keynmol commented Feb 6, 2021

olafurpg left a comment

Choose a reason for hiding this comment

olafurpg Feb 6, 2021

Choose a reason for hiding this comment

keynmol Feb 6, 2021

Choose a reason for hiding this comment

keynmol Feb 6, 2021

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

keynmol commented Feb 6, 2021

keynmol commented Feb 6, 2021 •

edited

Loading