New: Children rendering improvements. Fixes #120. Implements Renderab…

…le from #96.

* Resilience against externally removed child elements
* Support for moving child elements from one inserter to another
* onMountInsert gracefully handling the above
* Text nodes are now updated, not re-created
* New Renderable typeclass to expand text-to-node and child.text capabilities
* Laminar no longer clears the DOM of `{child,children,child.*} <-- stream` when the element is unmounted and re-mounted, it now retains the previously added elements in the DOM

project/Versions.scala

@@ -20,5 +20,5 @@ object Versions {
 
   val JsDom = "16.4.0"
 
-  val ScalaDomTestUtils = "0.16.0-RC4"
+  val ScalaDomTestUtils = "0.16.0-SNAPSHOT"
 }

src/main/scala/com/raquo/laminar/DomApi.scala

@@ -419,4 +419,24 @@ object DomApi {
     }
   }
 
+  def debugNodeOuterHtml(node: dom.Node): String = {
+    node match {
+      case el: dom.Element => el.outerHTML
+      case el: dom.Text => s"Text(${el.textContent})"
+      case el: dom.Comment => s"Comment(${el.textContent})"
+      case null => "<null>"
+      case other => s"OtherNode(${other.toString})"
+    }
+  }
+
+  def debugNodeInnerHtml(node: dom.Node): String = {
+    node match {
+      case el: dom.Element => el.innerHTML
+      case el: dom.Text => s"Text(${el.textContent})"
+      case el: dom.Comment => s"Comment(${el.textContent})"
+      case null => "<null>"
+      case other => s"OtherNode(${other.toString})"
+    }
+  }
+
 }

src/main/scala/com/raquo/laminar/Implicits.scala

@@ -7,7 +7,7 @@ import com.raquo.laminar.Implicits.RichSource
 import com.raquo.laminar.api.Laminar.StyleEncoder
 import com.raquo.laminar.keys.CompositeKey.CompositeValueMappers
 import com.raquo.laminar.keys.{DerivedStyleProp, EventProcessor, EventProp}
-import com.raquo.laminar.modifiers.{Binder, ChildInserter, ChildrenInserter, Inserter, Modifier, Setter}
+import com.raquo.laminar.modifiers.{Binder, ChildInserter, ChildTextInserter, ChildrenInserter, Inserter, Modifier, Renderable, Setter}
 import com.raquo.laminar.nodes.{ChildNode, ReactiveElement, TextNode}
 import org.scalajs.dom
 
@@ -27,13 +27,10 @@ trait Implicits extends Implicits.LowPriorityImplicits with CompositeValueMapper
     EventProcessor.empty(eventProp)
   }
 
-  @inline implicit def textToNode(text: String): TextNode = new TextNode(text)
-
-  @inline implicit def boolToNode(bool: Boolean): TextNode = new TextNode(bool.toString)
-
-  @inline implicit def intToNode(int: Int): TextNode = new TextNode(int.toString)
-
-  @inline implicit def doubleToNode(double: Double): TextNode = new TextNode(double.toString)
+  /** Implicit [[Renderable]] instances are available for primitive types (defined in modifiers/Renderable.scala) */
+  implicit def renderableToTextNode[A](value: A)(implicit renderable: Renderable[A]): TextNode = {
+    renderable.asTextNode(value)
+  }
 
   /** Create a setter that applies each of the setters in a seq */
   implicit def seqToSetter[El <: ReactiveElement.Base](setters: scala.collection.Seq[Setter[El]]): Setter[El] = {
@@ -123,11 +120,11 @@ object Implicits {
     // Inserter implicits are needlessly expensive if we just need a Modifier, so we de-prioritize them
 
     implicit def nodeToInserter(node: ChildNode.Base): Inserter[ReactiveElement.Base] = {
-      ChildInserter[ReactiveElement.Base](_ => Val(node))
+      ChildInserter[ReactiveElement.Base](Val(node))
     }
 
     implicit def nodesSeqToInserter(nodes: scala.collection.Seq[ChildNode.Base]): Inserter[ReactiveElement.Base] = {
-      ChildrenInserter[ReactiveElement.Base](_ => Val(nodes.toList))
+      ChildrenInserter[ReactiveElement.Base](Val(nodes.toList))
     }
 
     implicit def nodesArrayToInserter(nodes: Array[ChildNode.Base]): Inserter[ReactiveElement.Base] = {
@@ -137,6 +134,10 @@ object Implicits {
     @inline implicit def nodesJsArrayToInserter[N <: ChildNode.Base](nodes: js.Array[N]): Inserter[ReactiveElement.Base] = {
       nodesSeqToInserter(nodes)
     }
+
+    implicit def renderableToInserter[A](value: A)(implicit renderable: Renderable[A]): Inserter[ReactiveElement.Base] = {
+      ChildTextInserter[A, ReactiveElement.Base](Val(value), renderable)
+    }
   }
 
   /** Some of these methods are redundant, but we need them for type inference to work. */

src/main/scala/com/raquo/laminar/api/Laminar.scala

@@ -310,12 +310,14 @@ private[laminar] object Laminar
   def onMountInsert[El <: Element](fn: MountContext[El] => Inserter[El]): Modifier[El] = {
     Modifier[El] { element =>
       var maybeSubscription: Option[DynamicSubscription] = None
-      val lockedContext = InsertContext.reserveSpotContext[El](element)
+      // We start the context in loose mode for performance, because it's cheaper to go from there
+      // to strict mode, than the other way. The inserters are able to handle any initial mode.
+      val lockedInsertContext = InsertContext.reserveSpotContext[El](element, strictMode = false)
       element.amend(
         onMountUnmountCallback[El](
-          mount = { c =>
-            val inserter = fn(c).withContext(lockedContext)
-            maybeSubscription = Some(inserter.bind(c.thisNode))
+          mount = { mountContext =>
+            val inserter = fn(mountContext).withContext(lockedInsertContext)
+            maybeSubscription = Some(inserter.bind(mountContext.thisNode))
           },
           unmount = { _ =>
             maybeSubscription.foreach(_.kill())
@@ -397,10 +399,13 @@ private[laminar] object Laminar
         } else {
           state = Some(mount(c))
         }
-        new Subscription(c.owner, cleanup = () => {
-          unmount(element, state)
-          state = None
-        })
+        new Subscription(
+          c.owner,
+          cleanup = () => {
+            unmount(element, state)
+            state = None
+          }
+        )
       }
     }
   }

src/main/scala/com/raquo/laminar/lifecycle/InsertContext.scala

@@ -1,6 +1,8 @@
 package com.raquo.laminar.lifecycle
 
 import com.raquo.ew.JsMap
+import com.raquo.laminar.DomApi
+import com.raquo.laminar.modifiers.ChildrenInserter.Children
 import com.raquo.laminar.nodes.{ChildNode, CommentNode, ParentNode, ReactiveElement}
 import org.scalajs.dom
 
@@ -11,26 +13,160 @@ import scala.collection.immutable
 
 // @Note only parentNode and sentinelNode are used by all Inserter-s.
 //  - Other fields may remain un-updated if they are not needed for a particular use case.
+
+/**
+ * InsertContext represents the state of the DOM inside an inserter block like `child <-- ...`,
+ * `children <-- ...`, `child.text <-- ...`, etc. The data stored in this context is used
+ * by Laminar to efficiently update the DOM, to detect (and recover from) external changes
+ * to the DOM, and for other related tasks.
+ *
+ * InsertContext is a mutable data structure that is created once for each inserter, and the
+ * inserter updates it as it processes new data. However, in case of `onMountInsert`, only
+ * one context is created, and is then reused for all inserters created inside
+ * `onMountInsert`. This allows for intuitive preservation of DOM state if the element is
+ * unmounted and mounted again (for example, `onMountInsert(child <-- stream)` will
+ * keep the last emitted child in the DOM even if the element is unmounted and re-mounted).
+ *
+ * #Note: The params that describe `extraNodes` below can get out of sync with the real DOM.
+ *
+ * This can happen if an child element is removed from the DOM – either externally, or more
+ * likely because it was moved from this inserter into another one, and the addition to the
+ * other inserter was processed before the removal from this inserter is processed (the
+ * order of these operations depends on the propagation order of the observables feeding
+ * these two inserters). The Inserter code must account for this and not fail in such cases,
+ * and must correct the values accordingly on the next observable update.
+ *
+ * #Note: The params that describe `extraNodes` below must be kept consistent manually (#Perf)
+ *
+ *                              Inserter "steals" an element from this one just before the
+ *                              observable source of this inserter provides a new list of
+ *                              children (with the stolen element removed from the list).
+ *
+ * @param sentinelNode        - A special invisible comment node that tells Laminar where to
+ *                              insert the dynamic children, and where to expect previously
+ *                              inserted dynamic children.
+ * @param strictMode          - If true, Laminar guarantees that it will keep a dedicated
+ *                              sentinel node instead of using the extra node (content node)
+ *                              for that purpose. This is needed in order to allow users to
+ *                              move an element from one inserter to another, or to externally
+ *                              remove some of the elements previously added by an inserter.
+ *                              child.text does not need any of that, so for performance it
+ *                              does not use strict mode, it replaces the sentinel comment
+ *                              node with the subsequent text nodes. Inserters should be able
+ *                              to safely switch to their preferred mode when receiving
+ *                              context left by the previous inserter in onMountBind.
+ * @param extraNodeCount      - Number of child nodes in addition to the sentinel node.
+ *                              Warning: can get out of sync with the real DOM!
+ * @param extraNodes          - Ordered list of child nodes in addition to the sentinel node.
+ *                              Warning: can get out of sync with the real DOM!
+ * @param extraNodesMap       - Map of child nodes, for more efficient search
+ *                              Warning: can get out of sync with the real DOM!
+ */
 final class InsertContext[+El <: ReactiveElement.Base](
   val parentNode: El,
   var sentinelNode: ChildNode.Base,
+  var strictMode: Boolean,
   var extraNodeCount: Int, // This is separate from `extraNodesMap` for performance #TODO[Performance]: Check if this is still relevant with JsMap
+  var extraNodes: Children,
   var extraNodesMap: JsMap[dom.Node, ChildNode.Base]
-)
+) {
+
+  /**
+   * This method converts the InsertContext from loose mode to strict mode.
+   * ChildrenInserter and ChildInserter call this when receiving a context from
+   * ChildTextInserter. This can happen when switching from `child.text <-- ...`
+   * to e.g. `children <-- ...` inside onMountInsert.
+   *
+   * Prerequisite: context must be in loose mode, and in valid state: no extra nodes allowed.
+   */
+  def forceSetStrictMode(): Unit = {
+    if (strictMode || extraNodeCount != 0) {
+      // #Note: if extraNodeCount == 0, it is also assumed (but not tested) that extraNodes and extraNodesMap are empty.
+      throw new Exception(s"forceSetStrictMode invoked when not allowed, inside parent = ${DomApi.debugNodeOuterHtml(parentNode.ref)}")
+    }
+    if (extraNodesMap == null) {
+      // In loose mode, extraNodesMap is likely to be null, so we need to initialize it.
+      extraNodesMap = new JsMap()
+    }
+    if (sentinelNode.ref.isInstanceOf[dom.Comment]) {
+      // This means there are no content nodes.
+      // We assume that all extraNode fields are properly zeroed, so there is nothing to do.
+    } else {
+      // In loose mode, child content nodes are written to sentinelNode field,
+      // so there are no extraNodes.
+      // So, if we find a content node in sentinelNode, we need to reclassify
+      // it as such for the strict mode, and insert a new sentinel node into the DOM.
+      val contentNode = sentinelNode
+      val newSentinelNode = new CommentNode("")
+      ParentNode.insertChild(
+        parent = parentNode,
+        child = newSentinelNode,
+        atIndex = ParentNode.indexOfChild(
+          parent = parentNode,
+          child = contentNode
+        )
+      )
+
+      // Convert loose mode context values to strict mode context values
+      sentinelNode = newSentinelNode
+      extraNodeCount = 1
+      extraNodes = contentNode :: Nil
+      extraNodesMap.set(contentNode.ref, contentNode) // we initialized the map above
+    }
+    strictMode = true
+  }
+
+  /** #Note: this does NOT update the context to match the DOM. */
+  def removeOldChildNodesFromDOM(after: ChildNode.Base): Unit = {
+    var remainingOldExtraNodeCount = extraNodeCount
+    while (remainingOldExtraNodeCount > 0) {
+      val prevChildRef = after.ref.nextSibling
+      if (prevChildRef == null) {
+        // We expected more previous children to be in the DOM, but we reached the end of the DOM.
+        // Those children must have been removed from the DOM manually, or moved to a different inserter.
+        // So, the DOM state is now correct, albeit "for the wrong reasons". All is good. End the loop.
+        remainingOldExtraNodeCount = 0
+      } else {
+        val maybePrevChild = extraNodesMap.get(prevChildRef)
+        if (maybePrevChild.isEmpty) {
+          // Similar to the prevChildRef == null case above, we've exhausted the DOM,
+          // except we stumbled on some unrelated element instead. We only allow external
+          // removals from the DOM, not external additions in the middle of dynamic children list,
+          // so this unrelated element is good evidence that there are no more old child nodes
+          // to be found.
+          remainingOldExtraNodeCount = 0
+        } else {
+          maybePrevChild.foreach { prevChild =>
+            // @Note: DOM update
+            ParentNode.removeChild(parent = parentNode, child = prevChild)
+            remainingOldExtraNodeCount -= 1
+          }
+        }
+      }
+    }
+  }
+}
 
 object InsertContext {
 
   /** Reserve the spot for when we actually insert real nodes later */
-  def reserveSpotContext[El <: ReactiveElement.Base](parentNode: El): InsertContext[El] = {
+  def reserveSpotContext[El <: ReactiveElement.Base](
+    parentNode: El,
+    strictMode: Boolean
+  ): InsertContext[El] = {
     val sentinelNode = new CommentNode("")
 
     ParentNode.appendChild(parent = parentNode, child = sentinelNode)
 
+    // #Warning[Fragile] - We avoid instantiating a JsMap in loose mode, for performance.
+    //  The JsMap is initialized if/when needed, in forceSetStrictMode.
     new InsertContext[El](
       parentNode = parentNode,
       sentinelNode = sentinelNode,
+      strictMode = strictMode,
       extraNodeCount = 0,
-      extraNodesMap = new JsMap()
+      extraNodes = Nil,
+      extraNodesMap = if (strictMode) new JsMap() else null
     )
   }
 

src/main/scala/com/raquo/laminar/modifiers/ChildInserter.scala

@@ -1,29 +1,65 @@
 package com.raquo.laminar.modifiers
 
 import com.raquo.airstream.core.Observable
-import com.raquo.laminar.lifecycle.{InsertContext, MountContext}
 import com.raquo.laminar.nodes.{ChildNode, ParentNode, ReactiveElement}
 
 import scala.scalajs.js
 
 object ChildInserter {
 
   def apply[El <: ReactiveElement.Base] (
-    $child: MountContext[El] => Observable[ChildNode.Base]
-  ): Inserter[El] = new Inserter[El](
-    insertFn = (ctx, owner) => {
-      val mountContext = new MountContext[El](
-        thisNode = ctx.parentNode,
-        owner = owner
-      )
-      var lastSeenChild: js.UndefOr[ChildNode.Base] = js.undefined
-      $child(mountContext).foreach { newChildNode =>
-        if (!lastSeenChild.exists(_ eq newChildNode)) { // #Note: auto-distinction
-          lastSeenChild = newChildNode
-          ParentNode.replaceChild(parent = ctx.parentNode, oldChild = ctx.sentinelNode, newChild = newChildNode)
-          ctx.sentinelNode = newChildNode
+    $child: Observable[ChildNode.Base]
+  ): Inserter[El] = {
+    new Inserter[El](
+      preferStrictMode = true,
+      insertFn = (ctx, owner) => {
+        if (!ctx.strictMode) {
+          ctx.forceSetStrictMode()
         }
-      }(owner)
-    }
-  )
+
+        var maybeLastSeenChild: js.UndefOr[ChildNode.Base] = js.undefined
+
+        $child.foreach { newChildNode =>
+          var remainingOldExtraNodeCount = ctx.extraNodeCount
+
+          maybeLastSeenChild
+            .filter(_.ref == ctx.sentinelNode.ref.nextSibling) // Assert that the prev child node was not moved. Note: nextSibling could be null
+            .fold {
+              // Inserting the child for the first time, OR after the previous child was externally moved / removed.
+              val sentinelNodeIndex = ParentNode.indexOfChild(ctx.parentNode, ctx.sentinelNode)
+              ParentNode.insertChild(parent = ctx.parentNode, newChildNode, atIndex = sentinelNodeIndex + 1)
+              ()
+            } { lastSeenChild =>
+              // We found the existing child in the right place in the DOM
+              // Just need to check that the new child is actually different from the old one
+              // Replace the child with new one.
+              // #Note: auto-distinction inside (lastSeenChild != newChildNode filter)
+              val replaced = ParentNode.replaceChild(
+                parent = ctx.parentNode,
+                oldChild = lastSeenChild,
+                newChild = newChildNode
+              )
+              if (replaced || lastSeenChild == newChildNode) { // #TODO[Performance,Integrity] Not liking this redundant auto-distinction
+                // The only time we DON'T decrement this is when replacing fails for unexpected reasons.
+                // - If lastSeenChild == newChildNode, then it's not an "old" node anymore, so we decrement
+                // - If replaced == true, then lastSeenChild was removed from the DOM, so we decrement
+                remainingOldExtraNodeCount -= 1
+              }
+              ()
+            }
+
+          // We've just inserted newChildNode after the sentinel, or replaced the first old node with newChildNode,
+          // so any remaining old child nodes must be directly under it.
+          ctx.removeOldChildNodesFromDOM(after = newChildNode)
+
+          ctx.extraNodesMap.clear()
+          ctx.extraNodesMap.set(newChildNode.ref, newChildNode)
+          ctx.extraNodes = newChildNode :: Nil
+          ctx.extraNodeCount = 1
+
+          maybeLastSeenChild = newChildNode
+        }(owner)
+      }
+    )
+  }
 }