Fix bug with shouldComponentUpdate registration (#135)

shadaj · May 3, 2018 · 622157d · 622157d
1 parent 422f288
commit 622157d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
 ## vNEXT
++ Fix bug with `shouldComponentUpdate` not being registered correctly on the component [PR #135](https://github.com/shadaj/slinky/pull/135)
 
 ## v0.4.1
 + Fix exception when hot-reloading Slinky components [PR #134](https://github.com/shadaj/slinky/pull/134)

diff --git a/core/src/main/scala/slinky/core/DefinitionBase.scala b/core/src/main/scala/slinky/core/DefinitionBase.scala
@@ -156,7 +156,7 @@ abstract class DefinitionBase[Props, State, Snapshot](jsProps: js.Object) extend
     }
 
     if (this.asInstanceOf[js.Dynamic].shouldComponentUpdate != defaultBase.shouldComponentUpdate) {
-      val orig = this.asInstanceOf[js.Dynamic].shouldComponentUpdate.bind(this).asInstanceOf[js.Function2[Props, State, Unit]]
+      val orig = this.asInstanceOf[js.Dynamic].shouldComponentUpdate.bind(this).asInstanceOf[js.Function2[Props, State, Boolean]]
       this.asInstanceOf[js.Dynamic].shouldComponentUpdate = (nextProps: js.Object, nextState: js.Object) => {
         orig(
           readPropsValue(nextProps),

diff --git a/tests/src/test/scala/slinky/core/ComponentTest.scala b/tests/src/test/scala/slinky/core/ComponentTest.scala
@@ -52,6 +52,28 @@ object TestComponentForSetStateCallback extends ComponentWrapper {
   }
 }
 
+object TestComponentForShouldComponentUpdate extends ComponentWrapper {
+  type Props = () => Unit
+  type State = Int
+
+  class Def(jsProps: js.Object) extends Definition(jsProps) {
+    override def initialState: Int = 0
+
+    override def shouldComponentUpdate(nextProps: Props, nextState: State) = {
+      nextState == 123
+    }
+
+    override def componentDidUpdate(prevProps: Props, prevState: State) = {
+      println("component did update?")
+      prevProps.apply()
+    }
+
+    override def render(): ReactElement = {
+      null
+    }
+  }
+}
+
 object TestComponentForSnapshot extends ComponentWrapper {
   type Props = Int => Unit
   type State = Int
@@ -194,6 +216,25 @@ class ComponentTest extends AsyncFunSuite {
     assert(!js.isUndefined(element.asInstanceOf[js.Dynamic].ref))
   }
 
+  test("shouldComponentUpdate controls when component is updated") {
+    var called = false
+    var ref: TestComponentForShouldComponentUpdate.Def = null
+
+    ReactDOM.render(
+      TestComponentForShouldComponentUpdate(() => {
+        called = true
+      }).withRef(r => ref = r),
+      dom.document.createElement("div")
+    )
+
+    ref.setState(123)
+    assert(called)
+
+    called = false
+    ref.setState(1)
+    assert(!called)
+  }
+
   test("Force updating a component by its ref works") {
     val promise: Promise[Assertion] = Promise()