diff --git a/notes/Inserters.md b/notes/Inserters.md index a8b50dac..7719ee88 100644 --- a/notes/Inserters.md +++ b/notes/Inserters.md @@ -96,6 +96,15 @@ An item that leaves the list is removed from the DOM and its per-item lifecycle ### Remove (no-op after a steal) If the node/group is no longer under this parent (already stolen), `removeFromDynamicList` does nothing — the new host owns it now. This is the "last write wins" safety valve. +### Remove, keeping nested content (un-nesting) +Removing old content can leave specific items in place: `removeContentMapNodesFromDom` takes a `keepItem` predicate, and passes it down through `removeFromDynamicList` → `NestedGroup.removeFromParent` into every nested item it tears down. The predicate is called with each item's `stableFirstNode` – its identity, the same key as in `contentMap` – not with an inserter object, because the same item can be represented by different inserter objects over time (e.g. fresh `SlottableChildInserter` wrappers on every emission). A kept item – a plain node, or a nested dynamic inserter with its whole span and live subscription – stays in the parent's DOM where it was, while the nested group around it is torn down. It never loses its parent, so it is not unmounted. Like moving a plain element out of a wrapper that's being removed, within the same parent. + +The caller must then promptly place every kept item where it belongs – otherwise it would be left in the DOM, untracked. Callers: + +- `child <--` taking over a span that contains its new node, even nested (§7). +- `children <--` reconciliation removing a leaving item that contains nodes / inserters that the same emission keeps (`InserterMoveSpec` §4e). +- `children.command <--` `ReplaceAll(newNodes, minimizeDiff = true)`, keeping the nodes that stay in the list. It also leaves kept nodes that are already in the right place untouched in the DOM. With `minimizeDiff = false`, `ReplaceAll` skips this work for performance: it removes all current nodes, then appends `newNodes`, re-mounting any overlap. + ### External mutation Laminar tolerates a user/third-party removing an inserter's node from the DOM directly: the next reconciliation walks the live span and simply doesn't find it, correcting the count (`InserterExternalMutationSpec`; `ChildrenInserter.updateChildren` count-correction, referencing issue #120). External _insertion_ into a bracketed span (between our sentinels) is reported as an error on teardown/removeAll paths (`removeContentMapNodesFromDom` with a trailing sentinel), because we can't tell an intruder from our own content otherwise. External insertion into an unbracketed (`child <--`) span is silently treated as "the next sibling after our span" — we stop the walk there. @@ -149,7 +158,8 @@ If the previous host genuinely _removed_ the group (set `nestedGroupOpt = js.und - **Per-item lifecycle is preserved across a transfer** — the owner is transferred, not rebuilt, so per-item `onMount`/`onUnmount` and internal subscriptions stay live (`InserterMoveSpec` "add-first / steal keeps the item's per-item lifecycle intact"). - **Ordering is empirical but pinned.** Some teardown/swap orders are not obvious and are deliberately encoded by tests (see `notes/Testing.md`): - `child <--` self-replace swaps **unmount-old-then-mount-new**. - - A takeover of a _foreign_ span mounts-new-then-unmounts-old. + - A `child <--` takeover of a _foreign_ span also unmounts the old content before mounting the new node. The node it keeps (§7) is neither unmounted nor re-mounted. + - `children.command <--` `ReplaceAll` unmounts the leaving nodes before mounting the new ones. With `minimizeDiff = false`, all old nodes are considered "leaving", including those that are then re-inserted. - `children <--` teardown walks `contentMap` in **insertion order**, not current DOM order (several `#Note` comments; e.g. `InserterMoveSpec:890`, `InserterExternalMutationSpec:148`). This is teardown only — relocation uses DOM order (§5). - `Replace` (command) unmounts the old node then mounts the new (`InserterMoveSpec:1718`). @@ -166,7 +176,7 @@ If the previous host genuinely _removed_ the group (set `nestedGroupOpt = js.und - Switching TO `child <--` / `text <--` clears prior multi-node content down to (at most) the one node being kept, and drops the trailing sentinel. - Switching TO `children.command <--` clears any content left by a _non-command_ inserter (commands can't patch foreign content to a target state), but **preserves** content it built itself across a mere remount, and preserves content built by a _different_ command inserter (`InserterTakeoverSpec` "children.command → children.command (different inserter) keeps the previous content"). - A takeover that tears down a span reports an externally-inserted intruder (§3). -- A takeover does **not** blindly tear down everything: `children <--` (a,b) → `child <-- b` keeps b mounted and unmounts only a (`InserterTakeoverSpec`). +- A takeover does **not** blindly tear down everything: `children <--` (a,b) → `child <-- b` keeps b mounted and unmounts only a (`InserterTakeoverSpec`). This holds even if b is nested inside one of the list's dynamic items, at any depth: those items are torn down around b (§3 "un-nesting"). ### The `setNextInserterType` invariant Whenever there is no trailing sentinel, `contentMap` holds **at most one** node. The guard in `setNextInserterType` throws if we ever switch to a trailing-sentinel type while lacking a trailing sentinel but already holding >1 content node (we wouldn't know where the pre-existing content ends). This is structurally unreachable via the public API; `InserterInvariantSpec` pins both the near-miss safety and the white-box guard firing. @@ -226,10 +236,12 @@ Distinct from the last-write-wins contest above: when a slot could come from an ## 10. Known deviations from the ideal -### Provably unachievable (not bugs we can fix) +### Accepted limitations - **Issue #163 — moving an element between two sibling `child <--` bindings re-mounts in one direction only.** When one element is shown via one of two independent `child <--` bindings toggled by a single signal, whether the toggle re-mounts is _order-dependent_: the binding that GAINS the element must fire before the one that LOSES it for the transfer to be seamless. When the losing binding fires first, the element is detached to `None` (unmount) before re-attachment (mount). This is inherent to synchronous propagation order; a `delaySync` workaround only fixes one direction. Characterized (not "fixed") in `InserterMoveSpec` "CHARACTERIZATION (issue #163)". The `probe(addFirst=false)` remove-first re-mount in the REFERENCE test is the same underlying limitation. +- **Re-wrapping a nested node into a NEW nested inserter can re-mount it.** `children <--` (nested `child <-- b`, a) → (a, NEW nested `child <-- b`): reconciliation removes the leaving item before it reaches the new one, and nothing is known about the new inserter's content until it subscribes, so b can't be kept (§3 "un-nesting"), and is unmounted, then mounted again. If the new inserter comes BEFORE the leaving item in the list, it takes b before the old item is removed, which is seamless. Fixing the general case would require deferring the teardown of leaving items until the end of reconciliation. Characterized in `InserterMoveSpec` "CHARACTERIZATION: re-wrapping a nested node…". + --- ## 11. Suspicious / uncertain intent (flagged for review) diff --git a/notes/Testing.md b/notes/Testing.md index c439a5b2..358376c0 100644 --- a/notes/Testing.md +++ b/notes/Testing.md @@ -41,7 +41,7 @@ Assertion messages should include enough details to pinpoint and diagnose the is ## Ordering is empirical -Exact teardown / swap order is behaviour worth pinning but not always obvious — e.g. `child <--` swaps **unmount-old-then-mount-new** (self-replace) but a takeover of a foreign span is **mount-new-then-unmount-old**; `children <--` teardown walks the contentMap in **insertion order**, not current DOM order. Write your best guess, run the spec, and encode the order the failure reports, with a one-line // #Note: comment explaining it. +Exact teardown / swap order is behaviour worth pinning but not always obvious — e.g. `child <--` swaps **unmount-old-then-mount-new** (both self-replace and a takeover of a foreign span); `children <--` teardown walks the contentMap in **insertion order**, not current DOM order. Write your best guess, run the spec, and encode the order the failure reports, with a one-line // #Note: comment explaining it. ## Misc style diff --git a/src/main/scala/com/raquo/laminar/inserters/ChildInserter.scala b/src/main/scala/com/raquo/laminar/inserters/ChildInserter.scala index 89615b98..9753a99f 100644 --- a/src/main/scala/com/raquo/laminar/inserters/ChildInserter.scala +++ b/src/main/scala/com/raquo/laminar/inserters/ChildInserter.scala @@ -52,13 +52,13 @@ object ChildInserter { .filter(_.ref == ctx.sentinelNode.ref.nextSibling) // Assert that the prev child node was not moved. Note: nextSibling could be null .fold { // Anything that does exist in the DOM, is not ours. Clean it up first. - // It's possible that previous node(s) already contained newChildNode, - // so make sure to avoid unmounting it. + // The previous content may already contain newChildNode (even nested inside + // one of its dynamic items) – if so, it's left in place, not unmounted. ctx.clearPreviousInserterContent( replaceContentMapWithSingleNode = newChildNode, nextInserterType = InserterType.ChildType ) - // Render the new child + // Render the new child (or move it into place, if it was retained above) DomApi.insertChildAfter( parent = ctx.currentParentNode, newChild = newChildNode, diff --git a/src/main/scala/com/raquo/laminar/inserters/ChildrenCommandInserter.scala b/src/main/scala/com/raquo/laminar/inserters/ChildrenCommandInserter.scala index 20f0d79c..4217c7cf 100644 --- a/src/main/scala/com/raquo/laminar/inserters/ChildrenCommandInserter.scala +++ b/src/main/scala/com/raquo/laminar/inserters/ChildrenCommandInserter.scala @@ -1,6 +1,7 @@ package com.raquo.laminar.inserters import com.raquo.airstream.core.EventStream +import com.raquo.ew.JsSet import com.raquo.laminar.domapi.DomApi import com.raquo.laminar.modifiers.RenderableNode import com.raquo.laminar.nodes.{ChildNode, CommentNode} @@ -158,25 +159,71 @@ object ChildrenCommandInserter { } case CollectionCommand.RemoveAll => - ctx.removeContentMapNodesFromDom(keepNodeIfPresent = js.undefined) + ctx.removeContentMapNodesFromDom(keepItem = InsertContext.keepNoItems) ctx.contentMap.clear() - case CollectionCommand.ReplaceAll(newNodes) => - ctx.removeContentMapNodesFromDom(keepNodeIfPresent = js.undefined) - ctx.contentMap.clear() - val trailingSentinelRef = ctx.trailingSentinelNodeOpt.get.ref - newNodes.foreach { node => - if ( - DomApi.insertChildBefore( - parent = ctx.currentParentNode, - newChild = node, - referenceChildRef = trailingSentinelRef, - slotName = ctx.currentSlotName - ) - ) { - ctx.contentMap.set(node.ref, node) - } + case CollectionCommand.ReplaceAll(newNodes, minimizeDiff) => + if (minimizeDiff) { + replaceAllMinimizingDiff(newNodes, ctx) + } else { + replaceAllNaively(newNodes, ctx) } } } + + private def replaceAllNaively( + newNodes: collection.immutable.Seq[ChildNode.Base], + ctx: InsertContext + ): Unit = { + ctx.removeContentMapNodesFromDom(keepItem = InsertContext.keepNoItems) + ctx.contentMap.clear() + val trailingSentinelRef = ctx.trailingSentinelNodeOpt.get.ref + newNodes.foreach { node => + if ( + DomApi.insertChildBefore( + parent = ctx.currentParentNode, + newChild = node, + referenceChildRef = trailingSentinelRef, + slotName = ctx.currentSlotName + ) + ) { + ctx.contentMap.set(node.ref, node) + } + } + } + + /** Keep the nodes that are staying in the list, so that they aren't re-mounted. + * The old nodes that are leaving are removed (and unmounted) before the new ones mount. + */ + private def replaceAllMinimizingDiff( + newNodes: collection.immutable.Seq[ChildNode.Base], + ctx: InsertContext + ): Unit = { + val newNodeRefs = JsSet.empty[dom.Node] + newNodes.foreach(node => newNodeRefs.add(node.ref)) + ctx.removeContentMapNodesFromDom(keepItem = newNodeRefs.has) + ctx.contentMap.clear() + // Place the nodes in order. Kept nodes that are already in the right place stay put. + var afterRef: dom.Node = ctx.sentinelNode.ref + newNodes.foreach { node => + val isPlaced = if (afterRef.nextSibling eq node.ref) { + // Re-affirm the slot, just like inserting the node would, so that this reconcile + // wins over a manual `slot` override, whether or not the node had to move + // (consistent last-write-wins semantics). + node.applySlot(ctx.currentParentNode, ctx.currentSlotName) + true + } else { + DomApi.insertChildAfter( + parent = ctx.currentParentNode, + newChild = node, + referenceChildRef = afterRef, + slotName = ctx.currentSlotName + ) + } + if (isPlaced) { + ctx.contentMap.set(node.ref, node) + afterRef = node.ref + } + } + } } diff --git a/src/main/scala/com/raquo/laminar/inserters/ChildrenInserter.scala b/src/main/scala/com/raquo/laminar/inserters/ChildrenInserter.scala index da1e297d..eb375811 100644 --- a/src/main/scala/com/raquo/laminar/inserters/ChildrenInserter.scala +++ b/src/main/scala/com/raquo/laminar/inserters/ChildrenInserter.scala @@ -100,8 +100,14 @@ object ChildrenInserter { prevItemRef = prevItemRef.nextSibling }) { prevInserter => val nextPrevItemRef = prevInserter.lastNode.nextSibling + // Keep the items nested inside prevInserter that the new list re-uses. They're left + // just before the cursor, and are placed on their turn (prevInserter itself is never + // in nextInsertersMap here). // @Note: DOM update - prevInserter.removeFromDynamicList(listParentNode) + prevInserter.removeFromDynamicList( + parent = listParentNode, + keepNestedItem = nextInsertersMap.has + ) prevItemRef = nextPrevItemRef currentItemCount -= 1 } diff --git a/src/main/scala/com/raquo/laminar/inserters/CollectionCommand.scala b/src/main/scala/com/raquo/laminar/inserters/CollectionCommand.scala index 2ce13a12..9fd23b03 100644 --- a/src/main/scala/com/raquo/laminar/inserters/CollectionCommand.scala +++ b/src/main/scala/com/raquo/laminar/inserters/CollectionCommand.scala @@ -52,11 +52,27 @@ object CollectionCommand { @inline override def map[A](project: Nothing => A): CollectionCommand[A] = this } - /** Replace the entire contents of the collection with `newItems` */ - case class ReplaceAll[+Item](newItems: collection.immutable.Seq[Item]) extends CollectionCommand[Item] { + /** Replace the entire contents of the collection with `newItems`. + * + * @param minimizeDiff + * - `false`: all DOM nodes managed by this `children.command <--` are removed from + * the DOM first, then all the `newItems` are added. This is the fastest option + * when you know that `newItems` does not contain existing nodes (e.g. switching + * to an unrelated list). + * - `true`: `children.command` will run a smarter diffing algorithm that will avoid + * unmounting existing nodes if they are present in `newItems`, preventing a + * re-mount. This is more similar to what `children <--` does (but more basic), + * and involves more overhead, especially for very large lists, but can be + * desirable if you don't want mount/unmount hooks to run unnecessarily in + * this transition. + */ + case class ReplaceAll[+Item]( + newItems: collection.immutable.Seq[Item], + minimizeDiff: Boolean + ) extends CollectionCommand[Item] { @inline override def map[A](project: Item => A): ReplaceAll[A] = { - ReplaceAll(newItems.map(project)) + ReplaceAll(newItems.map(project), minimizeDiff) } } @@ -105,7 +121,7 @@ object CollectionCommand { case RemoveAll => Vector.empty - case ReplaceAll(newItems) => + case ReplaceAll(newItems, _) => newItems.toVector } } diff --git a/src/main/scala/com/raquo/laminar/inserters/InsertContext.scala b/src/main/scala/com/raquo/laminar/inserters/InsertContext.scala index 71c81f23..2b1ef7a4 100644 --- a/src/main/scala/com/raquo/laminar/inserters/InsertContext.scala +++ b/src/main/scala/com/raquo/laminar/inserters/InsertContext.scala @@ -182,14 +182,21 @@ final class InsertContext( * * @param replaceContentMapWithSingleNode * If specified, will ensure that the resulting contentMap has this node. - * If specified, will prevent this node from being removed from the DOM, but will NOT add it to the DOM. + * If specified, will prevent this node from being removed from the DOM + * (even if it's nested in a dynamic item), but will NOT add it to the DOM. + * @param keepItem Items to leave in the DOM – see [[removeContentMapNodesFromDom]]. */ def clearPreviousInserterContent( replaceContentMapWithSingleNode: js.UndefOr[ChildNode.Base], - nextInserterType: js.UndefOr[InserterType] + nextInserterType: js.UndefOr[InserterType], + keepItem: dom.Node => Boolean = InsertContext.keepNoItems ): Unit = { // Remove from the DOM any old nodes that shouldn't be retained. - removeContentMapNodesFromDom(keepNodeIfPresent = replaceContentMapWithSingleNode) + removeContentMapNodesFromDom( + keepItem = replaceContentMapWithSingleNode.fold(keepItem) { singleNode => + ref => (ref eq singleNode.ref) || keepItem(ref) + } + ) // Update the context to match the DOM state contentMap.clear() @@ -213,7 +220,20 @@ final class InsertContext( } /** Walk this context's span forward from [[sentinelNode]], removing every tracked - * ([[contentMap]]) node from the DOM except `keepNodeIfPresent`. + * ([[contentMap]]) item from the DOM except those matching `keepItem`. + * + * @param keepItem Called with each item's `stableFirstNode`, the item's identity (the same + * key as in [[contentMap]]): the node itself for a plain or slotted node, + * or the leading sentinel for a dynamic inserter. + * + * `keepItem` is also applied recursively inside the removed dynamic items: a kept item is + * left in place under the same parent (a dynamic inserter with its whole span and live + * subscription), even if the nested group that contained it is torn down. It never loses + * its parent, so it's not unmounted. This lets the caller re-insert it seamlessly. + * + * #Warning: The caller must promptly place every kept item into the DOM where it belongs, + * and track it there. A kept item is left where it was, untracked by this context – and + * if it came from inside a removed nested group, it's untracked by anyone. * * Where the walk stops depends on whether we have a [[_trailingSentinelNodeOpt]]: * - With one (a `children <--` or `children.command <--` span), it marks the definite @@ -231,7 +251,7 @@ final class InsertContext( * #Note: this does NOT update [[contentMap]] to match the new DOM. */ def removeContentMapNodesFromDom( - keepNodeIfPresent: js.UndefOr[ChildNode.Base] + keepItem: dom.Node => Boolean ): Unit = { val hasTrailingSentinel = _trailingSentinelNodeOpt.nonEmpty var maybeRef = sentinelNode.ref.nextSibling @@ -241,10 +261,10 @@ final class InsertContext( if (_trailingSentinelNodeOpt.exists(_.ref == childRef)) { // Reached the end of our span. Stop. continue = false - } else if (keepNodeIfPresent.exists(_.ref == childRef)) { - // The node we're keeping. Leave it in place, step over it (a plain content node, so - // a single node) and keep clearing whatever old content sits after it. - maybeRef = childRef.nextSibling + } else if (keepItem(childRef)) { + // An item we're keeping. Leave it in place, step over it (in one go, if it's a nested + // inserter's span), and keep clearing whatever old content sits after it. + maybeRef = contentMap.get(childRef).fold(childRef.nextSibling)(_.lastNode.nextSibling) } else { contentMap.get(childRef).fold { if (hasTrailingSentinel) { @@ -265,7 +285,7 @@ final class InsertContext( // so a multi-node span (e.g. a nested group) is stepped over in one go. val nextRef = inserter.lastNode.nextSibling // @Note: DOM update - inserter.removeFromDynamicList(currentParentNode) + inserter.removeFromDynamicList(currentParentNode, keepItem) maybeRef = nextRef } } @@ -324,6 +344,8 @@ final class InsertContext( object InsertContext { + private[laminar] val keepNoItems: dom.Node => Boolean = _ => false + /** Reserve the spot for when we actually insert real nodes later */ def reserveSpotContext( parentNode: ReactiveElement.Base, diff --git a/src/main/scala/com/raquo/laminar/inserters/Inserter.scala b/src/main/scala/com/raquo/laminar/inserters/Inserter.scala index a36d8d61..88892b49 100644 --- a/src/main/scala/com/raquo/laminar/inserters/Inserter.scala +++ b/src/main/scala/com/raquo/laminar/inserters/Inserter.scala @@ -69,8 +69,16 @@ trait DiffableInserter extends Inserter { * * Pre-requisite: you must have called [[addToDynamicList]] * with the same parent before calling this. + * + * @param keepNestedItem Items nested INSIDE this inserter that must be left in place + * rather than removed – see [[InsertContext.removeContentMapNodesFromDom]]. + * #Warning: The caller is responsible for checking whether THIS + * inserter's own `stableFirstNode` against `keepItem`. */ - private[laminar] def removeFromDynamicList(parent: ReactiveElement.Base): Unit + private[laminar] def removeFromDynamicList( + parent: ReactiveElement.Base, + keepNestedItem: dom.Node => Boolean + ): Unit /** No mount / re-mount, just a lateral move. * @@ -290,13 +298,16 @@ class DynamicInserter( } } - override private[laminar] def removeFromDynamicList(parent: ReactiveElement.Base): Unit = { + override private[laminar] def removeFromDynamicList( + parent: ReactiveElement.Base, + keepNestedItem: dom.Node => Boolean + ): Unit = { val group = nestedGroupOpt.getOrElse( throw new Exception("Can not removeFromDynamicList: nested group not found (addToDynamicList was not called first). This is a bug in Laminar.") ) if (group.leadingSentinel.ref.parentNode == parent.ref) { // This list still hosts the group's span – a genuine removal – #Note: probably – see below - group.removeFromParent() + group.removeFromParent(keepNestedItem) nestedGroupOpt = js.undefined } else { // The group was already moved to a different parent, stolen by its new host, so there is diff --git a/src/main/scala/com/raquo/laminar/inserters/NestedGroup.scala b/src/main/scala/com/raquo/laminar/inserters/NestedGroup.scala index 0607a3cf..064a4a78 100644 --- a/src/main/scala/com/raquo/laminar/inserters/NestedGroup.scala +++ b/src/main/scala/com/raquo/laminar/inserters/NestedGroup.scala @@ -170,7 +170,10 @@ final class NestedGroup( } } - private[laminar] def removeFromParent(): Unit = { + /** @param keepItem Content items to leave in the parent's DOM – see + * [[InsertContext.removeContentMapNodesFromDom]]. + */ + private[laminar] def removeFromParent(keepItem: dom.Node => Boolean): Unit = { // #Note: order of operations mirrors that in willSetParent(None) of ReactiveElement // - first, disown the subscription, then, update the DOM. @@ -182,9 +185,11 @@ final class NestedGroup( // a `children <--` list, a `children.command` span, or a plain `child <--` / `text <--` // node), tearing down any per-item lifecycle. The context knows its own content type, // so this one call picks the right teardown strategy. + // Kept items are left in the parent, exactly where this group's span was. nestedInsertContext.clearPreviousInserterContent( replaceContentMapWithSingleNode = js.undefined, - nextInserterType = js.undefined + nextInserterType = js.undefined, + keepItem = keepItem ) // A. Remove the sentinel nodes from the parent DOM. Use the CURRENT parent diff --git a/src/main/scala/com/raquo/laminar/inserters/SlottableChildInserter.scala b/src/main/scala/com/raquo/laminar/inserters/SlottableChildInserter.scala index b345c25c..90fc05e4 100644 --- a/src/main/scala/com/raquo/laminar/inserters/SlottableChildInserter.scala +++ b/src/main/scala/com/raquo/laminar/inserters/SlottableChildInserter.scala @@ -78,7 +78,10 @@ with Slottable[SlottableChildInserter] { ) } - override private[laminar] def removeFromDynamicList(parent: ReactiveElement.Base): Unit = { + override private[laminar] def removeFromDynamicList( + parent: ReactiveElement.Base, + keepNestedItem: dom.Node => Boolean // Unused – no nested items to keep + ): Unit = { DomApi.removeChild(parent = parent, child = child) } diff --git a/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala b/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala index b717ec80..5d0b2d39 100644 --- a/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala +++ b/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala @@ -66,7 +66,8 @@ with DiffableInserter { } override private[laminar] def removeFromDynamicList( - parent: ReactiveElement.Base + parent: ReactiveElement.Base, + keepNestedItem: dom.Node => Boolean // Unused – no nested items to keep ): Unit = { DomApi.removeChild(parent = parent, child = this) } diff --git a/src/test/scala/com/raquo/laminar/tests/ChildrenCommandReceiverSpec.scala b/src/test/scala/com/raquo/laminar/tests/ChildrenCommandReceiverSpec.scala index b6f45758..5e7e6071 100644 --- a/src/test/scala/com/raquo/laminar/tests/ChildrenCommandReceiverSpec.scala +++ b/src/test/scala/com/raquo/laminar/tests/ChildrenCommandReceiverSpec.scala @@ -3,10 +3,11 @@ package com.raquo.laminar.tests import com.raquo.domtestutils.matching.Rule import com.raquo.laminar.api.L._ import com.raquo.laminar.domapi.{DomApi, DomError} -import com.raquo.laminar.inserters.CollectionCommand.{Append, Insert, Prepend, Remove, RemoveAll, Replace, ReplaceAll} -import com.raquo.laminar.inserters.{CollectionCommand, DynamicInserter, InsertContext, Inserter} import com.raquo.laminar.fixtures.TestableOwner +import com.raquo.laminar.inserters.{CollectionCommand, DynamicInserter, InsertContext, Inserter} +import com.raquo.laminar.inserters.CollectionCommand.{Append, Insert, Prepend, Remove, RemoveAll, Replace, ReplaceAll} import com.raquo.laminar.utils.UnitSpec +import org.scalajs.dom import scala.collection.immutable @@ -73,66 +74,285 @@ class ChildrenCommandReceiverSpec extends UnitSpec { } } - it("RemoveAll and ReplaceAll") { - val commandBus = new EventBus[CollectionCommand[Node]] + List(true, false).foreach { minimizeDiff => - val span0 = span(text0) - val span1 = span(text1) - val div2 = div(text2) - val div3 = div(text3) - val span4 = span(text4) + it(s"RemoveAll and ReplaceAll (minimizeDiff = $minimizeDiff)") { + val commandBus = new EventBus[CollectionCommand[Node]] + + val span0 = span(text0) + val span1 = span(text1) + val div2 = div(text2) + val div3 = div(text3) + val span4 = span(text4) + + val el = div( + "Hello", + children.command <-- commandBus.events, + div("World") + ) + + mount(el) + expectChildren("initial:") + + commandBus.writer.onNext(Append(span0)) + commandBus.writer.onNext(Append(span1)) + commandBus.writer.onNext(Prepend(div2)) + expectChildren( + "built up:", + div of text2, + span of text0, + span of text1 + ) + + // RemoveAll clears all tracked content, but keeps the sentinels in place. + commandBus.writer.onNext(RemoveAll) + expectChildren( + "after RemoveAll:", + // no children + ) + // Commands keep working after a full clear. + commandBus.writer.onNext(Append(span4)) + expectChildren( + "append after RemoveAll:", + span of text4 + ) + + // ReplaceAll swaps the entire contents, in order. + commandBus.writer.onNext(ReplaceAll(div2 :: div3 :: Nil, minimizeDiff)) + expectChildren( + "after ReplaceAll:", + div of text2, + div of text3 + ) + + // ReplaceAll with an empty seq is equivalent to RemoveAll. + commandBus.writer.onNext(ReplaceAll(Nil, minimizeDiff)) + expectChildren( + "after empty ReplaceAll:", + // no children + ) + + // Still functional after an empty ReplaceAll. + commandBus.writer.onNext(Append(span0)) + expectChildren( + "append after empty ReplaceAll:", + span of text0 + ) + + commandBus.writer.onNext(Append(span1)) + commandBus.writer.onNext(Append(div2)) + expectChildren( + "built up again:", + span of text0, + span of text1, + div of text2 + ) + + // ReplaceAll may include nodes that are currently rendered (span0, span1 here): they are + // placed in the new order next to the new nodes (whether they're re-mounted depends on + // `minimizeDiff` – see the ReplaceAll lifecycle tests below). So this keeps span1 & span0 + // (reordered), drops div2, and adds a fresh div3. + commandBus.writer.onNext(ReplaceAll(span1 :: div3 :: span0 :: Nil, minimizeDiff)) + expectChildren( + "after ReplaceAll reusing rendered nodes:", + span of text1, + div of text3, + span of text0 + ) + + def expectChildren(clue: String, childRules: Rule*): Unit = { + withClue(clue) { + val first: Rule = "Hello" + val last: Rule = div of "World" + val rules: immutable.Seq[Rule] = first +: (sentinel: Rule) +: childRules :+ (sentinel: Rule) :+ last + expectNode(div.of(rules: _*)) + } + } + } + } + + it("ReplaceAll(minimizeDiff = true) keeps the nodes that stay mounted, and unmounts the leaving nodes before mounting new ones") { + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val x = tracker.createDiv("x") + val y = tracker.createDiv("y") + tracker.clear() + + val commandBus = new EventBus[CollectionCommand[Node]] val el = div( "Hello", children.command <-- commandBus.events, div("World") ) + withClue("initial:") { + mount(el) + commandBus.emit(Append(a)) + commandBus.emit(Append(b)) + commandBus.emit(Append(c)) + expectNode(div.of("Hello", sentinel, div of "a", div of "b", div of "c", sentinel, div of "World")) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("b"), + _.mounted("c") + ) + .clear() + } + + withClue("ReplaceAll(c, x, a): b unmounts, then x mounts; a and c are kept and reordered:") { + commandBus.emit(ReplaceAll(c :: x :: a :: Nil, minimizeDiff = true)) + expectNode(div.of("Hello", sentinel, div of "c", div of "x", div of "a", sentinel, div of "World")) + tracker + .assertEvents( + _.unmounted("b"), + _.mounted("x") + ) + .clear() + } + + withClue("the kept nodes are tracked as usual: Remove works on them:") { + commandBus.emit(Remove(a)) + expectNode(div.of("Hello", sentinel, div of "c", div of "x", sentinel, div of "World")) + tracker + .assertEvents( + _.unmounted("a") + ) + .clear() + } + + withClue("Append still lands at the end of the span:") { + commandBus.emit(Append(y)) + expectNode(div.of("Hello", sentinel, div of "c", div of "x", div of "y", sentinel, div of "World")) + tracker + .assertEvents( + _.mounted("y") + ) + .clear() + } + + withClue("RemoveAll reaches both kept and new nodes:") { + commandBus.emit(RemoveAll) + expectNode(div.of("Hello", sentinel, sentinel, div of "World")) + tracker + .assertEvents( + _.unmounted("c"), + _.unmounted("x"), + _.unmounted("y") + ) + .clear() + } + } + + it("ReplaceAll(minimizeDiff = true) does not move the kept nodes that are already in the right place") { + // Moving a DOM node has side effects even without re-mounting (e.g. it loses focus, and + // resets iframes), so the nodes that don't need to move are left untouched. + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val x = tracker.createDiv("x") + tracker.clear() + + val commandBus = new EventBus[CollectionCommand[Node]] + val el = div(children.command <-- commandBus.events) + mount(el) - expectChildren("initial:") + commandBus.emit(Append(a)) + commandBus.emit(Append(b)) + commandBus.emit(Append(c)) + tracker.clear() - commandBus.writer.onNext(Append(span0)) - commandBus.writer.onNext(Append(span1)) - commandBus.writer.onNext(Prepend(div2)) - expectChildren("built up:", div of text2, span of text0, span of text1) + // Records every DOM insertion / removal under `el`, including moves (a removal + an insertion). + val observer = new dom.MutationObserver((_, _) => ()) + observer.observe(el.ref, new dom.MutationObserverInit { childList = true }) + def takeMutatedNodes(): List[String] = { + observer.takeRecords().toList.flatMap { record => + record.removedNodes.toList.map(n => s"removed:${n.textContent}") ++ + record.addedNodes.toList.map(n => s"added:${n.textContent}") + } + } - // RemoveAll clears all tracked content, but keeps the sentinels in place. - commandBus.writer.onNext(RemoveAll) - expectChildren("after RemoveAll:") + withClue("ReplaceAll with the same nodes in the same order: no DOM changes at all:") { + commandBus.emit(ReplaceAll(a :: b :: c :: Nil, minimizeDiff = true)) + expectNode(div.of(sentinel, div of "a", div of "b", div of "c", sentinel)) + assert(takeMutatedNodes() == Nil) + tracker.assertNoEvents.clear() + } - // Commands keep working after a full clear. - commandBus.writer.onNext(Append(span4)) - expectChildren("append after RemoveAll:", span of text4) + withClue("ReplaceAll(a, x, c): b is removed and x is added, but a and c are not moved:") { + commandBus.emit(ReplaceAll(a :: x :: c :: Nil, minimizeDiff = true)) + expectNode(div.of(sentinel, div of "a", div of "x", div of "c", sentinel)) + assert(takeMutatedNodes() == List("removed:b", "added:x")) + tracker + .assertEvents( + _.unmounted("b"), + _.mounted("x") + ) + .clear() + } - // ReplaceAll swaps the entire contents, in order. - commandBus.writer.onNext(ReplaceAll(div2 :: div3 :: Nil)) - expectChildren("after ReplaceAll:", div of text2, div of text3) + observer.disconnect() + } - // ReplaceAll with an empty seq is equivalent to RemoveAll. - commandBus.writer.onNext(ReplaceAll(Nil)) - expectChildren("after empty ReplaceAll:") + it("ReplaceAll(minimizeDiff = false) removes all current nodes, then inserts the new ones, re-mounting any overlap") { + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val x = tracker.createDiv("x") + tracker.clear() - // Still functional after an empty ReplaceAll. - commandBus.writer.onNext(Append(span0)) - expectChildren("append after empty ReplaceAll:", span of text0) + val commandBus = new EventBus[CollectionCommand[Node]] + val el = div( + "Hello", + children.command <-- commandBus.events, + div("World") + ) - commandBus.writer.onNext(Append(span1)) - commandBus.writer.onNext(Append(div2)) - expectChildren("built up again:", span of text0, span of text1, div of text2) + withClue("initial:") { + mount(el) + commandBus.emit(Append(a)) + commandBus.emit(Append(b)) + commandBus.emit(Append(c)) + expectNode(div.of("Hello", sentinel, div of "a", div of "b", div of "c", sentinel, div of "World")) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("b"), + _.mounted("c") + ) + .clear() + } - // ReplaceAll may include nodes that are currently rendered (span0, span1 here): they get - // torn down along with everything else, then re-inserted in the new order next to the new - // nodes. So this keeps span1 & span0 (reordered), drops div2, and adds a fresh div3. - commandBus.writer.onNext(ReplaceAll(span1 :: div3 :: span0 :: Nil)) - expectChildren("after ReplaceAll reusing rendered nodes:", span of text1, div of text3, span of text0) + withClue("ReplaceAll(c, x, a): everything unmounts first, then the new list mounts in order:") { + commandBus.emit(ReplaceAll(c :: x :: a :: Nil, minimizeDiff = false)) + expectNode(div.of("Hello", sentinel, div of "c", div of "x", div of "a", sentinel, div of "World")) + tracker + .assertEvents( + _.unmounted("a"), + _.unmounted("b"), + _.unmounted("c"), + _.mounted("c"), + _.mounted("x"), + _.mounted("a") + ) + .clear() + } - def expectChildren(clue: String, childRules: Rule*): Unit = { - withClue(clue) { - val first: Rule = "Hello" - val last: Rule = div of "World" - val rules: immutable.Seq[Rule] = first +: (sentinel: Rule) +: childRules :+ (sentinel: Rule) :+ last - expectNode(div.of(rules: _*)) - } + withClue("the new nodes are tracked as usual: RemoveAll reaches them:") { + commandBus.emit(RemoveAll) + expectNode(div.of("Hello", sentinel, sentinel, div of "World")) + tracker + .assertEvents( + _.unmounted("c"), + _.unmounted("x"), + _.unmounted("a") + ) + .clear() } } diff --git a/src/test/scala/com/raquo/laminar/tests/InserterInvariantSpec.scala b/src/test/scala/com/raquo/laminar/tests/InserterInvariantSpec.scala index 3470f934..e5b1af52 100644 --- a/src/test/scala/com/raquo/laminar/tests/InserterInvariantSpec.scala +++ b/src/test/scala/com/raquo/laminar/tests/InserterInvariantSpec.scala @@ -127,7 +127,7 @@ class InserterInvariantSpec extends UnitSpec { // No `addToDynamicList` was ever called on `inserter`, so it has no NestedGroup. val thrown = intercept[Exception] { - inserter.removeFromDynamicList(parent) + inserter.removeFromDynamicList(parent, InsertContext.keepNoItems) } assert(thrown.getMessage.contains("nested group not found")) } diff --git a/src/test/scala/com/raquo/laminar/tests/InserterMoveSpec.scala b/src/test/scala/com/raquo/laminar/tests/InserterMoveSpec.scala index 07eaaec1..f2a3b7f7 100644 --- a/src/test/scala/com/raquo/laminar/tests/InserterMoveSpec.scala +++ b/src/test/scala/com/raquo/laminar/tests/InserterMoveSpec.scala @@ -23,6 +23,7 @@ import com.raquo.laminar.utils.UnitSpec * 4c. A moved span relocates exactly its LIVE DOM span (DOM order + departed nodes left behind). * 4d. Re-placing a torn-down group: with no live span to move, rebuild + re-mount, like re-adding * a removed element. + * 4e. Un-nesting: a leaving nested item releases the nodes / inserters the list keeps. * 5. Promote / demote across static application and a list (static <-> list, static <-> static). * 6. The inserter-TYPE matrix: `children.command <--` and `text <--` as moved items. * 7. The degenerate same-transaction double-add. @@ -2322,6 +2323,257 @@ class InserterMoveSpec extends UnitSpec { } } + // ---------------------------------------------------------------------------------- + // 4e. A leaving nested item releases the nodes that the same emission keeps (no re-mount) + // ---------------------------------------------------------------------------------- + + // When a list re-emits WITHOUT a nested dynamic item, but WITH a node (or a nested inserter) + // that currently lives inside that item, the node is un-nested seamlessly: the leaving item is + // torn down around it, and the node is re-parented into the list – like moving a plain element + // out of a wrapper that's being removed, within the same parent. This must hold regardless of + // where the kept node lands relative to the leaving item, and at any nesting depth. + + List( + ( + "after", // position + (a: Div, b: Div) => List(a, b), // nextItems + List[Rule](sentinel, div of "a", div of "b", sentinel) // expectedDom + ), + ( + "before", // position + (a: Div, b: Div) => List(b, a), // nextItems + List[Rule](sentinel, div of "b", div of "a", sentinel) // expectedDom + ) + ).foreach { case (position, nextItems, expectedDom) => + + it(s"un-nesting: (nested `child <-- b`, a) -> b $position a keeps b mounted, and the nested inserter is dead") { + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + tracker.clear() + + val nestedVar = Var(b) + val items = Var[List[Inserter]](List(child <-- nestedVar.signal, a)) + + withClue("initial:") { + mount(div(children <-- items.signal)) + expectNode(div.of(sentinel, sentinel, div of "b", sentinel, div of "a", sentinel)) + tracker + .assertEvents( + _.mounted("b"), + _.mounted("a") + ) + .clear() + } + + withClue(s"the list drops the nested item, and places b directly, $position a:") { + items.set(nextItems(a, b)) + expectNode(div.of(expectedDom: _*)) + tracker.assertNoEvents.clear() + } + + withClue("the old nested `child <--` was torn down, so it no longer renders anything:") { + nestedVar.set(c) + expectNode(div.of(expectedDom: _*)) + tracker.assertNoEvents.clear() + } + } + } + + it("un-nesting at depth 2: (nested `children <--` (c, nested `child <-- b`), a) -> (a, b) keeps b, unmounts c") { + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + tracker.clear() + + val items = Var[List[Inserter]](List(children <-- Val(List[Inserter](c, child <-- Val(b))), a)) + + withClue("initial:") { + mount(div(children <-- items.signal)) + expectNode( + div.of( + sentinel, // outer list + sentinel, // nested list + div of "c", + sentinel, div of "b", sentinel, // nested child + sentinel, // nested list trailing + div of "a", + sentinel // outer list trailing + ) + ) + tracker + .assertEvents( + _.mounted("c"), + _.mounted("b"), + _.mounted("a") + ) + .clear() + } + + withClue("both nested items are torn down around b, which stays mounted:") { + items.set(List(a, b)) + expectNode(div.of(sentinel, div of "a", div of "b", sentinel)) + tracker + .assertEvents( + _.unmounted("c") + ) + .clear() + } + } + + it("un-nesting a dynamic inserter: (nested `children <--` (c, nested `child <--` I), a) -> (a, I) keeps I live") { + // The kept thing can be a nested inserter too (matched by its identity, not its content). + // Its whole span is released from the leaving item, and it keeps rendering in its new place. + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val d = tracker.createDiv("d") + tracker.clear() + + val innerVar = Var(b) + val inner: Inserter = child <-- innerVar.signal + val items = Var[List[Inserter]](List(children <-- Val(List[Inserter](c, inner)), a)) + + withClue("initial:") { + mount(div(children <-- items.signal)) + expectNode( + div.of( + sentinel, // outer list + sentinel, // nested list + div of "c", + sentinel, div of "b", sentinel, // I + sentinel, // nested list trailing + div of "a", + sentinel // outer list trailing + ) + ) + tracker + .assertEvents( + _.mounted("c"), + _.mounted("b"), + _.mounted("a") + ) + .clear() + } + + withClue("the nested list is torn down around I, which moves after a without re-mounting:") { + items.set(List(a, inner)) + expectNode(div.of(sentinel, div of "a", sentinel, div of "b", sentinel, sentinel)) + tracker + .assertEvents( + _.unmounted("c") + ) + .clear() + } + + withClue("I is still live in its new place:") { + innerVar.set(d) + expectNode(div.of(sentinel, div of "a", sentinel, div of "d", sentinel, sentinel)) + tracker + .assertEvents( + _.unmounted("b"), + _.mounted("d") + ) + .clear() + } + } + + it("un-nesting several nodes out of several leaving items, reordered, unmounts only the dropped node") { + // c lands BEFORE its leaving item (a steal at the cursor), while b and d land AFTER a, so + // the list removes both leaving items before placing them (the early-removal path). + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val d = tracker.createDiv("d") + val e = tracker.createDiv("e") + tracker.clear() + + val items = Var[List[Inserter]](List( + children <-- Val(List(b, c, e)), + child <-- Val(d), + a + )) + + withClue("initial:") { + mount(div(children <-- items.signal)) + expectNode( + div.of( + sentinel, // outer list + sentinel, div of "b", div of "c", div of "e", sentinel, // nested list + sentinel, div of "d", sentinel, // nested child + div of "a", + sentinel // outer list trailing + ) + ) + tracker + .assertEvents( + _.mounted("b"), + _.mounted("c"), + _.mounted("e"), + _.mounted("d"), + _.mounted("a") + ) + .clear() + } + + withClue("(c, a, d, b): only e, which is dropped, unmounts:") { + items.set(List(c, a, d, b)) + expectNode(div.of(sentinel, div of "c", div of "a", div of "d", div of "b", sentinel)) + tracker + .assertEvents( + _.unmounted("e") + ) + .clear() + } + } + + // Similar to https://github.com/raquo/Laminar/issues/163 + it("CHARACTERIZATION: re-wrapping a nested node into a NEW nested inserter re-mounts it when the old item leaves first") { + // (nested `child <-- b`, a) -> (a, NEW nested `child <-- b`): the list removes the leaving + // item before it reaches the new one, and at that point nothing is known about the new + // inserter's content – it's only rendered once the new item subscribes. So b is unmounted + // with the leaving item, then mounted again by the new one. + // #Note: known limitation – see "Known deviations" in notes/Inserters.md. + val tracker = createEventTracker() + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + tracker.clear() + + val items = Var[List[Inserter]](List(child <-- Val(b), a)) + + withClue("initial:") { + mount(div(children <-- items.signal)) + expectNode(div.of(sentinel, sentinel, div of "b", sentinel, div of "a", sentinel)) + tracker + .assertEvents( + _.mounted("b"), + _.mounted("a") + ) + .clear() + } + + withClue("b is re-mounted:") { + items.set(List(a, child <-- Val(b))) + expectNode(div.of(sentinel, div of "a", sentinel, div of "b", sentinel, sentinel)) + tracker + .assertEvents( + _.unmounted("b"), + _.mounted("b") + ) + .clear() + } + + withClue("reference: when the new inserter comes FIRST, it takes b before the old item leaves:") { + items.set(List(child <-- Val(b), a)) + expectNode(div.of(sentinel, sentinel, div of "b", sentinel, div of "a", sentinel)) + tracker.assertNoEvents.clear() + } + } + // ---------------------------------------------------------------------------------- // 5. Promote / demote: static application <-> list, static <-> static // ---------------------------------------------------------------------------------- diff --git a/src/test/scala/com/raquo/laminar/tests/InserterTakeoverSpec.scala b/src/test/scala/com/raquo/laminar/tests/InserterTakeoverSpec.scala index cb6e818b..ff544e84 100644 --- a/src/test/scala/com/raquo/laminar/tests/InserterTakeoverSpec.scala +++ b/src/test/scala/com/raquo/laminar/tests/InserterTakeoverSpec.scala @@ -109,7 +109,9 @@ class InserterTakeoverSpec extends UnitSpec { // `children <--` and emits a node that's ALREADY in that span, only the OTHER items unmount. // The surviving node is retained in place – neither unmounted nor re-mounted – as its // ownership passes from the list to the `child <--`. This is the counterpart to the teardown - // tests above, and pins the `clearPreviousInserterContent(keep = ...)` branch of `switchToChild`. -- + // tests above, and pins the `clearPreviousInserterContent(keep = ...)` branch of `switchToChild`. + // The same holds when the surviving node is nested inside one of the list's dynamic items, + // at any depth: the nested item is torn down around it. -- it("onMountInsert: `children <--` (a, b) -> `child <-- b` keeps b mounted, unmounts only a") { val tracker = createEventTracker() @@ -163,6 +165,289 @@ class InserterTakeoverSpec extends UnitSpec { } } + it("onMountInsert: `children <--` (a, nested `child <-- b`) -> `child <-- b` keeps b mounted, unmounts only a") { + // Same as the test above, except that the list renders b through a nested `child <--` item. + // Nesting must not change the outcome: b is already in the context's span, so the takeover + // retains it rather than tearing down the nested item and then re-inserting b. + val tracker = createEventTracker() + val childrenBus = new EventBus[List[Inserter]] + val childBus = new EventBus[Div] + + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + tracker.clear() + + var dynamicInserter: Inserter = children <-- childrenBus.events + val takeoverInserter: Inserter = child <-- childBus.events + + val el = div("Hello ", onMountInsert(_ => dynamicInserter), " world") + + withClue("initial: children <-- renders a, and b through a nested `child <--`:") { + mount(el) + childrenBus.emit(List(a, child <-- Val(b))) + expectNode(div of ("Hello ", sentinel, div of "a", sentinel, div of "b", sentinel, sentinel, " world")) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("b") + ) + .clear() + } + + withClue("unmount then remount rides a and b on the element's own lifecycle:") { + unmount() + dynamicInserter = takeoverInserter + mount(el) + tracker + .assertEvents( + _.unmounted("a"), + _.unmounted("b"), + _.mounted("a"), + _.mounted("b") + ) + .clear() + } + + withClue("`child <-- b` takes over: b (already present) stays put, only a is torn down:") { + childBus.emit(b) + expectNode(div of ("Hello ", sentinel, div of "b", " world")) + // As in the un-nested test above: b is RETAINED, not re-mounted, so there's no `mount:b`. + tracker.assertEvents( + _.unmounted("a") + ) + } + } + + it("onMountInsert: `children <--` (a, nested `children <--` (c, nested `child <-- b`)) -> `child <-- b` keeps b mounted at depth 2") { + val tracker = createEventTracker() + val childrenBus = new EventBus[List[Inserter]] + val childBus = new EventBus[Div] + + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + tracker.clear() + + var dynamicInserter: Inserter = children <-- childrenBus.events + val takeoverInserter: Inserter = child <-- childBus.events + + val el = div("Hello ", onMountInsert(_ => dynamicInserter), " world") + + withClue("initial: b sits two levels deep, next to c:") { + mount(el) + childrenBus.emit(List(a, children <-- Val(List[Inserter](c, child <-- Val(b))))) + expectNode( + div.of( + "Hello ", + sentinel, // outer list + div of "a", + sentinel, // nested list + div of "c", + sentinel, div of "b", sentinel, // nested child + sentinel, // nested list trailing + sentinel, // outer list trailing + " world" + ) + ) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("c"), + _.mounted("b") + ) + .clear() + } + + withClue("unmount then remount rides everything on the element's own lifecycle:") { + unmount() + dynamicInserter = takeoverInserter + mount(el) + tracker + .assertEvents( + _.unmounted("a"), + _.unmounted("c"), + _.unmounted("b"), + _.mounted("a"), + _.mounted("c"), + _.mounted("b") + ) + .clear() + } + + withClue("`child <-- b` takes over: both nested items are torn down around b, which stays mounted:") { + childBus.emit(b) + expectNode(div of ("Hello ", sentinel, div of "b", " world")) + tracker.assertEvents( + _.unmounted("a"), + _.unmounted("c") + ) + } + } + + it("onMountInsert: after `child <-- b` takes b over from a nested `child <--`, that nested inserter is dead") { + // The retained node's previous owner (the nested `child <--`) is torn down, so its source + // must no longer be able to touch the DOM – b now belongs to the takeover `child <--` alone. + val tracker = createEventTracker() + val childBus = new EventBus[Div] + + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val c = tracker.createDiv("c") + val d = tracker.createDiv("d") + tracker.clear() + + val nestedVar = Var(b) + var dynamicInserter: Inserter = children <-- Val(List[Inserter](a, child <-- nestedVar.signal)) + val takeoverInserter: Inserter = child <-- childBus.events + + val el = div("Hello ", onMountInsert(_ => dynamicInserter), " world") + + withClue("initial:") { + mount(el) + expectNode(div of ("Hello ", sentinel, div of "a", sentinel, div of "b", sentinel, sentinel, " world")) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("b") + ) + .clear() + } + + withClue("remount with the takeover inserter, which then emits b:") { + unmount() + dynamicInserter = takeoverInserter + mount(el) + tracker + .assertEvents( + _.unmounted("a"), + _.unmounted("b"), + _.mounted("a"), + _.mounted("b") + ) + .clear() + childBus.emit(b) + expectNode(div of ("Hello ", sentinel, div of "b", " world")) + tracker + .assertEvents( + _.unmounted("a") + ) + .clear() + } + + withClue("the old nested `child <--` no longer renders anything:") { + nestedVar.set(c) + expectNode(div of ("Hello ", sentinel, div of "b", " world")) + tracker.assertNoEvents.clear() + } + + withClue("the takeover `child <--` keeps working normally:") { + childBus.emit(d) + expectNode(div of ("Hello ", sentinel, div of "d", " world")) + tracker + .assertEvents( + _.unmounted("b"), + _.mounted("d") + ) + .clear() + } + } + + it("onMountInsert: `children <--` (a, nested `child <-- b`) -> `child <-- x` unmounts a and b before mounting x") { + // The counterpart of the tests above: a nested node that the takeover does NOT emit is torn + // down as usual, and the old content unmounts before the new child mounts. + val tracker = createEventTracker() + val childrenBus = new EventBus[List[Inserter]] + val childBus = new EventBus[Div] + + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + val x = tracker.createDiv("x") + tracker.clear() + + var dynamicInserter: Inserter = children <-- childrenBus.events + val takeoverInserter: Inserter = child <-- childBus.events + + val el = div("Hello ", onMountInsert(_ => dynamicInserter), " world") + + withClue("initial:") { + mount(el) + childrenBus.emit(List(a, child <-- Val(b))) + expectNode(div of ("Hello ", sentinel, div of "a", sentinel, div of "b", sentinel, sentinel, " world")) + tracker + .assertEvents( + _.mounted("a"), + _.mounted("b") + ) + .clear() + } + + withClue("remount with the takeover inserter, which then emits x:") { + unmount() + dynamicInserter = takeoverInserter + mount(el) + tracker + .assertEvents( + _.unmounted("a"), + _.unmounted("b"), + _.mounted("a"), + _.mounted("b") + ) + .clear() + childBus.emit(x) + expectNode(div of ("Hello ", sentinel, div of "x", " world")) + tracker.assertEvents( + _.unmounted("a"), + _.unmounted("b"), + _.mounted("x") + ) + } + } + + it("onMountInsert: `children <--` (nested `child <-- b`, a) -> another `children <--` (a, b) keeps b mounted") { + // A `children <--` takeover reconciles against the previous list's content, so it gets the + // same treatment as a regular `children <--` update: b is released from the nested item + // that's leaving, instead of being re-mounted. + val tracker = createEventTracker() + val takeoverBus = new EventBus[List[Node]] + + val a = tracker.createDiv("a") + val b = tracker.createDiv("b") + tracker.clear() + + var dynamicInserter: Inserter = children <-- Val(List[Inserter](child <-- Val(b), a)) + val takeoverInserter: Inserter = children <-- takeoverBus.events + + val el = div("Hello ", onMountInsert(_ => dynamicInserter), " world") + + withClue("initial:") { + mount(el) + expectNode(div of ("Hello ", sentinel, sentinel, div of "b", sentinel, div of "a", sentinel, " world")) + tracker + .assertEvents( + _.mounted("b"), + _.mounted("a") + ) + .clear() + } + + withClue("remount with the takeover inserter, which then emits (a, b):") { + unmount() + dynamicInserter = takeoverInserter + mount(el) + tracker + .assertEvents( + _.unmounted("b"), + _.unmounted("a"), + _.mounted("b"), + _.mounted("a") + ) + .clear() + takeoverBus.emit(List(a, b)) + expectNode(div of ("Hello ", sentinel, div of "a", div of "b", sentinel, " world")) + tracker.assertNoEvents + } + } + // -- `children <--` context: the parent stays mounted throughout, so unmount callbacks // fire exactly when content is torn down – an unambiguous check. -- diff --git a/src/test/scala/com/raquo/laminar/tests/SlotAttributeStealingSpec.scala b/src/test/scala/com/raquo/laminar/tests/SlotAttributeStealingSpec.scala index 5590ec64..bd773f5a 100644 --- a/src/test/scala/com/raquo/laminar/tests/SlotAttributeStealingSpec.scala +++ b/src/test/scala/com/raquo/laminar/tests/SlotAttributeStealingSpec.scala @@ -1,6 +1,8 @@ package com.raquo.laminar.tests import com.raquo.laminar.api.L._ +import com.raquo.laminar.inserters.CollectionCommand +import com.raquo.laminar.inserters.CollectionCommand.{Append, ReplaceAll} import com.raquo.laminar.nodes.Slot import com.raquo.laminar.utils.UnitSpec @@ -141,6 +143,59 @@ class SlotAttributeStealingSpec extends UnitSpec { } } + List(true, false).foreach { minimizeDiff => + + it(s"a slotted `children.command` ReplaceAll(minimizeDiff = $minimizeDiff) re-asserts the Slot's slot over a manual override, even on a node that stays in place") { + // Same as the reorder above: ReplaceAll is a fresh Slot write for every node it's given, + // regardless of whether the node needs to move, and whether it's kept or re-inserted. + val tracker = createEventTracker() + val e = tracker.createSpan("E") + val f = tracker.createSpan("F") + tracker.clear() + val commandBus = new EventBus[CollectionCommand[HtmlElement]] + val host = div(new Slot("prefix")(children.command <-- commandBus.events)) + + withClue("both elements get the Slot's slot on mount:") { + mount(div(host)) + commandBus.emit(Append(e)) + commandBus.emit(Append(f)) + tracker + .assertEvents( + _.mounted("E"), + _.mounted("F") + ) + .clear() + e.ref.getAttribute("slot") shouldBe "prefix" + f.ref.getAttribute("slot") shouldBe "prefix" + } + + withClue("a manual slot := overrides just that element:") { + e.amend(slot := "manual") + tracker.assertNoEvents.clear() + e.ref.getAttribute("slot") shouldBe "manual" + f.ref.getAttribute("slot") shouldBe "prefix" + } + + withClue("ReplaceAll(e, f) re-asserts the Slot's slot on e, which stays in place:") { + commandBus.emit(ReplaceAll(e :: f :: Nil, minimizeDiff)) + if (minimizeDiff) { + tracker.assertNoEvents.clear() + } else { + tracker + .assertEvents( + _.unmounted("E"), + _.unmounted("F"), + _.mounted("E"), + _.mounted("F") + ) + .clear() + } + e.ref.getAttribute("slot") shouldBe "prefix" + f.ref.getAttribute("slot") shouldBe "prefix" + } + } + } + // -- A `slot <--` with an initial value, mounted into a Slot -- it("a slot <-- signal with an initial value wins over the Slot on mount") {