Outlets: Add observers for controller element attributes (#624)

* Extract `DomTestCase` helpers

In some cases, calls to `Element.setAttribute`,
`Element.removeAttribute`, `Node.appendChild`, or `Element.remove` must
be followed up with a call to `await this.nextFrame` so that Stimulus
has an opportunity to synchronize.

This commit introduces asynchronous helper method versions of those
calls that bake-in the subsequent call to `this.nextFrame`.

* Outlets: Add observers for controller element attributes

With the current Outlet implementation, they're only ever connected or
disconnected when an element _matching_ the outlet selector connects or
disconnects. The selector _declared on the controller_ element can only
ever be set once: when it's connected. If that attribute ever changes
(for example, when it's set to a new value or removed entirely), the
outlets are not updated.

This commit adds test coverage to ensure the list of outlets and their
items is synchronized with changes on both sides: when matching elements
are connected and disconnected _as well as_ when the selector that
dictates whether or not they match is added, updated, or removed.

To do so, this commit extends the `OutletObserver` to also manage the
lifecycle of an `AttributeObserver` instance alongside its internally
managed `SelectorObserver`.

---------

Co-authored-by: Marco Roth <[email protected]>

src/core/outlet_observer.ts

@@ -1,84 +1,130 @@
 import { Multimap } from "../multimap"
+import { AttributeObserver, AttributeObserverDelegate } from "../mutation-observers"
 import { SelectorObserver, SelectorObserverDelegate } from "../mutation-observers"
 import { Context } from "./context"
 import { Controller } from "./controller"
 
 import { readInheritableStaticArrayValues } from "./inheritable_statics"
 
-type SelectorObserverDetails = { outletName: string }
+type OutletObserverDetails = { outletName: string }
 
 export interface OutletObserverDelegate {
   outletConnected(outlet: Controller, element: Element, outletName: string): void
   outletDisconnected(outlet: Controller, element: Element, outletName: string): void
 }
 
-export class OutletObserver implements SelectorObserverDelegate {
+export class OutletObserver implements AttributeObserverDelegate, SelectorObserverDelegate {
+  started: boolean
   readonly context: Context
   readonly delegate: OutletObserverDelegate
   readonly outletsByName: Multimap<string, Controller>
   readonly outletElementsByName: Multimap<string, Element>
   private selectorObserverMap: Map<string, SelectorObserver>
+  private attributeObserverMap: Map<string, AttributeObserver>
 
   constructor(context: Context, delegate: OutletObserverDelegate) {
+    this.started = false
     this.context = context
     this.delegate = delegate
     this.outletsByName = new Multimap()
     this.outletElementsByName = new Multimap()
     this.selectorObserverMap = new Map()
+    this.attributeObserverMap = new Map()
   }
 
   start() {
-    if (this.selectorObserverMap.size === 0) {
+    if (!this.started) {
       this.outletDefinitions.forEach((outletName) => {
-        const selector = this.selector(outletName)
-        const details: SelectorObserverDetails = { outletName }
-
-        if (selector) {
-          this.selectorObserverMap.set(outletName, new SelectorObserver(document.body, selector, this, details))
-        }
+        this.setupSelectorObserverForOutlet(outletName)
+        this.setupAttributeObserverForOutlet(outletName)
       })
-
-      this.selectorObserverMap.forEach((observer) => observer.start())
+      this.started = true
+      this.dependentContexts.forEach((context) => context.refresh())
     }
+  }
 
-    this.dependentContexts.forEach((context) => context.refresh())
+  refresh() {
+    this.selectorObserverMap.forEach((observer) => observer.refresh())
+    this.attributeObserverMap.forEach((observer) => observer.refresh())
   }
 
   stop() {
-    if (this.selectorObserverMap.size > 0) {
+    if (this.started) {
+      this.started = false
       this.disconnectAllOutlets()
+      this.stopSelectorObservers()
+      this.stopAttributeObservers()
+    }
+  }
+
+  stopSelectorObservers() {
+    if (this.selectorObserverMap.size > 0) {
       this.selectorObserverMap.forEach((observer) => observer.stop())
       this.selectorObserverMap.clear()
     }
   }
 
-  refresh() {
-    this.selectorObserverMap.forEach((observer) => observer.refresh())
+  stopAttributeObservers() {
+    if (this.attributeObserverMap.size > 0) {
+      this.attributeObserverMap.forEach((observer) => observer.stop())
+      this.attributeObserverMap.clear()
+    }
   }
 
   // Selector observer delegate
 
-  selectorMatched(element: Element, _selector: string, { outletName }: SelectorObserverDetails) {
+  selectorMatched(element: Element, _selector: string, { outletName }: OutletObserverDetails) {
     const outlet = this.getOutlet(element, outletName)
 
     if (outlet) {
       this.connectOutlet(outlet, element, outletName)
     }
   }
 
-  selectorUnmatched(element: Element, _selector: string, { outletName }: SelectorObserverDetails) {
+  selectorUnmatched(element: Element, _selector: string, { outletName }: OutletObserverDetails) {
     const outlet = this.getOutletFromMap(element, outletName)
 
     if (outlet) {
       this.disconnectOutlet(outlet, element, outletName)
     }
   }
 
-  selectorMatchElement(element: Element, { outletName }: SelectorObserverDetails) {
-    return (
-      this.hasOutlet(element, outletName) &&
-      element.matches(`[${this.context.application.schema.controllerAttribute}~=${outletName}]`)
-    )
+  selectorMatchElement(element: Element, { outletName }: OutletObserverDetails) {
+    const selector = this.selector(outletName)
+    const hasOutlet = this.hasOutlet(element, outletName)
+    const hasOutletController = element.matches(`[${this.schema.controllerAttribute}~=${outletName}]`)
+
+    if (selector) {
+      return hasOutlet && hasOutletController && element.matches(selector)
+    } else {
+      return false
+    }
+  }
+
+  // Attribute observer delegate
+
+  elementMatchedAttribute(_element: Element, attributeName: string) {
+    const outletName = this.getOutletNameFromOutletAttributeName(attributeName)
+
+    if (outletName) {
+      this.updateSelectorObserverForOutlet(outletName)
+    }
+  }
+
+  elementAttributeValueChanged(_element: Element, attributeName: string) {
+    const outletName = this.getOutletNameFromOutletAttributeName(attributeName)
+
+    if (outletName) {
+      this.updateSelectorObserverForOutlet(outletName)
+    }
+  }
+
+  elementUnmatchedAttribute(_element: Element, attributeName: string) {
+    const outletName = this.getOutletNameFromOutletAttributeName(attributeName)
+
+    if (outletName) {
+      this.updateSelectorObserverForOutlet(outletName)
+    }
   }
 
   // Outlet management
@@ -111,12 +157,48 @@ export class OutletObserver implements SelectorObserverDelegate {
     }
   }
 
+  // Observer management
+
+  private updateSelectorObserverForOutlet(outletName: string) {
+    const observer = this.selectorObserverMap.get(outletName)
+
+    if (observer) {
+      observer.selector = this.selector(outletName)
+    }
+  }
+
+  private setupSelectorObserverForOutlet(outletName: string) {
+    const selector = this.selector(outletName)
+    const selectorObserver = new SelectorObserver(document.body, selector!, this, { outletName })
+
+    this.selectorObserverMap.set(outletName, selectorObserver)
+
+    selectorObserver.start()
+  }
+
+  private setupAttributeObserverForOutlet(outletName: string) {
+    const attributeName = this.attributeNameForOutletName(outletName)
+    const attributeObserver = new AttributeObserver(this.scope.element, attributeName, this)
+
+    this.attributeObserverMap.set(outletName, attributeObserver)
+
+    attributeObserver.start()
+  }
+
   // Private
 
   private selector(outletName: string) {
     return this.scope.outlets.getSelectorForOutletName(outletName)
   }
 
+  private attributeNameForOutletName(outletName: string) {
+    return this.scope.schema.outletAttributeForScope(this.identifier, outletName)
+  }
+
+  private getOutletNameFromOutletAttributeName(attributeName: string) {
+    return this.outletDefinitions.find((outletName) => this.attributeNameForOutletName(outletName) === attributeName)
+  }
+
   private get outletDependencies() {
     const dependencies = new Multimap<string, string>()
 
@@ -159,6 +241,10 @@ export class OutletObserver implements SelectorObserverDelegate {
     return this.context.scope
   }
 
+  private get schema() {
+    return this.context.schema
+  }
+
   private get identifier() {
     return this.context.identifier
   }

src/mutation-observers/selector_observer.ts

@@ -8,14 +8,14 @@ export interface SelectorObserverDelegate {
 }
 
 export class SelectorObserver implements ElementObserverDelegate {
-  private selector: string
-  private elementObserver: ElementObserver
-  private delegate: SelectorObserverDelegate
-  private matchesByElement: Multimap<string, Element>
-  private details: object
-
-  constructor(element: Element, selector: string, delegate: SelectorObserverDelegate, details: object = {}) {
-    this.selector = selector
+  private readonly elementObserver: ElementObserver
+  private readonly delegate: SelectorObserverDelegate
+  private readonly matchesByElement: Multimap<string, Element>
+  private readonly details: object
+  _selector: string | null
+
+  constructor(element: Element, selector: string, delegate: SelectorObserverDelegate, details: object) {
+    this._selector = selector
     this.details = details
     this.elementObserver = new ElementObserver(element, this)
     this.delegate = delegate
@@ -26,6 +26,15 @@ export class SelectorObserver implements ElementObserverDelegate {
     return this.elementObserver.started
   }
 
+  get selector() {
+    return this._selector
+  }
+
+  set selector(selector: string | null) {
+    this._selector = selector
+    this.refresh()
+  }
+
   start() {
     this.elementObserver.start()
   }
@@ -49,47 +58,73 @@ export class SelectorObserver implements ElementObserverDelegate {
   // Element observer delegate
 
   matchElement(element: Element): boolean {
-    const matches = element.matches(this.selector)
+    const { selector } = this
 
-    if (this.delegate.selectorMatchElement) {
-      return matches && this.delegate.selectorMatchElement(element, this.details)
-    }
+    if (selector) {
+      const matches = element.matches(selector)
+
+      if (this.delegate.selectorMatchElement) {
+        return matches && this.delegate.selectorMatchElement(element, this.details)
+      }
 
-    return matches
+      return matches
+    } else {
+      return false
+    }
   }
 
   matchElementsInTree(tree: Element): Element[] {
-    const match = this.matchElement(tree) ? [tree] : []
-    const matches = Array.from(tree.querySelectorAll(this.selector)).filter((match) => this.matchElement(match))
-    return match.concat(matches)
+    const { selector } = this
+
+    if (selector) {
+      const match = this.matchElement(tree) ? [tree] : []
+      const matches = Array.from(tree.querySelectorAll(selector)).filter((match) => this.matchElement(match))
+      return match.concat(matches)
+    } else {
+      return []
+    }
   }
 
   elementMatched(element: Element) {
-    this.selectorMatched(element)
+    const { selector } = this
+
+    if (selector) {
+      this.selectorMatched(element, selector)
+    }
   }
 
   elementUnmatched(element: Element) {
-    this.selectorUnmatched(element)
+    const selectors = this.matchesByElement.getKeysForValue(element)
+
+    for (const selector of selectors) {
+      this.selectorUnmatched(element, selector)
+    }
   }
 
   elementAttributeChanged(element: Element, _attributeName: string) {
-    const matches = this.matchElement(element)
-    const matchedBefore = this.matchesByElement.has(this.selector, element)
+    const { selector } = this
 
-    if (!matches && matchedBefore) {
-      this.selectorUnmatched(element)
+    if (selector) {
+      const matches = this.matchElement(element)
+      const matchedBefore = this.matchesByElement.has(selector, element)
+
+      if (matches && !matchedBefore) {
+        this.selectorMatched(element, selector)
+      } else if (!matches && matchedBefore) {
+        this.selectorUnmatched(element, selector)
+      }
     }
   }
 
-  private selectorMatched(element: Element) {
-    if (this.delegate.selectorMatched) {
-      this.delegate.selectorMatched(element, this.selector, this.details)
-      this.matchesByElement.add(this.selector, element)
-    }
+  // Selector management
+
+  private selectorMatched(element: Element, selector: string) {
+    this.delegate.selectorMatched(element, selector, this.details)
+    this.matchesByElement.add(selector, element)
   }
 
-  private selectorUnmatched(element: Element) {
-    this.delegate.selectorUnmatched(element, this.selector, this.details)
-    this.matchesByElement.delete(this.selector, element)
+  private selectorUnmatched(element: Element, selector: string) {
+    this.delegate.selectorUnmatched(element, selector, this.details)
+    this.matchesByElement.delete(selector, element)
   }
 }

src/tests/cases/dom_test_case.ts

@@ -60,6 +60,34 @@ export class DOMTestCase extends TestCase {
     return event
   }
 
+  async setAttribute(selectorOrElement: string | Element, name: string, value: string) {
+    const element = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement
+
+    element.setAttribute(name, value)
+    await this.nextFrame
+  }
+
+  async removeAttribute(selectorOrElement: string | Element, name: string) {
+    const element = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement
+
+    element.removeAttribute(name)
+    await this.nextFrame
+  }
+
+  async appendChild<T extends Node>(selectorOrElement: T | string, child: T) {
+    const parent = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement
+
+    parent.appendChild(child)
+    await this.nextFrame
+  }
+
+  async remove(selectorOrElement: Element | string) {
+    const element = typeof selectorOrElement == "string" ? this.findElement(selectorOrElement) : selectorOrElement
+
+    element.remove()
+    await this.nextFrame
+  }
+
   findElement(selector: string) {
     const element = this.fixtureElement.querySelector(selector)
     if (element) {