From 622157d721538aecd54cdc74e4c8ba2494086ace Mon Sep 17 00:00:00 2001
From: Shadaj Laddad <shadaj@users.noreply.github.com>
Date: Wed, 2 May 2018 22:32:58 -0700
Subject: [PATCH] Fix bug with shouldComponentUpdate registration (#135)

---
 CHANGELOG.md                                  |  1 +
 .../scala/slinky/core/DefinitionBase.scala    |  2 +-
 .../scala/slinky/core/ComponentTest.scala     | 41 +++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 28394cb5..dda014e9 100644
--- 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
index 50915925..12073194 100644
--- 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
index 844750c4..5783762d 100644
--- 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()