Merge pull request #498 from japgolly/unmounted-dom

Fix getDOMNode on unmounted components

Author: japgolly

core/src/main/scala/japgolly/scalajs/react/ComponentDom.scala

@@ -69,6 +69,13 @@ object ComponentDom {
       case null | ()      => Unmounted
     }
 
+  def findDOMNode(a: dom.Element | Raw.React.ComponentUntyped): ComponentDom = {
+    val b: Raw.ReactDOM.DomNode | Null =
+      try Raw.ReactDOM.findDOMNode(a)
+      catch {case t: Throwable => null}
+    apply(b)
+  }
+
   case object Unmounted extends ComponentDom {
     override def mounted = None
   }

core/src/main/scala/japgolly/scalajs/react/ReactDOM.scala

@@ -10,7 +10,7 @@ object ReactDOM {
 
   /** For mounted components, use .getDOMNode */
   def findDOMNode(componentOrElement: dom.Element | Raw.React.ComponentUntyped): Option[ComponentDom.Mounted] =
-    ComponentDom(raw.findDOMNode(componentOrElement)).mounted
+    ComponentDom.findDOMNode(componentOrElement).mounted
 
   def unmountComponentAtNode(container: dom.Node): Boolean =
     raw.unmountComponentAtNode(container)

core/src/main/scala/japgolly/scalajs/react/component/Js.scala

@@ -133,7 +133,7 @@ object Js extends JsBaseComponentTemplate[RAW.React.ComponentClassP] {
       override def props         = raw.props
       override def propsChildren = PropsChildren.fromRawProps(raw.props)
       override def state         = raw.state
-      override def getDOMNode    = ComponentDom(RAW.ReactDOM.findDOMNode(raw))
+      override def getDOMNode    = ComponentDom.findDOMNode(raw)
 
       override def setState(state: S, callback: Callback): Unit =
         raw.setState(state, callback.toJsFn)

core/src/main/scala/japgolly/scalajs/react/raw/ReactDOM.scala

@@ -25,11 +25,12 @@ trait ReactDOM extends js.Object {
   final def unmountComponentAtNode(container: dom.Node): Boolean = js.native
 
   // ==========================================================================
-  // NOTE: Ensure that DomNode is kept up-to-date with this type
+  // NOTE: Ensure that ComponentDom is kept up-to-date with this type
   //
   final type DomNode = dom.Element | dom.Text
   // ==========================================================================
 
+  @throws[js.JavaScriptException]("if arg isn't a React component or its unmounted")
   final def findDOMNode(componentOrElement: dom.Element | React.ComponentUntyped): DomNode | Null = js.native
 
   final def createPortal(child: React.Node, container: Container): React.Node = js.native

doc/USAGE.md

@@ -337,9 +337,9 @@ React Extensions
   rather than just getting back a VDOM reference, the return type is an ADT like this:
 
   ```
-                  ComponentDom
-                        ↑
-               ComponentDom.Mounted
+                                   ComponentDom
+                                    ↑        ↑
+                 ComponentDom.Mounted        ComponentDom.Unmounted
                   ↑            ↑
   ComponentDom.Element      ComponentDom.Text
   ```

doc/changelog/1.3.0.md

@@ -7,9 +7,9 @@
   There is now a proper ADT:
 
   ```
-                  ComponentDom
-                        ↑
-               ComponentDom.Mounted
+                                 ComponentDom
+                                  ↑        ↑
+               ComponentDom.Mounted        ComponentDom.Unmounted
                   ↑            ↑
   ComponentDom.Element      ComponentDom.Text
   ```

doc/changelog/1.3.1.md

@@ -0,0 +1,7 @@
+## 1.3.1
+
+* In the previous release `.getDOMNode` got a new data type capable of handling
+  when the component is unmounted.
+  Silly me forgot to actually test and catch the case when it's unmounted.
+  This is fixed in this release.
+

test/src/test/scala/japgolly/scalajs/react/core/ScalaComponentTest.scala

@@ -149,7 +149,7 @@ object ScalaComponentPTest extends TestSuite {
         .componentDidCatch($ => $.setState(Some($.error.message.replaceFirst("'.+' *", ""))))
         .build
 
-      ReactTestUtils.withNewBodyElement { mountNode =>
+      val staleDomNodeCallback = ReactTestUtils.withNewBodyElement { mountNode =>
         assertMountCount(0)
 
         var mounted = Comp(Props(1, 2, 3)).renderIntoDOM(mountNode)
@@ -169,11 +169,15 @@ object ScalaComponentPTest extends TestSuite {
         mounted = Comp(null).renderIntoDOM(mountNode)
         assertOuterHTML(mounted.getDOMNode.asMounted().asElement(), "<div>Error: Cannot read property of null</div>")
         assertEq("willUnmountCount", willUnmountCount, 1)
+
+        mounted.withEffectsPure.getDOMNode
       }
 
       assertMountCount(1)
       assertEq("willUnmountCount", willUnmountCount, 1)
-      assertEq("recievedPropDeltas", recievedPropDeltas, Vector(Props(0, 0, 5), Props(0, 3, 0)))
+      assertEq("receivedPropDeltas", recievedPropDeltas, Vector(Props(0, 0, 5), Props(0, 3, 0)))
+
+      assert(staleDomNodeCallback.runNow().mounted.isEmpty)
     }
 
     'lifecycle2 - {