From 8dec49c496d25119fd91af6791c9cab8f7370e6c Mon Sep 17 00:00:00 2001 From: Nikita Gazarov Date: Sun, 19 Jan 2020 01:46:43 -0800 Subject: [PATCH] New: Rework Ownership and Lifecycle events - Use new DynamicOwner from Airstream - Element subscriptions are only activated on element mount, not on element init - This fixes the main memory management gotcha: Fixes #33 - New pilot subscription fixes #47 --- CHANGELOG.md | 6 + build.sbt | 2 +- docs/Documentation.md | 2 +- .../com/raquo/laminar/api/Airstream.scala | 4 +- .../scala/com/raquo/laminar/api/Laminar.scala | 2 +- .../raquo/laminar/lifecycle/MountEvent.scala | 32 +- .../com/raquo/laminar/nodes/ChildNode.scala | 2 +- .../raquo/laminar/nodes/ReactiveElement.scala | 104 +++--- .../laminar/nodes/ReactiveHtmlElement.scala | 3 +- .../laminar/nodes/ReactiveSvgElement.scala | 3 +- .../com/raquo/laminar/nodes/RootNode.scala | 7 +- .../raquo/laminar/LifecycleEventSpec.scala | 322 ++++++++++-------- .../laminar/SubscriptionLifecycleSpec.scala | 12 +- .../com/raquo/laminar/WeirdCasesSpec.scala | 11 +- .../laminar/example/components/TreeSpec.scala | 44 +-- .../laminar/fixtures/TestableOwner.scala | 8 +- .../com/raquo/laminar/utils/LaminarSpec.scala | 18 +- 17 files changed, 326 insertions(+), 256 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67b344ce..19fcf1f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ Breaking changes in **bold**. #### v0.8 – TBD +* **API: Lifecycle events & Ownership overhaul** + * Use Airstream's new `DynamicOwner` and `DynamicSubscription` features: `ReactiveElement` subscriptions are now activated only when the element is mounted, not when it is created. + * Transaction delay for `mountEvents` now applies to all element subscriptions + * Allow remounting of unmounted components (document pilot subscription memory management gotcha) + * `NodeWasDiscarded` event is not fired anymore. See `NodeWillUnmount`. +* **API: Hide `parentChangeEvents` and `maybeParentSignal`. This functionality is not available anymore as it requires undesirable tradeoffs. * **API: Rename types:** `ReactiveHtmlBuilders` -> `HtmlBuilders`, `ReactiveSvgBuilders` -> `SvgBuilders`, `ReactiveRoot` -> `RootNode`, `ReactiveComment` -> `CommentNode`, `ReactiveText` -> `TextNode`, `ReactiveChildNode` -> `ChildNode` * **API: Move `ChildrenCommand` out of the poorly named `collection` package** * **API: Eliminate dependency on _Scala DOM Builder_** diff --git a/build.sbt b/build.sbt index b4baf60d..6176c359 100644 --- a/build.sbt +++ b/build.sbt @@ -6,7 +6,7 @@ enablePlugins(ScalaJSBundlerPlugin) // resolvers += Resolver.sonatypeRepo("snapshots") libraryDependencies ++= Seq( - "com.raquo" %%% "airstream" % "0.7.2", + "com.raquo" %%% "airstream" % "0.7.3-SNAPSHOT", "com.raquo" %%% "domtypes" % "0.9.6", "com.raquo" %%% "domtestutils" % "0.9.1" % Test, "org.scalatest" %%% "scalatest" % "3.0.8" % Test diff --git a/docs/Documentation.md b/docs/Documentation.md index 7f4f6fbe..573f1b46 100644 --- a/docs/Documentation.md +++ b/docs/Documentation.md @@ -164,7 +164,7 @@ Thanks to the `optionToModifier` implicit mentioned above, you can use `Option[R And so, an empty DOM node can be created with... `emptyNode`. -// @NC What type actually +// @nc What type actually ```scala val node: Node = if (foo) element else emptyNode diff --git a/src/main/scala/com/raquo/laminar/api/Airstream.scala b/src/main/scala/com/raquo/laminar/api/Airstream.scala index ce834528..19663577 100644 --- a/src/main/scala/com/raquo/laminar/api/Airstream.scala +++ b/src/main/scala/com/raquo/laminar/api/Airstream.scala @@ -16,8 +16,6 @@ trait Airstream { type Observer[-A] = airstream.core.Observer[A] - type Owned = airstream.ownership.Owned - type Owner = airstream.ownership.Owner type Ref[+A <: AnyRef] = airstream.util.Ref[A] @@ -28,7 +26,7 @@ trait Airstream { type StrictSignal[+A] = airstream.signal.StrictSignal[A] - type Subscription = airstream.core.Subscription + type Subscription = airstream.ownership.Subscription type Val[A] = airstream.signal.Val[A] diff --git a/src/main/scala/com/raquo/laminar/api/Laminar.scala b/src/main/scala/com/raquo/laminar/api/Laminar.scala index eada9015..f1f27605 100644 --- a/src/main/scala/com/raquo/laminar/api/Laminar.scala +++ b/src/main/scala/com/raquo/laminar/api/Laminar.scala @@ -160,7 +160,7 @@ private[laminar] object Laminar @inline def render( container: dom.Element, - rootNode: nodes.ChildNode[dom.Element] + rootNode: nodes.ReactiveElement.Base ): RootNode = { new RootNode(container, rootNode) } diff --git a/src/main/scala/com/raquo/laminar/lifecycle/MountEvent.scala b/src/main/scala/com/raquo/laminar/lifecycle/MountEvent.scala index 80de2b8b..7333fcca 100644 --- a/src/main/scala/com/raquo/laminar/lifecycle/MountEvent.scala +++ b/src/main/scala/com/raquo/laminar/lifecycle/MountEvent.scala @@ -4,13 +4,20 @@ package com.raquo.laminar.lifecycle /** Mount Events can inform you about certain stages of the Node's lifecycle, * letting you perform the required initialization / cleanup. * - * They are also used internally for managing the lifecycle of stream subscriptions. + * They are also used internally for managing the lifecycle of element subscriptions. * * A Node is considered mounted if it's present in the DOM, i.e. it has `dom.document` * among its ancestors. A node that is not mounted is considered unmounted. * * All MountEvent-s are cascading: if a certain event is fired on a node, it will also * fire on all descendants of that node. + * + * Important: All MountEvent-s are emitted AFTER the current Airstream transaction completes. + * That means that any code that listens to those events including all element subscriptions + * will be run AFTER whatever event caused the element to get mounted has finished propagating. + * So don't expect to fire an event, mount an element in response to that event, and have one + * of this new element's subscriptions also react to the same event. The subscription will not see + * that event. See the ignored test in WeirdCasesSpec. */ sealed trait MountEvent @@ -22,30 +29,11 @@ sealed trait MountEvent object NodeDidMount extends MountEvent /** `NodeWillUnmount` event fires when a previously mounted node is about to be unmounted. - * Note: when the even fires the node is still mounted. + * Note: when the event fires, the node is still mounted. * * Internally, Laminar will deactivate the node's subscriptions when it's unmounted, - * except for the internal subscription that listens to mountEvents (so that we can + * except for the internal pilot subscription that listens to mountEvents (so that we can * re-activate the other subscriptions when the node gets mounted again). - * - * Note: currently, every `NodeWillUnmount` event is followed by a `NodeWillBeDiscarded` - * event which deactivates the subscription that listens to mountEvents */ object NodeWillUnmount extends MountEvent -// @TODO Provide a way to avoid discarding nodes (and how do we manually discard them then? Discard when parent is discarded...? Think about it and update the docs before implementing) -// @TODO Think about the inheritance mechanics of discarding, and document all this -/** `NodeWasDiscarded` event fires after `NodeWillUnmount` unless the end - * user specified that the node should not be discarded when it's unmounted. - * - * A discarded node is defined as a node that is unmounted and will not be mounted again. - * The latter is a promise by the end user, not enforced by Laminar. - * - * Internally, when discarding a node Laminar kills all of the subscriptions that it owned. - * NodeWasDiscarded event is your last chance to do any cleanup related to this node. - * - * Note: There currently is no way for the end user to specify that the node should not be - * discarded, so the end user implicitly promises to not re-mount nodes which have - * been unmounted. - */ -object NodeWasDiscarded extends MountEvent diff --git a/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala b/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala index 1afd59f0..d03a6fcc 100644 --- a/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala +++ b/src/main/scala/com/raquo/laminar/nodes/ChildNode.scala @@ -36,7 +36,7 @@ trait ChildNode[+Ref <: dom.Node] * * @param maybeNextParent `None` means this node is about to be detached form its parent */ - @inline def willSetParent(maybeNextParent: Option[ParentNode.Base]): Unit = {} + @inline def willSetParent(maybeNextParent: Option[ParentNode.Base]): Unit = () override def apply(parentNode: ParentNode.Base): Unit = { // @TODO[Performance] Consider making ChildNode -> Modifier conversion implicit instead (but watch compile times) diff --git a/src/main/scala/com/raquo/laminar/nodes/ReactiveElement.scala b/src/main/scala/com/raquo/laminar/nodes/ReactiveElement.scala index de0e1124..e9827359 100644 --- a/src/main/scala/com/raquo/laminar/nodes/ReactiveElement.scala +++ b/src/main/scala/com/raquo/laminar/nodes/ReactiveElement.scala @@ -1,15 +1,15 @@ package com.raquo.laminar.nodes -import com.raquo.airstream.core.{Observable, Observer, Subscription} -import com.raquo.airstream.eventbus.{EventBus, EventBusSource, WriteBus} +import com.raquo.airstream.core.{Observable, Observer} +import com.raquo.airstream.eventbus.{EventBus, WriteBus} import com.raquo.airstream.eventstream.EventStream -import com.raquo.airstream.ownership.{Owned, Owner} +import com.raquo.airstream.ownership.{DynamicOwner, DynamicSubscription, Owner, Subscription} import com.raquo.airstream.signal.Signal import com.raquo.domtypes import com.raquo.domtypes.generic.keys.EventProp import com.raquo.laminar.DomApi import com.raquo.laminar.emitter.EventPropEmitter -import com.raquo.laminar.lifecycle.{MountEvent, NodeDidMount, NodeWasDiscarded, NodeWillUnmount, ParentChangeEvent} +import com.raquo.laminar.lifecycle.{MountEvent, NodeDidMount, NodeWillUnmount, ParentChangeEvent} import com.raquo.laminar.nodes.ChildNode.isParentMounted import com.raquo.laminar.nodes.ReactiveElement.noMountEvents import com.raquo.laminar.receivers.{ChildReceiver, ChildrenCommandReceiver, ChildrenReceiver, MaybeChildReceiver, TextChildReceiver} @@ -21,35 +21,35 @@ import scala.collection.mutable trait ReactiveElement[+Ref <: dom.Element] extends ChildNode[Ref] with ParentNode[Ref] - with domtypes.generic.nodes.Element - with Owner { + with domtypes.generic.nodes.Element { // @TODO[Naming] We reuse EventPropSetter to represent an active event listener. Makes for a bit confusing naming. private[ReactiveElement] var _maybeEventListeners: Option[mutable.Buffer[EventPropSetter[_]]] = None @inline def maybeEventListeners: Option[List[EventPropSetter[_]]] = _maybeEventListeners.map(_.toList) + private[this] val dynamicOwner: DynamicOwner = new DynamicOwner + /** Event bus that emits parent change events. * For efficiency, it is only populated when someone accesses [[parentChangeEvents]] */ - private[laminar] var maybeParentChangeBus: Option[EventBus[ParentChangeEvent]] = None + private[this] var maybeParentChangeBus: Option[EventBus[ParentChangeEvent]] = None /** Event bus that emits this node's mount events. * For efficiency, it is only populated when someone accesses [[thisNodeMountEvents]] */ - private[laminar] var maybeThisNodeMountEventBus: Option[EventBus[MountEvent]] = None + private[this] var maybeThisNodeMountEventBus: Option[EventBus[MountEvent]] = None /** Stream of parent change events for this node. - * For efficiency, it is lazy loaded, only being initialized when accessed, - * either directly or (more commonly) as a dependency of [[mountEvents]] + * For efficiency, it is lazy loaded, only being initialized when accessed */ - lazy val parentChangeEvents: EventStream[ParentChangeEvent] = { + private[this] lazy val parentChangeEvents: EventStream[ParentChangeEvent] = { val parentChangeBus = new EventBus[ParentChangeEvent] maybeParentChangeBus = Some(parentChangeBus) parentChangeBus.events } - lazy val maybeParentSignal: Signal[Option[ParentNode.Base]] = parentChangeEvents + private[this] lazy val maybeParentSignal: Signal[Option[ParentNode.Base]] = parentChangeEvents .filter(_.alreadyChanged) // @TODO[Integrity] is this important? .map(_.maybeNextParent) .toSignal(maybeParent) @@ -57,7 +57,7 @@ trait ReactiveElement[+Ref <: dom.Element] /** Emits mount events that were caused by this element's parent changing its parent, * or any such changes up the chain. Does not include mount events triggered by this * element changing its parent - see [[thisNodeMountEvents]] for that. */ - private lazy val ancestorMountEvents: EventStream[MountEvent] = { + private[this] lazy val ancestorMountEvents: EventStream[MountEvent] = { maybeParentSignal.map { case Some(nextParent: ReactiveElement[_]) => nextParent.mountEvents @@ -69,7 +69,7 @@ trait ReactiveElement[+Ref <: dom.Element] /** Emits mount events caused by this node changing its parent. Does not include mount * events triggered by changes higher in the hierarchy (grandparent and up) – see * [[ancestorMountEvents]] for that. */ - private lazy val thisNodeMountEvents: EventStream[MountEvent] = { + private[this] lazy val thisNodeMountEvents: EventStream[MountEvent] = { val thisNodeMountEventBus = new EventBus[MountEvent] maybeThisNodeMountEventBus = Some(thisNodeMountEventBus) thisNodeMountEventBus.events @@ -80,6 +80,25 @@ trait ReactiveElement[+Ref <: dom.Element] EventStream.merge(ancestorMountEvents, thisNodeMountEvents) } + // @nc normally this would not work memory management wise, as we create a sub on element init, and never even bother to kill it + // - however, this sub only looks at mount events + // - thisNodeMountEvents are contained within this node and don't require any leaky resources + // - ancestorMountEvents look at parent nodes' mountEvents + // - while this node is mounted, all ancestors are also mounted and that is ok + // - if this node becomes unmounted, the ancestor chain includes only other unmounted nodes + // - so this will lock together that ancestor chain, but that's like meh + // - even if we re-mounted this node to different parents / ancestors, this chain would be broken as `mountEvents` would now point elsewhere + // - as long as we understand and document this limitation, I think it's ok + // @nc Can we initialize this conditionally, only for elements that need it? + { + val pilotSubscriptionOwner: Owner = new Owner {} + + mountEvents.foreach{ + case NodeDidMount => dynamicOwner.activate() + case NodeWillUnmount => dynamicOwner.deactivate() + }(pilotSubscriptionOwner) + } + /** Create and get a stream of events on this node */ def events[Ev <: dom.Event]( eventProp: EventProp[Ev], @@ -108,45 +127,42 @@ trait ReactiveElement[+Ref <: dom.Element] final def <--[V](childrenCommandReceiver: ChildrenCommandReceiver.type): ChildrenCommandReceiver = new ChildrenCommandReceiver(this) - // @TODO[Naming] Not a fan of `subscribeO` name, but it needs to be different from `subscribe` for type inference to work + // @TODO[Naming] Not a fan of `subscribeS` / `subscribeO` names, but it needs to be different from `subscribe` for type inference to work + // @TODO[API] Consider having subscribe() return a Subscribe object that has several apply methods on it to reign in this madness + // - Also, do we really need currying here? + + def subscribeS(getSubscription: Owner => Subscription): DynamicSubscription = { + new DynamicSubscription(dynamicOwner, getSubscription) + } - def subscribeO[V](observable: Observable[V])(observer: Observer[V]): Subscription = { - observable.addObserver(observer)(owner = this) + def subscribeO[V](observable: Observable[V])(observer: Observer[V]): DynamicSubscription = { + subscribeS(owner => observable.addObserver(observer)(owner)) } // @TODO[Naming] Rename to subscribeThisO? - def subscribeO[V](getObservable: this.type => Observable[V])(observer: Observer[V]): Subscription = { + def subscribeO[V](getObservable: this.type => Observable[V])(observer: Observer[V]): DynamicSubscription = { subscribeO(getObservable(this))(observer) } - def subscribe[V](observable: Observable[V])(onNext: V => Unit): Subscription = { + def subscribe[V](observable: Observable[V])(onNext: V => Unit): DynamicSubscription = { subscribeO(observable)(Observer(onNext)) } // @TODO[Naming] Rename to subscribeThis? - def subscribe[V](getObservable: this.type => Observable[V])(onNext: V => Unit): Subscription = { + def subscribe[V](getObservable: this.type => Observable[V])(onNext: V => Unit): DynamicSubscription = { subscribeO(getObservable(this))(Observer(onNext)) } def subscribeBus[V]( sourceStream: EventStream[V], targetBus: WriteBus[V] - ): EventBusSource[V] = { - targetBus.addSource(sourceStream)(owner = this) - } - - /** Check whether the node is currently mounted. - * - * You can use this method to simplify your code and possibly improve performance - * where you'd otherwise need to subscribe to and transform [[mountEvents]] - */ - def isMounted: Boolean = { - isParentMounted(maybeParent) + ): DynamicSubscription = { + subscribeS(owner => targetBus.addSource(sourceStream)(owner)) } /** This hook is exposed by Scala DOM Builder for this exact purpose */ override def willSetParent(maybeNextParent: Option[ParentNode.Base]): Unit = { - // @TODO[Integrity] In dombuilder, willSetParent is called at incorrect time in ParentNode.appendChild + // super.willSetParent(maybeNextParent) // default implementation is a noop, so no need to call it. // dom.console.log(">>>> willSetParent", this.ref.tagName + "(" + this.ref.textContent + ")", maybeParent == maybeNextParent, maybeParent.toString, maybeNextParent.toString) if (maybeNextParent != maybeParent) { maybeParentChangeBus.foreach(bus => { @@ -171,37 +187,21 @@ trait ReactiveElement[+Ref <: dom.Element] super.setParent(maybeNextParent) // dom.console.log(">>>> setParent", this.ref.tagName + "(" + this.ref.textContent + ")", maybePrevParent == maybeNextParent, maybePrevParent.toString, maybeNextParent.toString) if (maybeNextParent != maybePrevParent) { - maybeParentChangeBus.foreach( bus => { - bus.writer.onNext(ParentChangeEvent( + maybeParentChangeBus.foreach(bus => { + val ev = ParentChangeEvent( alreadyChanged = true, maybePrevParent = maybePrevParent, maybeNextParent = maybeNextParent - )) + ) + bus.writer.onNext(ev) }) val prevParentIsMounted = isParentMounted(maybePrevParent) val nextParentIsMounted = isParentMounted(maybeNextParent) if (!prevParentIsMounted && nextParentIsMounted) { maybeThisNodeMountEventBus.foreach(_.writer.onNext(NodeDidMount)) } - if (prevParentIsMounted && !nextParentIsMounted) { - maybeThisNodeMountEventBus.foreach { bus => - bus.writer.onNext(NodeWasDiscarded) // @TODO this should be optional - } - } } } - - override protected[this] def onOwned(owned: Owned): Unit = { - val isFirstSubscription = possessions.length == 1 - if (isFirstSubscription) { - // Create a subscription that will kill all subscriptions that this element owns when it is discarded. - // Note: Once this subscription is created, it will persist until this element is discarded. - // This might be suboptimal in a narrow range of conditions, I don't think anyone will run into those. - // @TODO[Performance] ^^^^ The above can be addressed by e.g. overriding onKilledExternally (but permissions) - mountEvents.filter(_ == NodeWasDiscarded).foreach(_ => killPossessions() )(owner = this) - } - } - } object ReactiveElement { diff --git a/src/main/scala/com/raquo/laminar/nodes/ReactiveHtmlElement.scala b/src/main/scala/com/raquo/laminar/nodes/ReactiveHtmlElement.scala index 79b3ffe1..871da545 100644 --- a/src/main/scala/com/raquo/laminar/nodes/ReactiveHtmlElement.scala +++ b/src/main/scala/com/raquo/laminar/nodes/ReactiveHtmlElement.scala @@ -20,7 +20,8 @@ class ReactiveHtmlElement[+Ref <: dom.html.Element](val tag: HtmlTag[Ref]) final def <--[V](focus: FocusReceiver.type): FocusReceiver = new FocusReceiver(this) override def toString: String = { - s"ReactiveHtmlElement(${ref.outerHTML})" + // `ref` is not available inside ReactiveElement's constructor due to initialization order, so fall back to `tag`. + s"ReactiveHtmlElement(${ if (ref != null) ref.outerHTML else s"tag=${tag.name}"})" } } diff --git a/src/main/scala/com/raquo/laminar/nodes/ReactiveSvgElement.scala b/src/main/scala/com/raquo/laminar/nodes/ReactiveSvgElement.scala index fb826d23..2fabc684 100644 --- a/src/main/scala/com/raquo/laminar/nodes/ReactiveSvgElement.scala +++ b/src/main/scala/com/raquo/laminar/nodes/ReactiveSvgElement.scala @@ -14,7 +14,8 @@ class ReactiveSvgElement[+Ref <: dom.svg.Element](val tag: SvgTag[Ref]) final def <--[V](attr: ReactiveSvgAttr[V]): SvgAttrReceiver[V] = new SvgAttrReceiver(attr, this) override def toString: String = { - s"ReactiveSvgElement(${ref.outerHTML})" + // `ref` is not available inside ReactiveElement's constructor due to initialization order, so fall back to `tag`. + s"ReactiveSvgElement${ if (ref != null) ref.outerHTML else s"tag=${tag.name}"})" } } diff --git a/src/main/scala/com/raquo/laminar/nodes/RootNode.scala b/src/main/scala/com/raquo/laminar/nodes/RootNode.scala index a676b534..8140c43c 100644 --- a/src/main/scala/com/raquo/laminar/nodes/RootNode.scala +++ b/src/main/scala/com/raquo/laminar/nodes/RootNode.scala @@ -4,9 +4,14 @@ import org.scalajs.dom class RootNode( val container: dom.Element, - val child: ChildNode[dom.Element] + val child: ReactiveElement.Base ) extends ParentNode[dom.Element] { + // @nc Do we actually need this? If so, document the exact reason + if (!ChildNode.isNodeMounted(container)) { + throw new Exception("Unable to mount Laminar RootNode into an unmounted container.") + } + /** When we create a Root, we don't want to create a new HTML Element, we want to * use a reference to an existing element, the container. */ diff --git a/src/test/scala/com/raquo/laminar/LifecycleEventSpec.scala b/src/test/scala/com/raquo/laminar/LifecycleEventSpec.scala index 0a1ae10a..a293396c 100644 --- a/src/test/scala/com/raquo/laminar/LifecycleEventSpec.scala +++ b/src/test/scala/com/raquo/laminar/LifecycleEventSpec.scala @@ -1,19 +1,99 @@ package com.raquo.laminar +import com.raquo.domtestutils.matching.ExpectedNode import com.raquo.laminar.api.L._ -import com.raquo.laminar.lifecycle.{MountEvent, NodeDidMount, NodeWasDiscarded, NodeWillUnmount, ParentChangeEvent} +import com.raquo.laminar.lifecycle.{MountEvent, NodeDidMount, NodeWillUnmount, ParentChangeEvent} import com.raquo.laminar.nodes.ReactiveElement import com.raquo.laminar.utils.UnitSpec import org.scalajs.dom +import scala.collection.mutable + class LifecycleEventSpec extends UnitSpec { case class TestCase( action: ReactiveElement[dom.html.Element] => Unit, - expectedChangeEvents: Seq[ParentChangeEvent], +// expectedChangeEvents: Seq[ParentChangeEvent], expectedMountEvents: Seq[MountEvent] ) + it("mount event transaction timing - level 1") { + + val textBus = new EventBus[String] + + var events = mutable.Buffer[MountEvent](); + + val readMountEvents: Mod[HtmlElement] = new Mod[HtmlElement] { + override def apply(element: HtmlElement): Unit = { + element.subscribe(_.mountEvents){ ev => events.append(ev) } + } + } + + def makeChild(text: String): Div = div(readMountEvents, text) + + val $child = textBus.events.map(makeChild) + + // -- + + mount(section("Hello, ", child <-- $child)) + + events shouldEqual mutable.Buffer() + expectNode(section like ("Hello, ", ExpectedNode.comment())) + + // -- + + textBus.writer.onNext("blah") + + events shouldEqual mutable.Buffer(NodeDidMount) + expectNode(section like ("Hello, ", div like ("blah"))) + + // -- + + textBus.writer.onNext("world") + + events shouldEqual mutable.Buffer(NodeDidMount, NodeWillUnmount, NodeDidMount) + expectNode(section like ("Hello, ", div like ("world"))) + + } + + it("mount event transaction timing - level 2 (nesting)") { + + val textBus = new EventBus[String] + + var events = mutable.Buffer[MountEvent](); + + val readMountEvents: Mod[HtmlElement] = new Mod[HtmlElement] { + override def apply(element: HtmlElement): Unit = { + element.subscribe(_.mountEvents){ ev => events.append(ev) } + } + } + + def makeChild(text: String): Div = div(span(readMountEvents, text)) + + val $child = textBus.events.map(makeChild) + + // -- + + mount(section("Hello, ", child <-- $child)) + + events shouldEqual mutable.Buffer() + expectNode(section like ("Hello, ", ExpectedNode.comment())) + + // -- + + textBus.writer.onNext("blah") + + events shouldEqual mutable.Buffer(NodeDidMount) + expectNode(section like ("Hello, ", div like ( span like ("blah")))) + + // -- + + textBus.writer.onNext("world") + + events shouldEqual mutable.Buffer(NodeDidMount, NodeWillUnmount, NodeDidMount) + expectNode(section like ("Hello, ", div like ( span like ("world")))) + } + it("Changing parent on a node fires appropriate events") { val grandChild = b("grandChild ") @@ -23,53 +103,58 @@ class LifecycleEventSpec extends UnitSpec { val parent1 = div("parent1") // @TODO remove this child val parent2 = p("parent2 ", "(two) ") - var parentEvents = Seq[ParentChangeEvent]() +// var parentEvents = Seq[ParentChangeEvent]() var mountEvents = Seq[MountEvent]() var grandChildMountEvents = Seq[MountEvent]() mount(div("root ", parent1, parent2)) + // If we wanted to test ParentChangeEvent-s, we would need to: + // - relax val visibility to allow that, and + // - use a different owner to subscribe to those events, because subscriptions on the element itself + // will not see those events when they coincide with mounting / unmounting because of transaction delay. + // We could potentially special-case these similar to pilotSubscription, but I don't know id that's wise. + val testCases = Seq( TestCase( parent1.appendChild, - expectedChangeEvents = Seq( - ParentChangeEvent( - alreadyChanged = false, - maybePrevParent = None, - maybeNextParent = Some(parent1) - ), - ParentChangeEvent( - alreadyChanged = true, - maybePrevParent = None, - maybeNextParent = Some(parent1) - ) - ), +// expectedChangeEvents = Seq( +// ParentChangeEvent( +// alreadyChanged = false, +// maybePrevParent = None, +// maybeNextParent = Some(parent1) +// ), +// ParentChangeEvent( +// alreadyChanged = true, +// maybePrevParent = None, +// maybeNextParent = Some(parent1) +// ) +// ), expectedMountEvents = Seq(NodeDidMount) ), TestCase( parent1.appendChild, - expectedChangeEvents = Seq(), +// expectedChangeEvents = Seq(), expectedMountEvents = Seq() ), TestCase( child => parent2.insertChild(child, atIndex = 1), - expectedChangeEvents = Seq( - ParentChangeEvent( - alreadyChanged = false, - maybePrevParent = Some(parent1), - maybeNextParent = Some(parent2) - ), - ParentChangeEvent( - alreadyChanged = true, - maybePrevParent = Some(parent1), - maybeNextParent = Some(parent2) - ) - ), +// expectedChangeEvents = Seq( +// ParentChangeEvent( +// alreadyChanged = false, +// maybePrevParent = Some(parent1), +// maybeNextParent = Some(parent2) +// ), +// ParentChangeEvent( +// alreadyChanged = true, +// maybePrevParent = Some(parent1), +// maybeNextParent = Some(parent2) +// ) +// ), expectedMountEvents = Seq() ), - // @TODO this does not work anymore because unmounting perma-kills subscriptions now -// TestCase( -// parent2.removeChild, + TestCase( + parent2.removeChild, // expectedChangeEvents = Seq( // ParentChangeEvent( // alreadyChanged = false, @@ -82,13 +167,12 @@ class LifecycleEventSpec extends UnitSpec { // maybeNextParent = None // ) // ), -// expectedMountEvents = Seq( -// NodeWillUnmount, -// NodeWasDiscarded -// ) -// ), -// TestCase( -// parent2.appendChild, + expectedMountEvents = Seq( + NodeWillUnmount + ) + ), + TestCase( + parent2.appendChild, // expectedChangeEvents = Seq( // ParentChangeEvent( // alreadyChanged = false, @@ -101,40 +185,39 @@ class LifecycleEventSpec extends UnitSpec { // maybeNextParent = Some(parent2) // ) // ), -// expectedMountEvents = Seq(NodeDidMount) -// ), + expectedMountEvents = Seq(NodeDidMount) + ), TestCase( parent2.replaceChild(_, otherChild), - expectedChangeEvents = Seq( - ParentChangeEvent( - alreadyChanged = false, - maybePrevParent = Some(parent2), - maybeNextParent = None - ), - ParentChangeEvent( - alreadyChanged = true, - maybePrevParent = Some(parent2), - maybeNextParent = None - ) - ), +// expectedChangeEvents = Seq( +// ParentChangeEvent( +// alreadyChanged = false, +// maybePrevParent = Some(parent2), +// maybeNextParent = None +// ), +// ParentChangeEvent( +// alreadyChanged = true, +// maybePrevParent = Some(parent2), +// maybeNextParent = None +// ) +// ), expectedMountEvents = Seq( - NodeWillUnmount, - NodeWasDiscarded + NodeWillUnmount ) ) ) - child.subscribe(_.parentChangeEvents) { ev => - parentEvents = parentEvents :+ ev - } +// child.subscribe(_.parentChangeEvents) { ev => +// parentEvents = parentEvents :+ ev +// } child.subscribe(_.mountEvents) { ev => mountEvents = mountEvents :+ ev } - grandChild.subscribe(_.parentChangeEvents) { _ => - fail("grandChild received a ParentChangeEvent. This is not supposed to happen, as such events should not be inherited by child nodes.") - } +// grandChild.subscribe(_.parentChangeEvents) { _ => +// fail("grandChild received a ParentChangeEvent. This is not supposed to happen, as such events should not be inherited by child nodes.") +// } grandChild.subscribe(_.mountEvents) { ev => grandChildMountEvents = grandChildMountEvents :+ ev @@ -143,46 +226,49 @@ class LifecycleEventSpec extends UnitSpec { testCases.zipWithIndex.foreach { case (testCase, index) => withClue(s"Case index=$index:") { testCase.action(child) - parentEvents shouldEqual testCase.expectedChangeEvents + // parentEvents shouldEqual testCase.expectedChangeEvents mountEvents shouldEqual testCase.expectedMountEvents grandChildMountEvents shouldEqual testCase.expectedMountEvents // inheritance. assumes grandchild has no other parents and no mount events of its own - parentEvents = Seq() +// parentEvents = Seq() mountEvents = Seq() grandChildMountEvents = Seq() } } } - it("parentChangeEvents lazy val is not initialized if nothing asks for it even if the node has children") { - - val parent1 = div() - val parent2 = p("Yolo") - - val child = span(b("orly"), "yarly") - - child.maybeParentChangeBus.isDefined shouldBe false - child.maybeThisNodeMountEventBus.isDefined shouldBe false - - parent1(child) - - child.maybeParentChangeBus.isDefined shouldBe false - child.maybeThisNodeMountEventBus.isDefined shouldBe false - - parent2(child) - - child.maybeParentChangeBus.isDefined shouldBe false - child.maybeThisNodeMountEventBus.isDefined shouldBe false - - child.parentChangeEvents // Touching this should be enough to initialize `maybeParentChangeBus` - - child.maybeParentChangeBus.isDefined shouldBe true - child.maybeThisNodeMountEventBus.isDefined shouldBe false - - child.mountEvents // Touching this should be enough to initialize `maybeThisNodeMountEventBus` - - child.maybeParentChangeBus.isDefined shouldBe true - child.maybeThisNodeMountEventBus.isDefined shouldBe true - } + // This test will not pass because + // - these fields are now hidden + // - more importantly, we now initialize mountEvents from the start. Maybe we'll get around that eventually +// it("parentChangeEvents lazy val is not initialized if nothing asks for it even if the node has children") { +// +// val parent1 = div() +// val parent2 = p("Yolo") +// +// val child = span(b("orly"), "yarly") +// +// child.maybeParentChangeBus.isDefined shouldBe false +// child.maybeThisNodeMountEventBus.isDefined shouldBe false +// +// parent1(child) +// +// child.maybeParentChangeBus.isDefined shouldBe false +// child.maybeThisNodeMountEventBus.isDefined shouldBe false +// +// parent2(child) +// +// child.maybeParentChangeBus.isDefined shouldBe false +// child.maybeThisNodeMountEventBus.isDefined shouldBe false +// +// child.parentChangeEvents // Touching this should be enough to initialize `maybeParentChangeBus` +// +// child.maybeParentChangeBus.isDefined shouldBe true +// child.maybeThisNodeMountEventBus.isDefined shouldBe false +// +// child.mountEvents // Touching this should be enough to initialize `maybeThisNodeMountEventBus` +// +// child.maybeParentChangeBus.isDefined shouldBe true +// child.maybeThisNodeMountEventBus.isDefined shouldBe true +// } def makeMountEventTest( testAncestorMountEvents: Boolean, @@ -192,7 +278,9 @@ class LifecycleEventSpec extends UnitSpec { if (!testAncestorMountEvents && !testThisNodeMountEvents && testAncestorMountEvents) { fail("EventTest: there is nothing to test! Specify some options") } + } + it("Nested mount events") { val parent1 = div("parent1 ") val parent2 = p("parent2 ") val parent3 = article("parent3 ") @@ -211,10 +299,8 @@ class LifecycleEventSpec extends UnitSpec { var mountEvents: Seq[TargetNodeWithMountEvent] = Nil def subscribeToEvents(node: ReactiveElement[dom.html.Element]): Unit = { - if (testMountEvents) { - node.subscribe(_.mountEvents) { ev => - mountEvents = mountEvents :+ (node, ev) - } + node.subscribe(_.mountEvents) { ev => + mountEvents = mountEvents :+ (node, ev) } } @@ -223,9 +309,7 @@ class LifecycleEventSpec extends UnitSpec { expectedMountEvents: Seq[(ReactiveElement[dom.html.Element], MountEvent)] ): Unit = { withClue(clue + ": ") { - if (testMountEvents) { - mountEvents shouldEqual expectedMountEvents - } + mountEvents shouldEqual expectedMountEvents mountEvents = Nil } } @@ -294,14 +378,14 @@ class LifecycleEventSpec extends UnitSpec { expectNewEvents( "child4 was moved to parent5 which is unmounted", - Seq((child4, NodeWillUnmount), (child4, NodeWasDiscarded)) + Seq((child4, NodeWillUnmount)) ) parent5.appendChild(parent2) expectNewEvents( "parent2 was moved into parent5 which is unmounted", - Seq((child2, NodeWillUnmount), (child2, NodeWasDiscarded)) + Seq((child2, NodeWillUnmount)) ) subscribeToEvents(parent3) @@ -318,42 +402,8 @@ class LifecycleEventSpec extends UnitSpec { Seq( (parent3, NodeWillUnmount), (child3, NodeWillUnmount), - (grandChild3, NodeWillUnmount), - (parent3, NodeWasDiscarded), - (child3, NodeWasDiscarded), - (grandChild3, NodeWasDiscarded) + (grandChild3, NodeWillUnmount) ) ) - } - - // We test various combinations to ensure that the results are the same whether or not subscribe to streams that we test - - it("MountEventTest (ancestor, thisNode, combined) (false, false, true)")( - makeMountEventTest(testAncestorMountEvents = false, testThisNodeMountEvents = false, testMountEvents = true) - ) - - it("MountEventTest (ancestor, thisNode, combined) (false, true, false)")( - makeMountEventTest(testAncestorMountEvents = false, testThisNodeMountEvents = true, testMountEvents = false) - ) - - it("MountEventTest (ancestor, thisNode, combined) (false, true, true)")( - makeMountEventTest(testAncestorMountEvents = false, testThisNodeMountEvents = true, testMountEvents = true) - ) - - it("MountEventTest (ancestor, thisNode, combined) (true, false, false)")( - makeMountEventTest(testAncestorMountEvents = true, testThisNodeMountEvents = false, testMountEvents = false) - ) - - it("MountEventTest (ancestor, thisNode, combined) (true, false, true)")( - makeMountEventTest(testAncestorMountEvents = true, testThisNodeMountEvents = false, testMountEvents = true) - ) - - it("MountEventTest (ancestor, thisNode, combined) (true, true, false)")( - makeMountEventTest(testAncestorMountEvents = true, testThisNodeMountEvents = true, testMountEvents = false) - ) - - it("MountEventTest (ancestor, thisNode, combined) (true, true, true)")( - makeMountEventTest(testAncestorMountEvents = true, testThisNodeMountEvents = true, testMountEvents = true) - ) } diff --git a/src/test/scala/com/raquo/laminar/SubscriptionLifecycleSpec.scala b/src/test/scala/com/raquo/laminar/SubscriptionLifecycleSpec.scala index e313529e..cc817cbc 100644 --- a/src/test/scala/com/raquo/laminar/SubscriptionLifecycleSpec.scala +++ b/src/test/scala/com/raquo/laminar/SubscriptionLifecycleSpec.scala @@ -113,7 +113,7 @@ class SubscriptionLifecycleSpec extends UnitSpec { busB.writer.onNext(value0) expectNode(makeExpectedNodeA(value0)) counterA shouldBe 1 - counterB shouldBe 1 // even if not mounted, the subscriptions are active already + counterB shouldBe 0 // new in Laminar v0.8: childB subscriptions are not active yet because it is not mounted yet childCounter shouldBe 1 val value1 = values(1) @@ -121,22 +121,22 @@ class SubscriptionLifecycleSpec extends UnitSpec { busA.writer.onNext(value1) - expectNode(makeExpectedNodeB(value0)) // This used to expect an empty node B, but now with Airstream the noode starts listening when it's initialized, not when it's mounted + expectNode(emptyExpectedNodeB) // node B is mounted and activated, but it missed the value0 event in BusB counterA shouldBe 1 - counterB shouldBe 1 + counterB shouldBe 0 childCounter shouldBe 2 busB.writer.onNext(value1) expectNode(makeExpectedNodeB(value1)) counterA shouldBe 1 - counterB shouldBe 2 + counterB shouldBe 1 childCounter shouldBe 2 val value2 = values(2) busB.writer.onNext(value2) expectNode(makeExpectedNodeB(value2)) counterA shouldBe 1 - counterB shouldBe 3 + counterB shouldBe 2 childCounter shouldBe 2 unmount() @@ -150,7 +150,7 @@ class SubscriptionLifecycleSpec extends UnitSpec { childBus.writer.onNext(childA) expectNode(div like (alt is "unmounted")) counterA shouldBe 1 - counterB shouldBe 3 + counterB shouldBe 2 childCounter shouldBe 2 } } diff --git a/src/test/scala/com/raquo/laminar/WeirdCasesSpec.scala b/src/test/scala/com/raquo/laminar/WeirdCasesSpec.scala index 0b7d06a1..9207ccab 100644 --- a/src/test/scala/com/raquo/laminar/WeirdCasesSpec.scala +++ b/src/test/scala/com/raquo/laminar/WeirdCasesSpec.scala @@ -12,10 +12,19 @@ import com.raquo.laminar.utils.UnitSpec */ class WeirdCasesSpec extends UnitSpec { + // @nc I don't think we can get this to work. Can we maybe write a workaround to detect this condition? + // - Similar problem is described in https://github.com/raquo/Laminar/issues/47 + // - But now in Laminar v0.8 subscriptions are not activated until mount event fires, and that happens in a new transaction, + // but by then the original event that the synchronously dependent observable relies upon is long gone, so it doesn't get that event + // - Generally I think this is an acceptable tradeoff, as long as we document this. + // - Ideally we should offer a workaround. + // - I would assume most encounters of this would be a misuse of streams vs signals... + // @nc write a similar test for signals that works + /** See https://github.com/raquo/Laminar/issues/11 for a better approach * (basically, use child.text instead of nested observables) */ - it("nested, synchronously dependent observables work as expected for some reason") { + ignore("nested, synchronously dependent observables work as expected for some reason") { val bus = new EventBus[Int] diff --git a/src/test/scala/com/raquo/laminar/example/components/TreeSpec.scala b/src/test/scala/com/raquo/laminar/example/components/TreeSpec.scala index a4c21185..04fa8d67 100644 --- a/src/test/scala/com/raquo/laminar/example/components/TreeSpec.scala +++ b/src/test/scala/com/raquo/laminar/example/components/TreeSpec.scala @@ -4,6 +4,7 @@ import com.raquo.laminar.api.L import com.raquo.laminar.api.L._ import com.raquo.laminar.nodes.ChildNode import com.raquo.laminar.utils.UnitSpec +import org.scalajs.dom class TreeSpec extends UnitSpec { @@ -12,88 +13,89 @@ class TreeSpec extends UnitSpec { val container1 = div(alt := "foo").ref val otherContainer = div(alt := "foo2").ref + // Root needs to be attached to a mounted element + dom.document.body.appendChild(container1) + dom.document.body.appendChild(otherContainer) + val el0 = span("el0") val el10 = span("el10") val el11 = span("el11") val el2 = article("el2") val el3 = L.a("el3", href := "http://example.com") - val root = render(container1, el0) - val otherEl = div("otherEl") - val otherRoot = render(otherContainer, otherEl) + + val rootNode = render(container1, el0) + val otherRootNode = render(otherContainer, otherEl) val elx = b("elx") - ChildNode.isDescendantOf(node = el0.ref, ancestor = root.ref) shouldBe true + ChildNode.isDescendantOf(node = el0.ref, ancestor = rootNode.ref) shouldBe true ChildNode.isDescendantOf(node = el0.ref, ancestor = el10.ref) shouldBe false - ChildNode.isDescendantOf(node = el10.ref, ancestor = root.ref) shouldBe false + ChildNode.isDescendantOf(node = el10.ref, ancestor = rootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el11.ref, ancestor = el0.ref) shouldBe false ChildNode.isDescendantOf(node = el11.ref, ancestor = el11.ref) shouldBe false - ChildNode.isDescendantOf(node = el0.ref, ancestor = otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el0.ref, ancestor = otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el0.ref, ancestor = otherEl.ref) shouldBe false el0.appendChild(el10) el0.appendChild(el11) - ChildNode.isDescendantOf(node = el10.ref, ancestor = root.ref) shouldBe true - ChildNode.isDescendantOf(node = el11.ref, ancestor = root.ref) shouldBe true + ChildNode.isDescendantOf(node = el10.ref, ancestor = rootNode.ref) shouldBe true + ChildNode.isDescendantOf(node = el11.ref, ancestor = rootNode.ref) shouldBe true ChildNode.isDescendantOf(node = el10.ref, ancestor = el0.ref) shouldBe true ChildNode.isDescendantOf(node = el11.ref, ancestor = el0.ref) shouldBe true ChildNode.isDescendantOf(node = el11.ref, ancestor = el10.ref) shouldBe false ChildNode.isDescendantOf(node = el11.ref, ancestor = el2.ref) shouldBe false - ChildNode.isDescendantOf(node = el11.ref, ancestor = otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el11.ref, ancestor = otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el11.ref, ancestor = otherEl.ref) shouldBe false el10.appendChild(el2) - ChildNode.isDescendantOf(node = el2.ref, root.ref) shouldBe true + ChildNode.isDescendantOf(node = el2.ref, rootNode.ref) shouldBe true ChildNode.isDescendantOf(node = el2.ref, el0.ref) shouldBe true ChildNode.isDescendantOf(node = el2.ref, el10.ref) shouldBe true ChildNode.isDescendantOf(node = el2.ref, el11.ref) shouldBe false ChildNode.isDescendantOf(node = el2.ref, el2.ref) shouldBe false - ChildNode.isDescendantOf(node = el2.ref, otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el2.ref, otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el2.ref, otherEl.ref) shouldBe false el2.appendChild(el3) - ChildNode.isDescendantOf(node = el3.ref, ancestor = root.ref) shouldBe true + ChildNode.isDescendantOf(node = el3.ref, ancestor = rootNode.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el0.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el2.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el11.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = elx.ref) shouldBe false - ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = otherEl.ref) shouldBe false elx.insertChild(el3, atIndex = 0) ChildNode.isDescendantOf(node = el3.ref, ancestor = elx.ref) shouldBe true - ChildNode.isDescendantOf(node = el3.ref, ancestor = root.ref) shouldBe false + ChildNode.isDescendantOf(node = el3.ref, ancestor = rootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = el0.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = el11.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = el2.ref) shouldBe false - ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = otherEl.ref) shouldBe false el10.insertChild(el3, atIndex = 0) - ChildNode.isDescendantOf(node = el3.ref, ancestor = root.ref) shouldBe true + ChildNode.isDescendantOf(node = el3.ref, ancestor = rootNode.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el0.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el10.ref) shouldBe true ChildNode.isDescendantOf(node = el3.ref, ancestor = el11.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = el2.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = elx.ref) shouldBe false - ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRoot.ref) shouldBe false + ChildNode.isDescendantOf(node = el3.ref, ancestor = otherRootNode.ref) shouldBe false ChildNode.isDescendantOf(node = el3.ref, ancestor = otherEl.ref) shouldBe false } - it("ChildNode.isMounted") { - val container1 = div(alt := "foo").ref - val otherContainer = div(alt := "foo2").ref + it("ChildNode.isNodeMounted") { val el0 = span("el0") val el10 = span("el10") val el2 = article("el2") - val unmountedRoot = render(container1, el0) ChildNode.isNodeMounted(node = el0.ref) shouldBe false ChildNode.isNodeMounted(node = el10.ref) shouldBe false diff --git a/src/test/scala/com/raquo/laminar/fixtures/TestableOwner.scala b/src/test/scala/com/raquo/laminar/fixtures/TestableOwner.scala index 3eacf86f..9a6acfd4 100644 --- a/src/test/scala/com/raquo/laminar/fixtures/TestableOwner.scala +++ b/src/test/scala/com/raquo/laminar/fixtures/TestableOwner.scala @@ -1,15 +1,15 @@ package com.raquo.laminar.fixtures -import com.raquo.airstream.ownership.{Owned, Owner} +import com.raquo.airstream.ownership.{Subscription, Owner} import scala.scalajs.js // @TODO[Elegance] This duplicates a fixture defined in Airstream class TestableOwner extends Owner { - def _testPossessions: js.Array[Owned] = possessions + def _testSubscriptions: js.Array[Subscription] = subscriptions - override def killPossessions(): Unit = { - super.killPossessions() + override def killSubscriptions(): Unit = { + super.killSubscriptions() } } diff --git a/src/test/scala/com/raquo/laminar/utils/LaminarSpec.scala b/src/test/scala/com/raquo/laminar/utils/LaminarSpec.scala index a06693e4..a0442cc1 100644 --- a/src/test/scala/com/raquo/laminar/utils/LaminarSpec.scala +++ b/src/test/scala/com/raquo/laminar/utils/LaminarSpec.scala @@ -5,8 +5,7 @@ import com.raquo.domtestutils.{EventSimulator, MountOps} import com.raquo.domtypes.generic.keys.{HtmlAttr, SvgAttr} import com.raquo.laminar.api._ import com.raquo.laminar.keys.CompositeAttr -import com.raquo.laminar.nodes.{ChildNode, RootNode} -import org.scalajs.dom +import com.raquo.laminar.nodes.{ReactiveElement, RootNode} trait LaminarSpec extends MountOps @@ -21,7 +20,7 @@ trait LaminarSpec var root: RootNode = null def mount( - node: ChildNode[dom.Element], + node: ReactiveElement.Base, clue: String = defaultMountedElementClue ): Unit = { mountedElementClue = clue @@ -31,7 +30,7 @@ trait LaminarSpec def mount( clue: String, - node: ChildNode[dom.Element] + node: ReactiveElement.Base ): Unit = { mount(node, clue) } @@ -50,9 +49,20 @@ trait LaminarSpec root.unmount(), "ASSERT FAILED [laminar.unmount]: Laminar root failed to unmount" ) + root = null + // containerNode = null mountedElementClue = defaultMountedElementClue } +// @TODO I don't actually remember why I did this. Super's implementation seems functionally equivalent. Remove this if all is good. +// override def clearDOM(): Unit = { +// if (root != null) { +// unmount() +// } +// containerNode = null +// dom.document.body.textContent = "" // remove the container +// } + implicit def makeCompositeHtmlAttrTestable[V](attr: CompositeAttr[HtmlAttr[V]]): TestableHtmlAttr[V] = { new TestableHtmlAttr(attr.key) }