Pass sibling to `Renderer.mount`, fix update order (#301)

Resolves, but adds no tests cases to the test suite for #294. See the issue for the detailed description of the problem.

I will add end-to-end tests for this in future PRs.

I've tested these cases manually so far:

```swift
struct Choice: View {
  @State private var choice = false

  var body: some View {
    HStack {
      Button("Trigger") {
        choice.toggle()
      }
      if choice {
        Group {
          Text("true")
          Text("true")
        }
      } else {
        VStack {
          Text("false")
        }
      }
      Text("end")
    }
  }
}
```

Note the `Group` view with multiple children in this one, it uncovered required checks for `GroupView` conformance.

Also tested these more simple cases:

```swift
struct Choice: View {
  @State private var choice = false

  var body: some View {
    HStack {
      Button("Trigger") {
        choice.toggle()
      }
      if choice {
        Group {
          // single child
          Text("true")
        }
      } else {
        VStack {
          Text("false")
        }
      }
      Text("end")
    }
  }
}
```

and

```swift
struct Choice: View {
  @State private var choice = false

  var body: some View {
    HStack {
      Button("Trigger") {
        choice.toggle()
      }
      if choice {
        // single child, no nesting
        Text("true")
      } else {
        VStack {
          Text("false")
        }
      }
      Text("end")
    }
  }
}
```

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
       - uses: actions/checkout@v2
       - uses: swiftwasm/swiftwasm-action@v5.3
         with:
-          shell-action: swift build --triple wasm32-unknown-wasi --product TokamakDemo
+          shell-action: carton bundle --product TokamakDemo
 
   macos_build:
     runs-on: macos-10.15

Sources/TokamakCore/MountedViews/MountedApp.swift

@@ -22,12 +22,13 @@ import Runtime
 // is the computed content of the specified `Scene`, instead of having child
 // `View`s
 final class MountedApp<R: Renderer>: MountedCompositeElement<R> {
-  override func mount(with reconciler: StackReconciler<R>) {
+  override func mount(before _: R.TargetType? = nil, with reconciler: StackReconciler<R>) {
+    // `App` elements have no siblings, hence the `before` argument is discarded.
     let childBody = reconciler.render(mountedApp: self)
 
     let child: MountedElement<R> = mountChild(childBody)
     mountedChildren = [child]
-    child.mount(with: reconciler)
+    child.mount(before: nil, with: reconciler)
   }
 
   override func unmount(with reconciler: StackReconciler<R>) {

Sources/TokamakCore/MountedViews/MountedCompositeView.swift

@@ -19,12 +19,12 @@ import CombineShim
 import Runtime
 
 final class MountedCompositeView<R: Renderer>: MountedCompositeElement<R> {
-  override func mount(with reconciler: StackReconciler<R>) {
+  override func mount(before sibling: R.TargetType? = nil, with reconciler: StackReconciler<R>) {
     let childBody = reconciler.render(compositeView: self)
 
     let child: MountedElement<R> = childBody.makeMountedView(parentTarget, environmentValues)
     mountedChildren = [child]
-    child.mount(with: reconciler)
+    child.mount(before: sibling, with: reconciler)
 
     // `_TargetRef` is a composite view, so it's enough to check for it only here
     if var targetRef = view.view as? TargetRefType {

Sources/TokamakCore/MountedViews/MountedElement.swift

@@ -76,10 +76,10 @@ public class MountedElement<R: Renderer> {
   var typeConstructorName: String {
     switch element {
     case .app: fatalError("""
-    `App` values aren't supposed to be reconciled, thus the type constructor name is not stored \
-    for `App` elements. Please report this crash as a bug at \
-    https://github.com/swiftwasm/Tokamak/issues/new
-    """)
+      `App` values aren't supposed to be reconciled, thus the type constructor name is not stored \
+      for `App` elements. Please report this crash as a bug at \
+      https://github.com/swiftwasm/Tokamak/issues/new
+      """)
     case let .scene(scene): return scene.typeConstructorName
     case let .view(view): return view.typeConstructorName
     }
@@ -122,7 +122,7 @@ public class MountedElement<R: Renderer> {
     return info
   }
 
-  func mount(with reconciler: StackReconciler<R>) {
+  func mount(before sibling: R.TargetType? = nil, with reconciler: StackReconciler<R>) {
     fatalError("implement \(#function) in subclass")
   }
 
@@ -133,6 +133,19 @@ public class MountedElement<R: Renderer> {
   func update(with reconciler: StackReconciler<R>) {
     fatalError("implement \(#function) in subclass")
   }
+
+  /** Traverses the tree of elements from `self` to all first descendants looking for the nearest
+   `target` in a `MountedHostView`, skipping `GroupView`. The result is then used as a "cursor"
+   passed to the `mount` function of a `Renderer` implementation, allowing correct in-tree updates.
+   */
+  var firstDescendantTarget: R.TargetType? {
+    guard let hostView = self as? MountedHostView<R>, !(hostView.view.type is GroupView.Type)
+    else {
+      return mountedChildren.first?.firstDescendantTarget
+    }
+
+    return hostView.target
+  }
 }
 
 extension TypeInfo {

Sources/TokamakCore/MountedViews/MountedEmptyView.swift

@@ -16,7 +16,7 @@
 //
 
 final class MountedEmptyView<R: Renderer>: MountedElement<R> {
-  override func mount(with reconciler: StackReconciler<R>) {}
+  override func mount(before sibling: R.TargetType? = nil, with reconciler: StackReconciler<R>) {}
 
   override func unmount(with reconciler: StackReconciler<R>) {}
 

Sources/TokamakCore/MountedViews/MountedHostView.swift

@@ -25,7 +25,7 @@ public final class MountedHostView<R: Renderer>: MountedElement<R> {
    might not be a host view, but a composite view, we need to pass
    around the target of a host view to its closests descendant host
    views. Thus, a parent target is not always the same as a target of
-   a parent view. */
+   a parent `View`. */
   private let parentTarget: R.TargetType
 
   /// Target of this host view supplied by a renderer after mounting has completed.
@@ -37,8 +37,12 @@ public final class MountedHostView<R: Renderer>: MountedElement<R> {
     super.init(view, environmentValues)
   }
 
-  override func mount(with reconciler: StackReconciler<R>) {
-    guard let target = reconciler.renderer?.mountTarget(to: parentTarget, with: self)
+  override func mount(before sibling: R.TargetType? = nil, with reconciler: StackReconciler<R>) {
+    guard let target = reconciler.renderer?.mountTarget(
+      before: sibling,
+      to: parentTarget,
+      with: self
+    )
     else { return }
 
     self.target = target
@@ -48,7 +52,14 @@ public final class MountedHostView<R: Renderer>: MountedElement<R> {
     mountedChildren = view.children.map {
       $0.makeMountedView(target, environmentValues)
     }
-    mountedChildren.forEach { $0.mount(with: reconciler) }
+
+    /* Remember that `GroupView`s are always "flattened", their `target` instances are targets of
+     their parent elements. We need the insertion "cursor" `sibling` to be preserved when children
+     are mounted in that case. Thus pass the `sibling` target to the children if `view` is a
+     `GroupView`.
+     */
+    let isGroupView = view.type is GroupView.Type
+    mountedChildren.forEach { $0.mount(before: isGroupView ? sibling : nil, with: reconciler) }
   }
 
   override func unmount(with reconciler: StackReconciler<R>) {
@@ -91,18 +102,22 @@ public final class MountedHostView<R: Renderer>: MountedElement<R> {
       // iterate through every `mountedChildren` element and compare with
       // a corresponding `childrenViews` element, remount if type differs, otherwise
       // run simple update
-      while let child = mountedChildren.first, let firstChild = childrenViews.first {
+      while let mountedChild = mountedChildren.first, let childView = childrenViews.first {
         let newChild: MountedElement<R>
-        if firstChild.typeConstructorName == mountedChildren[0].view.typeConstructorName {
-          child.environmentValues = environmentValues
-          child.view = firstChild
-          child.updateEnvironment()
-          child.update(with: reconciler)
-          newChild = child
+        if childView.typeConstructorName == mountedChildren[0].view.typeConstructorName {
+          mountedChild.environmentValues = environmentValues
+          mountedChild.view = childView
+          mountedChild.updateEnvironment()
+          mountedChild.update(with: reconciler)
+          newChild = mountedChild
         } else {
-          child.unmount(with: reconciler)
-          newChild = firstChild.makeMountedView(target, environmentValues)
-          newChild.mount(with: reconciler)
+          /* note the order of operations here: we mount the new child first, use the mounted child
+            as a "cursor" sibling when mounting. Only then we can dispose of the old mounted child
+            by unmounting it.
+           */
+          newChild = childView.makeMountedView(target, environmentValues)
+          newChild.mount(before: mountedChild.firstDescendantTarget, with: reconciler)
+          mountedChild.unmount(with: reconciler)
         }
         newChildren.append(newChild)
         mountedChildren.removeFirst()

Sources/TokamakCore/MountedViews/MountedScene.swift

@@ -29,12 +29,12 @@ final class MountedScene<R: Renderer>: MountedCompositeElement<R> {
     mountedChildren = children
   }
 
-  override func mount(with reconciler: StackReconciler<R>) {
+  override func mount(before sibling: R.TargetType? = nil, with reconciler: StackReconciler<R>) {
     let childBody = reconciler.render(mountedScene: self)
 
     let child: MountedElement<R> = childBody.makeMountedElement(parentTarget, environmentValues)
     mountedChildren = [child]
-    child.mount(with: reconciler)
+    child.mount(before: sibling, with: reconciler)
   }
 
   override func unmount(with reconciler: StackReconciler<R>) {

Sources/TokamakCore/Renderer.swift

@@ -31,9 +31,6 @@ public protocol Renderer: AnyObject {
    */
   associatedtype TargetType: Target
 
-  /// Reconciler instance used by this renderer.
-  var reconciler: StackReconciler<Self>? { get }
-
   /** Function called by a reconciler when a new target instance should be
    created and added to the parent (either as a subview or some other way, e.g.
    installed if it's a layout constraint).
@@ -42,6 +39,7 @@ public protocol Renderer: AnyObject {
    - returns: The newly created target.
    */
   func mountTarget(
+    before sibling: TargetType?,
     to parent: TargetType,
     with host: MountedHost
   ) -> TargetType?

Sources/TokamakCore/Views/View.swift

@@ -34,7 +34,7 @@ public protocol ParentView {
   var children: [AnyView] { get }
 }
 
-/// A `View` type that is not rendered, but "flattened" rendering all its children instead.
+/// A `View` type that is not rendered but "flattened", rendering all its children instead.
 protocol GroupView: ParentView {}
 
 /** The distinction between "host" (truly primitive) and "composite" (that have meaningful `body`)

Sources/TokamakDOM/DOMRenderer.swift

@@ -71,7 +71,7 @@ func appendRootStyle(_ rootNode: JSObject) {
 }
 
 final class DOMRenderer: Renderer {
-  private(set) var reconciler: StackReconciler<DOMRenderer>?
+  private var reconciler: StackReconciler<DOMRenderer>?
 
   private let rootRef: JSObject
 
@@ -91,12 +91,16 @@ final class DOMRenderer: Renderer {
     ) { scheduler.schedule(options: nil, $0) }
   }
 
-  public func mountTarget(to parent: DOMNode, with host: MountedHost) -> DOMNode? {
+  public func mountTarget(
+    before sibling: DOMNode?,
+    to parent: DOMNode,
+    with host: MountedHost
+  ) -> DOMNode? {
     guard let anyHTML = mapAnyView(
       host.view,
       transform: { (html: AnyHTML) in html }
     ) else {
-      // handle cases like `TupleView`
+      // handle `GroupView` cases (such as `TupleView`, `Group` etc)
       if mapAnyView(host.view, transform: { (view: ParentView) in view }) != nil {
         return parent
       }
@@ -104,27 +108,36 @@ final class DOMRenderer: Renderer {
       return nil
     }
 
-    _ = parent.ref.insertAdjacentHTML!("beforeend", JSValue(stringLiteral: anyHTML.outerHTML))
+    let maybeNode: JSObject?
+    if let sibling = sibling {
+      _ = sibling.ref.insertAdjacentHTML!("beforebegin", anyHTML.outerHTML)
+      maybeNode = sibling.ref.previousSibling.object
+    } else {
+      _ = parent.ref.insertAdjacentHTML!("beforeend", anyHTML.outerHTML)
 
-    guard
-      let children = parent.ref.childNodes.object,
-      let length = children.length.number,
-      length > 0,
-      let lastChild = children[Int(length) - 1].object
-    else { return nil }
+      guard
+        let children = parent.ref.childNodes.object,
+        let length = children.length.number,
+        length > 0
+      else { return nil }
+
+      maybeNode = children[Int(length) - 1].object
+    }
+
+    guard let resultingNode = maybeNode else { return nil }
 
     let fillAxes = host.view.fillAxes
     if fillAxes.contains(.horizontal) {
-      lastChild.style.object!.width = "100%"
+      resultingNode.style.object!.width = "100%"
     }
     if fillAxes.contains(.vertical) {
-      lastChild.style.object!.height = "100%"
+      resultingNode.style.object!.height = "100%"
     }
 
     if let dynamicHTML = anyHTML as? AnyDynamicHTML {
-      return DOMNode(host.view, lastChild, dynamicHTML.listeners)
+      return DOMNode(host.view, resultingNode, dynamicHTML.listeners)
     } else {
-      return DOMNode(host.view, lastChild, [:])
+      return DOMNode(host.view, resultingNode, [:])
     }
   }
 
@@ -143,7 +156,7 @@ final class DOMRenderer: Renderer {
   ) {
     defer { completion() }
 
-    guard mapAnyView(host.view, transform: { (html: AnyHTML) in html }) != nil
+    guard let html = mapAnyView(host.view, transform: { (html: AnyHTML) in html })
     else { return }
 
     _ = parent.ref.removeChild!(target.ref)

Sources/TokamakDOM/JSScheduler.swift

@@ -12,12 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import CombineShim
 import JavaScriptKit
-#if canImport(Combine)
-import Combine
-#else
-import OpenCombine
-#endif
 
 final class JSScheduler: Scheduler {
   private final class CancellableTimer: Cancellable {

Sources/TokamakStaticHTML/StaticRenderer.swift

@@ -65,7 +65,7 @@ struct HTMLBody: AnyHTML {
 }
 
 public final class StaticHTMLRenderer: Renderer {
-  public private(set) var reconciler: StackReconciler<StaticHTMLRenderer>?
+  private var reconciler: StackReconciler<StaticHTMLRenderer>?
 
   var rootTarget: HTMLTarget
 
@@ -112,7 +112,11 @@ public final class StaticHTMLRenderer: Renderer {
     )
   }
 
-  public func mountTarget(to parent: HTMLTarget, with host: MountedHost) -> HTMLTarget? {
+  public func mountTarget(
+    before _: HTMLTarget?,
+    to parent: HTMLTarget,
+    with host: MountedHost
+  ) -> HTMLTarget? {
     guard let html = mapAnyView(
       host.view,
       transform: { (html: AnyHTML) in html }

Sources/TokamakTestRenderer/TestRenderer.swift

@@ -52,6 +52,7 @@ public final class TestRenderer: Renderer {
   }
 
   public func mountTarget(
+    before _: TestView?,
     to parent: TestView,
     with mountedHost: TestRenderer.MountedHost
   ) -> TestView? {

Tests/LinuxMain.swift

@@ -1,8 +0,0 @@
-import XCTest
-
-import TokamakTests
-
-var tests = [XCTestCaseEntry]()
-tests += TokamakTests.__allTests()
-
-XCTMain(tests)

Tests/TokamakTests/XCTestManifests.swift

@@ -1,31 +0,0 @@
-#if !canImport(ObjectiveC)
-import XCTest
-
-extension ColorTests {
-  // DO NOT MODIFY: This is autogenerated, use:
-  //   `swift test --generate-linuxmain`
-  // to regenerate.
-  static let __allTests__ColorTests = [
-    ("testHexColors", testHexColors),
-  ]
-}
-
-extension ReconcilerTests {
-  // DO NOT MODIFY: This is autogenerated, use:
-  //   `swift test --generate-linuxmain`
-  // to regenerate.
-  static let __allTests__ReconcilerTests = [
-    ("testDoubleUpdate", testDoubleUpdate),
-    ("testMount", testMount),
-    ("testUnmount", testUnmount),
-    ("testUpdate", testUpdate),
-  ]
-}
-
-public func __allTests() -> [XCTestCaseEntry] {
-  [
-    testCase(ColorTests.__allTests__ColorTests),
-    testCase(ReconcilerTests.__allTests__ReconcilerTests),
-  ]
-}
-#endif