diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala b/compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala index c3bfdb2edf..f5c95e0aae 100644 --- a/compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala +++ b/compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala @@ -135,6 +135,11 @@ trait JSDefinitions { lazy val EnableReflectiveInstantiationAnnotation = getRequiredClass("scala.scalajs.reflect.annotation.EnableReflectiveInstantiation") + lazy val ExecutionContextModule = getRequiredModule("scala.concurrent.ExecutionContext") + lazy val ExecutionContext_global = getMemberMethod(ExecutionContextModule, newTermName("global")) + + lazy val ExecutionContextImplicitsModule = getRequiredModule("scala.concurrent.ExecutionContext.Implicits") + lazy val ExecutionContextImplicits_global = getMemberMethod(ExecutionContextImplicitsModule, newTermName("global")) } // scalastyle:on line.size.limit diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala b/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala index 1889e43185..4d0faa7448 100644 --- a/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala +++ b/compiler/src/main/scala/org/scalajs/nscplugin/PrepJSInterop.scala @@ -377,6 +377,32 @@ abstract class PrepJSInterop[G <: Global with Singleton](val global: G) |program is unlikely to function properly.""".stripMargin) super.transform(tree) + case tree if tree.symbol == ExecutionContext_global || + tree.symbol == ExecutionContextImplicits_global => + if (scalaJSOpts.warnGlobalExecutionContext) { + global.runReporting.warning(tree.pos, + """The global execution context in Scala.js is based on JS Promises (microtasks). + |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably. + | + |Unfortunately, there is no way with ECMAScript only to implement a performant + |macrotask execution context (and hence Scala.js core does not contain one). + | + |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor + |Please refer to the README.md of that project for more details regarding + |microtask vs. macrotask execution contexts. + | + |If you do not care about macrotask fairness, you can silence this warning by: + |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Using scala.scalajs.concurrent.JSExecutionContext.queue + | (the implementation of ExecutionContext.global in Scala.js) directly. + |- Using scala.scalajs.concurrent.QueueExecutionContext.timeouts(): + | A setTimeout based ExecutionContext: Correct, but slow (due to clamping). + """.stripMargin, + WarningCategory.Other, tree.symbol) + } + super.transform(tree) + // Validate js.constructorOf[T] case TypeApply(ctorOfTree, List(tpeArg)) if ctorOfTree.symbol == JSPackage_constructorOf => diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSOptions.scala b/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSOptions.scala index 0edf02fe11..50cc0bf1c8 100644 --- a/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSOptions.scala +++ b/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSOptions.scala @@ -38,6 +38,11 @@ trait ScalaJSOptions { * should they be mapped to)? */ def sourceURIMaps: List[URIMap] + /** Whether to warn if the global execution context is used. + * + * See the warning itself or #4129 for context. + */ + def warnGlobalExecutionContext: Boolean } object ScalaJSOptions { diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSPlugin.scala b/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSPlugin.scala index 82d9de61a0..5e7ca6faba 100644 --- a/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSPlugin.scala +++ b/compiler/src/main/scala/org/scalajs/nscplugin/ScalaJSPlugin.scala @@ -62,6 +62,7 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin { else relSourceMap.toList.map(URIMap(_, absSourceMap)) } + var warnGlobalExecutionContext: Boolean = true var _sourceURIMaps: List[URIMap] = Nil var relSourceMap: Option[URI] = None var absSourceMap: Option[URI] = None @@ -121,6 +122,8 @@ class ScalaJSPlugin(val global: Global) extends NscPlugin { fixClassOf = true } else if (option == "genStaticForwardersForNonTopLevelObjects") { genStaticForwardersForNonTopLevelObjects = true + } else if (option == "nowarnGlobalExecutionContext") { + warnGlobalExecutionContext = false } else if (option.startsWith("mapSourceURI:")) { val uris = option.stripPrefix("mapSourceURI:").split("->") diff --git a/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextNoWarnTest.scala b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextNoWarnTest.scala new file mode 100644 index 0000000000..94decfd65a --- /dev/null +++ b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextNoWarnTest.scala @@ -0,0 +1,47 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.nscplugin.test + +import org.scalajs.nscplugin.test.util._ +import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn + +import org.junit.Assume._ +import org.junit.Test + +class GlobalExecutionContextNoWarnTest extends DirectTest with TestHelpers { + + override def extraArgs: List[String] = + super.extraArgs ::: List("-P:scalajs:nowarnGlobalExecutionContext") + + @Test + def noWarnOnUsage: Unit = { + """ + import scala.concurrent.ExecutionContext.global + + object Enclosing { + global + } + """.hasNoWarns() + } + + @Test + def noWarnOnImplicitUsage: Unit = { + """ + import scala.concurrent.ExecutionContext.Implicits.global + + object Enclosing { + scala.concurrent.Future { } + } + """.hasNoWarns() + } +} diff --git a/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala new file mode 100644 index 0000000000..d2e78305d4 --- /dev/null +++ b/compiler/src/test/scala/org/scalajs/nscplugin/test/GlobalExecutionContextWarnTest.scala @@ -0,0 +1,118 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.nscplugin.test + +import org.scalajs.nscplugin.test.util._ +import org.scalajs.nscplugin.test.util.VersionDependentUtils.scalaSupportsNoWarn + +import org.junit.Assume._ +import org.junit.Test + +class GlobalExecutionContextWarnTest extends DirectTest with TestHelpers { + + @Test + def warnOnUsage: Unit = { + """ + import scala.concurrent.ExecutionContext.global + + object Enclosing { + global + } + """ hasWarns + """ + |newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks). + |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably. + | + |Unfortunately, there is no way with ECMAScript only to implement a performant + |macrotask execution context (and hence Scala.js core does not contain one). + | + |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor + |Please refer to the README.md of that project for more details regarding + |microtask vs. macrotask execution contexts. + | + |If you do not care about macrotask fairness, you can silence this warning by: + |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Using scala.scalajs.concurrent.JSExecutionContext.queue + | (the implementation of ExecutionContext.global in Scala.js) directly. + |- Using scala.scalajs.concurrent.QueueExecutionContext.timeouts(): + | A setTimeout based ExecutionContext: Correct, but slow (due to clamping). + | + | global + | ^ + """ + } + + @Test + def warnOnImplicitUsage: Unit = { + """ + import scala.concurrent.ExecutionContext.Implicits.global + + object Enclosing { + scala.concurrent.Future { } + } + """ hasWarns + """ + |newSource1.scala:5: warning: The global execution context in Scala.js is based on JS Promises (microtasks). + |Using it may prevent macrotasks (I/O, timers, UI rendering) from running reliably. + | + |Unfortunately, there is no way with ECMAScript only to implement a performant + |macrotask execution context (and hence Scala.js core does not contain one). + | + |We recommend you use: https://github.com/scala-js/scala-js-macrotask-executor + |Please refer to the README.md of that project for more details regarding + |microtask vs. macrotask execution contexts. + | + |If you do not care about macrotask fairness, you can silence this warning by: + |- Adding @nowarn("cat=other") (Scala >= 2.13.x only) + |- Setting the -P:scalajs:nowarnGlobalExecutionContext compiler option + |- Using scala.scalajs.concurrent.JSExecutionContext.queue + | (the implementation of ExecutionContext.global in Scala.js) directly. + |- Using scala.scalajs.concurrent.QueueExecutionContext.timeouts(): + | A setTimeout based ExecutionContext: Correct, but slow (due to clamping). + | + | scala.concurrent.Future { } + | ^ + """ + } + + @Test + def noWarnIfSelectivelyDisabled: Unit = { + assumeTrue(scalaSupportsNoWarn) + + """ + import scala.annotation.nowarn + import scala.concurrent.ExecutionContext.global + + object Enclosing { + global: @nowarn("cat=other") + } + """.hasNoWarns() + } + + @Test + def noWarnQueue: Unit = { + /* Test that JSExecutionContext.queue does not warn for good measure. + * We explicitly say it doesn't so we want to notice if it does. + */ + + """ + import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue + + object Enclosing { + scala.concurrent.Future { } + } + """.hasNoWarns() + } + +} diff --git a/junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala b/junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala index 58eb712f36..6fe756b96a 100644 --- a/junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala +++ b/junit-async/js/src/main/scala/org/scalajs/junit/async/package.scala @@ -13,7 +13,13 @@ package org.scalajs.junit import scala.concurrent.Future -import scala.concurrent.ExecutionContext.Implicits.global + +/* Use the queue execution context (based on JS promises) explicitly: + * We do not have anything better at our disposal and it is accceptable in + * terms of fariness: All we use it for is to map over a completed Future once. + */ +import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue + import scala.util.{Try, Success} package object async { diff --git a/junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala b/junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala index 6d15d71858..a2ebe02212 100644 --- a/junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala +++ b/junit-runtime/src/main/scala/org/scalajs/junit/JUnitTask.scala @@ -13,7 +13,14 @@ package org.scalajs.junit import scala.concurrent.Future -import scala.concurrent.ExecutionContext.Implicits.global + +/* Use the queue execution context (based on JS promises) explicitly: + * We do not have anything better at our disposal and it is accceptable in + * terms of fariness: We only use it for test dispatching and orchestation. + * The real async work is done in Bootstrapper#invokeTest which does not take + * an (implicit) ExecutionContext parameter. + */ +import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue import scala.util.{Try, Success, Failure} diff --git a/project/Build.scala b/project/Build.scala index b4b6584b5b..8913d7ad2c 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1007,6 +1007,8 @@ object Build { ).zippedSettings("library")( commonLinkerSettings _ ).settings( + Test / scalacOptions ++= scalaJSCompilerOption("nowarnGlobalExecutionContext"), + if (isGeneratingForIDE) { unmanagedSourceDirectories in Compile += baseDirectory.value.getParentFile.getParentFile / "js/src/main/scala-ide-stubs" @@ -1576,6 +1578,7 @@ object Build { fatalWarningsSettings, name := "Scala.js test bridge", delambdafySetting, + Test / scalacOptions ++= scalaJSCompilerOption("nowarnGlobalExecutionContext"), /* By design, the test-bridge has a completely private API (it is * only loaded through a privately-known top-level export), so it * does not have `previousArtifactSetting` nor @@ -1973,6 +1976,7 @@ object Build { }, Test / scalacOptions ++= scalaJSCompilerOption("genStaticForwardersForNonTopLevelObjects"), + Test / scalacOptions ++= scalaJSCompilerOption("nowarnGlobalExecutionContext"), scalaJSLinkerConfig ~= { _.withSemantics(TestSuiteLinkerOptions.semantics _) }, scalaJSModuleInitializers in Test ++= TestSuiteLinkerOptions.moduleInitializers, diff --git a/test-bridge/src/main/scala/org/scalajs/testing/bridge/HTMLRunner.scala b/test-bridge/src/main/scala/org/scalajs/testing/bridge/HTMLRunner.scala index a997ba44a9..5213295a15 100644 --- a/test-bridge/src/main/scala/org/scalajs/testing/bridge/HTMLRunner.scala +++ b/test-bridge/src/main/scala/org/scalajs/testing/bridge/HTMLRunner.scala @@ -13,6 +13,13 @@ package org.scalajs.testing.bridge import scala.scalajs.js + +/* Use the queue (Promise) execution context by default to avoid slowdown by clamping + * + * To avoid blocking the UI thread, we use QueueExecutionContext.timeout in + * specific spots in the code. See #4129 for context. + */ +import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue import scala.scalajs.concurrent.QueueExecutionContext import scala.scalajs.js.annotation._ import js.URIUtils.{decodeURIComponent, encodeURIComponent} @@ -20,7 +27,6 @@ import js.URIUtils.{decodeURIComponent, encodeURIComponent} import scala.collection.mutable import scala.concurrent.{Future, Promise} -import scala.concurrent.ExecutionContext.Implicits.global import scala.util.Try diff --git a/test-bridge/src/main/scala/org/scalajs/testing/bridge/JSRPC.scala b/test-bridge/src/main/scala/org/scalajs/testing/bridge/JSRPC.scala index f73473fd70..6dd8619f6e 100644 --- a/test-bridge/src/main/scala/org/scalajs/testing/bridge/JSRPC.scala +++ b/test-bridge/src/main/scala/org/scalajs/testing/bridge/JSRPC.scala @@ -15,8 +15,15 @@ package org.scalajs.testing.bridge import scala.scalajs.js import scala.scalajs.js.annotation._ +/* Use the queue execution context (based on JS promises) explicitly: + * We do not have anything better at our disposal and it is accceptable in + * terms of fariness: JSRPC only handles in-between test communcation, so any + * future chain will "yield" to I/O (waiting for a message) or an RPC handler in + * a finite number of steps. + */ +import scala.scalajs.concurrent.JSExecutionContext.Implicits.queue + import scala.concurrent.duration._ -import scala.concurrent.ExecutionContext.Implicits.global import org.scalajs.testing.common.RPCCore