Merge pull request #1127 from japgolly/test_warnings_to_errors

Fixes for compile-time options

Author: japgolly

doc/CONFIG.md

@@ -6,7 +6,6 @@
   * [`.component.names.all`](#componentnamesall)
   * [`.component.names.implicit`](#componentnamesimplicit)
   * [`.config.class` *(Scala 3 only)*](#configclass-scala-3-only)
-  * [`.test.warnings.react`](#testwarningsreact)
 * Runtime Settings *(development-mode only)*
   * [Usage](#runtime-settings-usage)
   * [`Reusability.disableGloballyInDev()`](#reusabilitydisablegloballyindev)
@@ -26,6 +25,16 @@ If this is important to you or your organisation, feel free to reach out and spo
 
 # Compile-Time Settings: Usage
 
+First add this to your sbt:
+
+```scala
+scalacOptions ++= // Required since sbt 1.6.0
+  sys.props.iterator
+    .filter(_._1.contains("japgolly"))
+    .map(x => s"-D${x._1}=${x._2}")
+    .toSeq
+```
+
 Currently, you have to specify compile-time settings to `sbt` directly when you start it.
 
 Examples:
@@ -161,60 +170,6 @@ object CustomConfig extends ScalaJsReactConfig.Defaults {
 }
 ```
 
-
-# `.test.warnings.react`
-
-When using `LegacyReactTestUtils`, this setting can be used to catch React warnings and turn them into exceptions.
-
-### Usage:
-
-```
-sbt -Djapgolly.scalajs.react.test.warnings.react=warn|fatal
-```
-
-| Setting | Outcome |
-| -- | -- |
-| `warn` (default) | Print warnings and move on |
-| `fatal`  | Throw warnings as exceptions |
-
-### Example:
-
-```
-sbt -Djapgolly.scalajs.react.test.warnings.react=fatal
-```
-
-```scala
-package com.example
-
-import japgolly.scalajs.react._
-import japgolly.scalajs.react.test._
-import japgolly.scalajs.react.vdom.html_<^._
-import utest._
-
-object ExampleTest extends TestSuite {
-
-  override def tests = Tests {
-    "example" - {
-      val comp = ScalaFnComponent[Int](i => <.p(<.td(s"i = $i")))
-      LegacyReactTestUtils.withRenderedIntoBody(comp(123)).withParent { m =>
-        val html = m.outerHTML
-        assert(html == "<p><td>i = 123</td></p>")
-      }
-    }
-  }
-}
-```
-
-Running the above test will fail with this error message:
-
-```
-scala.scalajs.js.JavaScriptException: Warning: validateDOMNesting(...): <td> cannot appear as a child of <p>.
-  at td
-  at p
-  at $c_sjs_js_Any$.fromFunction1__F1__sjs_js_Function1
-```
-
-
 # Runtime Settings: Usage
 
 Runtime settings are designed for use in development-mode only (`fastOptJS`).
@@ -241,7 +196,8 @@ object Main {
 
     // Start app
     val container = dom.document.getElementById("root")
-    MyApp().renderIntoDOM(container)
+    val root = ReactDOMClient.createRoot(container)
+    root.render(MyApp())
   }
 }
 ```
@@ -293,7 +249,8 @@ object Main {
 
     // Start app
     val container = dom.document.getElementById("root")
-    MyApp().renderIntoDOM(container)
+    val root = ReactDOMClient.createRoot(container)
+    root.render(MyApp())
   }
 }
 ```

doc/TESTING.md

@@ -290,10 +290,7 @@ for how to write tests for real-world scalajs-react applications.
 
 # Fatal React warnings
 
-The easiest way to make `LegacyReactTestUtils` to turn React warnings into runtime exceptions,
-is via a [config option](./CONFIG.md#testwarningsreact).
-
-Alternatively, you can do any of the following...
+To turn React warnings into runtime exceptions, do any of the following...
 
 - Wrapping a test
 
@@ -304,7 +301,7 @@ Alternatively, you can do any of the following...
   }
   ```
 
-- Installing for all `LegacyReactTestUtils` usage
+- Installing for all `ReactTestUtils` usage
 
   ```scala
   import japgolly.scalajs.react.test.ReactTestUtilsConfig

doc/changelog/3.0.0.md

@@ -35,6 +35,9 @@
   - Wrap router actions in `startTransition`, making the change of pages interruptible.
   - Allow creating custom `Route` instances outside the DSL model. A `Route` just wraps functions to turn a `Path` into a potential page and vice-versa.
 
+- The ability to globally turn React warnings into errors via sbt system properties has been removed.
+  It's test case had an error in it — it never worked as intended.
+
 ### Deprecated
 
 - Anything deprecated by React 18 is now deprecated in Scala

downstream-tests/build.sbt

@@ -20,6 +20,7 @@ def scalacCommonFlags: Seq[String] = Seq(
 )
 
 def scalac2Flags = Seq(
+  "-Wconf:cat=lint-infer-any&msg=kind-polymorphic:s", // https://github.com/scala/bug/issues/13128
   "-Wunused:explicits",                            // Warn if an explicit parameter is unused.
   "-Wunused:implicits",                            // Warn if an implicit parameter is unused.
   "-Wunused:imports",                              // Warn if an import selector is not referenced.
@@ -63,6 +64,11 @@ def commonSettings: Project => Project = _
                               case (2, _) => scalac2Flags
                               case (3, _) => scalac3Flags
                             }.value,
+    scalacOptions ++= // Required since sbt 1.6.0
+      sys.props.iterator
+        .filter(_._1.matches("(downstream_tests|japgolly).*"))
+        .map(x => s"-D${x._1}=${x._2}")
+        .toSeq,
     dependencyOverrides ++= globalDependencyOverrides.value,
   )
 

downstream-tests/js/src/test/scala/downstream/RuntimeTests.scala

@@ -4,10 +4,8 @@ import cats.effect.IO
 import japgolly.microlibs.compiletime.CompileTimeInfo
 import japgolly.microlibs.testutil.TestUtil._
 import japgolly.scalajs.react._
-import japgolly.scalajs.react.vdom.html_<^._
 import japgolly.scalajs.react.test.ReactTestUtils
 import japgolly.scalajs.react.util.JsUtil
-import org.scalajs.dom.console
 import scala.scalajs.js
 import scala.scalajs.LinkingInfo.developmentMode
 import scala.util.Try
@@ -16,10 +14,9 @@ import japgolly.scalajs.react.AsyncTestSuite
 
 object RuntimeTests extends AsyncTestSuite {
 
-  val compNameAuto      = CompileTimeInfo.sysProp("japgolly.scalajs.react.component.names.implicit")
-  val compNameAll       = CompileTimeInfo.sysProp("japgolly.scalajs.react.component.names.all")
-  val configClass       = CompileTimeInfo.sysProp("japgolly.scalajs.react.config.class")
-  val testWarningsReact = CompileTimeInfo.sysProp("japgolly.scalajs.react.test.warnings.react")
+  val compNameAuto = CompileTimeInfo.sysProp("japgolly.scalajs.react.component.names.implicit")
+  val compNameAll  = CompileTimeInfo.sysProp("japgolly.scalajs.react.component.names.all")
+  val configClass  = CompileTimeInfo.sysProp("japgolly.scalajs.react.config.class")
 
   val dsCfg1 = configClass.contains("downstream.DownstreamConfig1")
   val dsCfg2 = configClass.contains("downstream.DownstreamConfig2")
@@ -91,26 +88,5 @@ object RuntimeTests extends AsyncTestSuite {
       } yield ()
     }
 
-    "testWarnings" - {
-
-      "react" - {
-        val c = ScalaFnComponent[Int](i => <.p(<.td(s"i = $i")))
-        ReactTestUtils.withRendered_(c(123))(_ => ()).attemptTry.map { t =>
-          assertEq(t.isFailure, testWarningsReact.contains("react"))
-        }
-      }
-
-      "unlreated" - {
-        val c = ScalaFnComponent[Int](i => <.p(s"i = $i"))
-        ReactTestUtils.withRendered_(c(123)) { _ =>
-          console.info(".")
-          console.log(".")
-          console.warn(".")
-          console.error(".")
-        }.attemptTry.map { t =>
-          assertEq(t.isFailure, false)
-        }
-      }
-    }
   }
 }

downstream-tests/run

@@ -6,7 +6,7 @@ skip2=skip2
 
 # Tests
 all_tests_values=(
-  "-Ddownstream_tests.enableJSCE -Djapgolly.scalajs.react.test.warnings.react=fatal"
+  "-Ddownstream_tests.enableJSCE"
   "-Djapgolly.scalajs.react.config.class=downstream.DownstreamConfig1 -Ddownstream_tests.reusability.dev=disable $skip2"
   "-Djapgolly.scalajs.react.component.names.implicit=blank -Djapgolly.scalajs.react.component.names.all=allow"
   "-Djapgolly.scalajs.react.component.names.implicit=short -Ddownstream_tests.reusability.dev=disable"

library/project/Build.scala

@@ -251,7 +251,7 @@ object ScalaJsReact {
 
   lazy val util = project
     .dependsOn(utilFallbacks % Provided)
-    .configure(commonSettings, publicationSettings, hasNoTests, disableScalaDoc3, prohibitDefaultEffects)
+    .configure(commonSettings, publicationSettings, utestSettings, disableScalaDoc3, prohibitDefaultEffects)
     .settings(
       libraryDependencies += Dep.scalaJsDom.value,
     )

library/testUtil/src/main/scala/japgolly/scalajs/react/test/LegacyReactTestUtils.scala

@@ -4,7 +4,6 @@ import japgolly.scalajs.react._
 import japgolly.scalajs.react.facade.{React => RawReact, ReactDOM => RawReactDOM}
 import japgolly.scalajs.react.hooks.Hooks
 import japgolly.scalajs.react.internal.CoreGeneral._
-import japgolly.scalajs.react.test.ReactTestUtilsConfig.aroundReact
 import japgolly.scalajs.react.util.DefaultEffects.{Async => DA, Sync => DS}
 import japgolly.scalajs.react.util.Effect._
 import japgolly.scalajs.react.util.JsUtil
@@ -106,30 +105,13 @@ object LegacyReactTestUtils extends LegacyReactTestUtils {
 
     def _withRenderedAsync[F[_], M, A](u: Unmounted[M], parent: Element, f: (M, Element) => F[A])
                                       (implicit F: Async[F]): F[A] =
-      aroundReactAsync {
+      ReactTestUtilsConfig.aroundReact.async {
         F.flatMap(F.delay(act(RawReactDOM.render(u.raw, parent)))) { c =>
           val m = u.mountRawOrNull(c)
           F.finallyRun(f(m, parent), F.delay(act(unmountRawComponent(c))))
         }
       }
 
-    def aroundReactAsync[F[_], A](body: F[A])(implicit F: Async[F]): F[A] = {
-      val start = F.delay {
-        val stop = aroundReact.start()
-        F.delay(stop())
-      }
-      F.flatMap(start) { stop =>
-        F.finallyRun(body, stop)
-      }
-    }
-
-    def aroundReactFuture[A](body: => Future[A])(implicit ec: ExecutionContext): Future[A] = {
-      val stop = aroundReact.start()
-      val f    = body
-      f.onComplete { _ => stop() }
-      f
-    }
-
   } // Internals
 }
 
@@ -279,7 +261,7 @@ trait LegacyReactTestUtils extends japgolly.scalajs.react.test.internal.ReactTes
     new WithRenderedDsl[M, Element] {
       override def apply[A](f: (M, Element) => A): A =
         withNewBodyElement { parent =>
-          aroundReact.sync {
+          ReactTestUtilsConfig.aroundReact.sync {
             val c = act(RawReactDOM.render(u.raw, parent))
             try
               f(u.mountRawOrNull(c), parent)
@@ -308,7 +290,7 @@ trait LegacyReactTestUtils extends japgolly.scalajs.react.test.internal.ReactTes
     */
   def withRenderedIntoBodyFuture[M, A](u: Unmounted[M])(f: M => Future[A])(implicit ec: ExecutionContext): Future[A] =
     withNewBodyElementFuture { parent =>
-      aroundReactFuture {
+      ReactTestUtilsConfig.aroundReact.future {
         val c = act(RawReactDOM.render(u.raw, parent))
         val m = u.mountRawOrNull(c)
         attemptFuture(f(m)).andThen { case _ => act(unmountRawComponent(c)) }
@@ -357,7 +339,7 @@ trait LegacyReactTestUtils extends japgolly.scalajs.react.test.internal.ReactTes
   def withRenderedIntoDocument[M](u: Unmounted[M]): WithRenderedDsl[M, Element] =
     new WithRenderedDsl[M, Element] {
       override def apply[A](f: (M, Element) => A): A =
-        aroundReact.sync {
+        ReactTestUtilsConfig.aroundReact.sync {
           val c = act(raw.renderIntoDocument(u.raw))
           try {
             val p = parentElement(c)
@@ -377,7 +359,7 @@ trait LegacyReactTestUtils extends japgolly.scalajs.react.test.internal.ReactTes
     * and asynchronously waits for the Future to complete before unmounting.
     */
   def withRenderedIntoDocumentFuture[M, A](u: Unmounted[M])(f: M => Future[A])(implicit ec: ExecutionContext): Future[A] =
-    aroundReactFuture {
+    ReactTestUtilsConfig.aroundReact.future {
       val c = act(raw.renderIntoDocument(u.raw))
       val m = u.mountRawOrNull(c)
       attemptFuture(f(m)).andThen { case _ => act(unmountRawComponent(c)) }

library/testUtil/src/main/scala/japgolly/scalajs/react/test/ReactTestUtils.scala

@@ -120,11 +120,18 @@ trait ReactTestUtils extends japgolly.scalajs.react.test.internal.ReactTestUtilE
     Resource.make(Effect[F].delay(newElement()), e => Effect[F].delay(removeElement(e)))
 
   val withReactRootSync: WithDsl[TestReactRoot, ImplicitUnit] =
-    withElementSync.mapResource(TestReactRoot(_))(_.unmountSync())
-
-  def withReactRoot[F[_]: Async]: Resource[F, TestReactRoot] =
-    withElement[F].flatMap{ e =>
-      Resource.make_[F, TestReactRoot](TestReactRoot(e), _.unmount[F]())
+    withElementSync
+      .mapResource(e => {
+        val stop = ReactTestUtilsConfig.aroundReact.start()
+        (e, stop)
+      })(_._2())
+      .mapResource(x => TestReactRoot(x._1))(_.unmountSync())
+
+  def withReactRoot[F[_]](implicit F: Async[F]): Resource[F, TestReactRoot] =
+    withElement[F].flatMap { e =>
+      Resource.make_[F, () => Unit](ReactTestUtilsConfig.aroundReact.start(), stop => F.delay(stop())).flatMap { _ =>
+        Resource.make_[F, TestReactRoot](TestReactRoot(e), _.unmount[F]())
+      }
     }
 
   def withRenderedSync[A](unmounted: A): WithDsl[TestDomWithRoot, Renderable[A]] =

library/testUtil/src/main/scala/japgolly/scalajs/react/test/ReactTestUtilsConfig.scala

@@ -1,6 +1,9 @@
 package japgolly.scalajs.react.test
 
 import japgolly.scalajs.react.test.internal._
+import japgolly.scalajs.react.util.Effect.Async
+import scala.concurrent.ExecutionContext
+import scala.concurrent.Future
 
 object ReactTestUtilsConfig extends ReactTestUtilsConfigTypes {
 
@@ -20,5 +23,22 @@ object ReactTestUtilsConfig extends ReactTestUtilsConfigTypes {
 
     override def start(): () => Unit =
       value.start()
+
+    def async[F[_], A](body: F[A])(implicit F: Async[F]): F[A] = {
+      val start = F.delay {
+        val stop = ReactTestUtilsConfig.aroundReact.start()
+        F.delay(stop())
+      }
+      F.flatMap(start) { stop =>
+        F.finallyRun(body, stop)
+      }
+    }
+
+    def future[A](body: => Future[A])(implicit ec: ExecutionContext): Future[A] = {
+      val stop = ReactTestUtilsConfig.aroundReact.start()
+      val f    = body
+      f.onComplete { _ => stop() }
+      f
+    }
   }
 }