Merge pull request #1028 from japgolly/issue/1027

Fix infinite loop in functional components with reuse + `Reusability.never`

Author: japgolly

coreGeneric/src/main/scala/japgolly/scalajs/react/hooks/CustomHook.scala

@@ -169,25 +169,38 @@ object CustomHook {
 
   // ===================================================================================================================
 
+  private final case class ReusableDepState[+A](value: A, rev: Int) {
+    var skipTest = true
+    val asTuple = (value, rev)
+  }
+
   def reusableDeps[D](implicit r: Reusability[D]): CustomHook[() => D, (D, Int)] =
     CustomHook[() => D]
-      .useState(Option.empty[(D, Int)])
+      .useState[Option[ReusableDepState[D]]](None)
       .buildReturning { (getDeps, depsState) =>
         val next = getDeps()
-        depsState.value match {
-
-          // Previous result can be reused
-          case Some(prevState @ (prev, _)) if r.test(prev, next) =>
-            prevState
-
-          // Not reusable
-          case o =>
-            val nextRev     = o.fold(1)(_._2 + 1)
-            val nextState   = (next, nextRev)
-            val updateState = depsState.setState(Some(nextState))
-            DefaultEffects.Sync.runSync(updateState)
-            nextState
-        }
+        val nextDepState =
+          depsState.value match {
+
+            // React is calling us again after the setState below
+            // Don't re-test reusability, we could end up in an infinite-loop (see #1027)
+            case Some(s) if s.skipTest =>
+              s.skipTest = false
+              s
+
+            // Previous result can be reused
+            case Some(prevState) if r.test(prevState.value, next) =>
+              prevState
+
+            // Not reusable
+            case o =>
+              val nextRev     = o.fold(1)(_.rev + 1)
+              val nextState   = ReusableDepState(next, nextRev)
+              val updateState = depsState.setState(Some(nextState)) // this causes a hook re-eval
+              DefaultEffects.Sync.runSync(updateState)
+              nextState
+          }
+        nextDepState.asTuple
       }
 
   def reusableByDeps[D, A](create: (D, Int) => A)(implicit r: Reusability[D]): CustomHook[() => D, Reusable[A]] =

doc/changelog/2.0.1.md

@@ -0,0 +1,5 @@
+## 2.0.1
+
+### Bug fixes
+
+* Fix infinite loop in functional components with reuse + `Reusability.never` ([#1027](https://github.com/japgolly/scalajs-react/issues/1027))

tests/src/test/scala/japgolly/scalajs/react/core/HooksTest.scala

@@ -1224,6 +1224,35 @@ object HooksTest extends TestSuite {
     }
   }
 
+  // See https://github.com/japgolly/scalajs-react/issues/1027
+  private def testRenderWithReuseNever(): Unit = {
+    implicit val reusability: Reusability[PI] = Reusability.never
+    var renders = 0
+    val comp = ScalaFnComponent.withHooks[PI]
+      .renderWithReuse { p =>
+        renders += 1
+        <.div(s"P=$p, R=$renders")
+      }
+
+    val wrapper = ScalaComponent.builder[PI]
+      .initialStateFromProps(identity)
+      .renderS { ($, s) =>
+        <.div(
+          comp(s),
+          <.button(^.onClick --> $.modState(_ + 0)),
+          <.button(^.onClick --> $.modState(_ + 1)),
+        )
+      }
+      .build
+
+    withRenderedIntoBody(wrapper(PI(3))) { (_, root) =>
+      val t = new Tester(root)
+      t.assertText("P=PI(3), R=1")
+      t.clickButton(2); t.assertText("P=PI(4), R=2")
+      t.clickButton(1); t.assertText("P=PI(4), R=3")
+    }
+  }
+
   // ===================================================================================================================
 
   override def tests = Tests {
@@ -1289,5 +1318,6 @@ object HooksTest extends TestSuite {
     "useStateSnapshot" - testUseStateSnapshot()
     "useStateSnapshotWithReuse" - testUseStateSnapshotWithReuse()
     "renderWithReuse" - testRenderWithReuse()
+    "renderWithReuseNever" - testRenderWithReuseNever()
   }
 }

tests/src/test/scala/japgolly/scalajs/react/core/ScalaFnComponentTest.scala

@@ -4,6 +4,7 @@ import japgolly.scalajs.react._
 import japgolly.scalajs.react.test.ReactTestUtils
 import japgolly.scalajs.react.test.TestUtil._
 import japgolly.scalajs.react.vdom.html_<^._
+import sourcecode.Line
 import utest._
 
 object ScalaFnComponentTest extends TestSuite {
@@ -54,5 +55,50 @@ object ScalaFnComponentTest extends TestSuite {
         assert(rendered == 3)
       }
     }
+
+    "reuse" - {
+      var renders = 0
+      val F = ScalaFnComponent.withReuse[Int] { p =>
+        renders += 1
+        <.span(p)
+      }
+      val C = ScalaComponent.builder[Int].render_P(F(_)).build
+      ReactTestUtils.withRenderedIntoBody(C(7)) { (m, p) =>
+        def test(expectedRenders: Int, expectedHtml: Int)(implicit q: Line): Unit = {
+          val a = (renders, p.innerHTML.trim)
+          val e = (expectedRenders, s"<span>$expectedHtml</span>")
+          assertEq(a, e)
+        }
+        test(1, 7)
+        ReactTestUtils.replaceProps(C, m)(7)
+        test(1, 7)
+        ReactTestUtils.replaceProps(C, m)(6)
+        test(2, 6)
+      }
+    }
+
+    // See https://github.com/japgolly/scalajs-react/issues/1027
+    "reuseNever" - {
+      implicit def reusability: Reusability[Int] = Reusability.never
+      var renders = 0
+      val F = ScalaFnComponent.withReuse[Int] { p =>
+        renders += 1
+        <.span(p)
+      }
+      val C = ScalaComponent.builder[Int].render_P(F(_)).build
+      ReactTestUtils.withRenderedIntoBody(C(7)) { (m, p) =>
+        def test(expectedRenders: Int, expectedHtml: Int)(implicit q: Line): Unit = {
+          val a = (renders, p.innerHTML.trim)
+          val e = (expectedRenders, s"<span>$expectedHtml</span>")
+          assertEq(a, e)
+        }
+        test(1, 7)
+        ReactTestUtils.replaceProps(C, m)(7)
+        test(2, 7)
+        ReactTestUtils.replaceProps(C, m)(6)
+        test(3, 6)
+      }
+    }
+
   }
 }