From 0201581a897ca0c5fa1a76823e9385f74019757c Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 10 Dec 2022 20:39:43 -0500 Subject: [PATCH 1/6] 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`. --- src/tests/cases/dom_test_case.ts | 32 +++++++++++++++++++++++-- src/tests/modules/core/outlet_tests.ts | 33 ++++++++++---------------- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/tests/cases/dom_test_case.ts b/src/tests/cases/dom_test_case.ts index 438239ef..abc04d04 100644 --- a/src/tests/cases/dom_test_case.ts +++ b/src/tests/cases/dom_test_case.ts @@ -60,8 +60,36 @@ export class DOMTestCase extends TestCase { return event } - findElement(selector: string) { - const element = this.fixtureElement.querySelector(selector) + 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(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) { return element } else { diff --git a/src/tests/modules/core/outlet_tests.ts b/src/tests/modules/core/outlet_tests.ts index 0ff686d8..1bbccfaa 100644 --- a/src/tests/modules/core/outlet_tests.ts +++ b/src/tests/modules/core/outlet_tests.ts @@ -149,13 +149,12 @@ export default class OutletTests extends ControllerTestCase(OutletController) { async "test outlet connected callback when element is inserted"() { const betaOutletElement = document.createElement("div") - betaOutletElement.setAttribute("class", "beta") - betaOutletElement.setAttribute("data-controller", "beta") + await this.setAttribute(betaOutletElement, "class", "beta") + await this.setAttribute(betaOutletElement, "data-controller", "beta") this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) - this.controller.element.appendChild(betaOutletElement) - await this.nextFrame + await this.appendChild(this.controller.element, betaOutletElement) this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 3) this.assert.ok( @@ -164,8 +163,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { ) this.assert.ok(betaOutletElement.isConnected, "element is present in document") - this.findElement("#container").appendChild(betaOutletElement.cloneNode(true)) - await this.nextFrame + await this.appendChild("#container", betaOutletElement.cloneNode(true)) this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 4) } @@ -175,9 +173,8 @@ export default class OutletTests extends ControllerTestCase(OutletController) { this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) - element.setAttribute("data-controller", "beta") - element.classList.add("beta") - await this.nextFrame + await this.setAttribute(element, "data-controller", "beta") + await this.setAttribute(element, "class", "beta") this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 3) this.assert.ok(element.classList.contains("connected"), `expected "${element.className}" to contain "connected"`) @@ -189,8 +186,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) - element.classList.add("beta") - await this.nextFrame + await this.setAttribute(element, "class", "beta") this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 3) this.assert.ok(element.classList.contains("connected"), `expected "${element.className}" to contain "connected"`) @@ -202,8 +198,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) - element.setAttribute(`data-controller`, "beta") - await this.nextFrame + await this.setAttribute(element, "data-controller", "beta") this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 3) this.assert.ok(element.classList.contains("connected"), `expected "${element.className}" to contain "connected"`) @@ -236,8 +231,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { `expected "${disconnectedAlpha.className}" not to contain "disconnected"` ) - disconnectedAlpha.parentElement?.removeChild(disconnectedAlpha) - await this.nextFrame + await this.remove(disconnectedAlpha) this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 1) this.assert.ok( @@ -256,8 +250,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { `expected "${disconnectedEpsilon.className}" not to contain "disconnected"` ) - disconnectedEpsilon.parentElement?.removeChild(disconnectedEpsilon) - await this.nextFrame + await this.remove(disconnectedEpsilon) this.assert.equal(this.controller.namespacedEpsilonOutletDisconnectedCallCountValue, 1) this.assert.ok( @@ -276,8 +269,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { `expected "${element.className}" not to contain "disconnected"` ) - element.removeAttribute(`id`) - await this.nextFrame + await this.removeAttribute(element, "id") this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 1) this.assert.ok( @@ -296,8 +288,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { `expected "${element.className}" not to contain "disconnected"` ) - element.removeAttribute(`data-controller`) - await this.nextFrame + await this.removeAttribute(element, "data-controller") this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 1) this.assert.ok( From 0c75ffdffa258c72ae85a5ff5ceaa0c1cdc535e0 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 10 Dec 2022 21:19:42 -0500 Subject: [PATCH 2/6] 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 `SelectorObserver` to also manage the lifecycle of an `AttributeObserver` instance alongside its internally managed `ElementObserver`. --- src/core/outlet_observer.ts | 12 +- src/mutation-observers/selector_observer.ts | 126 +++++++++++++----- src/tests/cases/dom_test_case.ts | 4 +- src/tests/controllers/outlet_controller.ts | 7 + src/tests/modules/core/outlet_tests.ts | 50 +++++++ .../selector_observer_tests.ts | 9 +- 6 files changed, 166 insertions(+), 42 deletions(-) diff --git a/src/core/outlet_observer.ts b/src/core/outlet_observer.ts index 0208129f..818db001 100644 --- a/src/core/outlet_observer.ts +++ b/src/core/outlet_observer.ts @@ -30,12 +30,12 @@ export class OutletObserver implements SelectorObserverDelegate { start() { if (this.selectorObserverMap.size === 0) { this.outletDefinitions.forEach((outletName) => { - const selector = this.selector(outletName) + const element = this.scope.element + const attributeName = this.outletAttributeFor(outletName) const details: SelectorObserverDetails = { outletName } + const selectorObserver = new SelectorObserver(element, attributeName, document.body, this, details) - if (selector) { - this.selectorObserverMap.set(outletName, new SelectorObserver(document.body, selector, this, details)) - } + this.selectorObserverMap.set(outletName, selectorObserver) }) this.selectorObserverMap.forEach((observer) => observer.start()) @@ -113,8 +113,8 @@ export class OutletObserver implements SelectorObserverDelegate { // Private - private selector(outletName: string) { - return this.scope.outlets.getSelectorForOutletName(outletName) + private outletAttributeFor(outletName: string) { + return this.context.schema.outletAttributeForScope(this.identifier, outletName) } private get outletDependencies() { diff --git a/src/mutation-observers/selector_observer.ts b/src/mutation-observers/selector_observer.ts index d18d09ec..2a3ff65d 100644 --- a/src/mutation-observers/selector_observer.ts +++ b/src/mutation-observers/selector_observer.ts @@ -1,3 +1,4 @@ +import { AttributeObserver, AttributeObserverDelegate } from "./attribute_observer" import { ElementObserver, ElementObserverDelegate } from "./element_observer" import { Multimap } from "../multimap" @@ -7,17 +8,25 @@ export interface SelectorObserverDelegate { selectorMatchElement?(element: Element, details: object): boolean } -export class SelectorObserver implements ElementObserverDelegate { - private selector: string - private elementObserver: ElementObserver - private delegate: SelectorObserverDelegate - private matchesByElement: Multimap - private details: object - - constructor(element: Element, selector: string, delegate: SelectorObserverDelegate, details: object = {}) { - this.selector = selector +export class SelectorObserver implements AttributeObserverDelegate, ElementObserverDelegate { + private readonly attributeObserver: AttributeObserver + private readonly elementObserver: ElementObserver + private readonly delegate: SelectorObserverDelegate + private readonly matchesByElement: Multimap + private readonly details: object + private selector: string | null + + constructor( + element: Element, + attributeName: string, + scope: Element, + delegate: SelectorObserverDelegate, + details: object + ) { this.details = details - this.elementObserver = new ElementObserver(element, this) + this.attributeObserver = new AttributeObserver(element, attributeName, this) + this.selector = element.getAttribute(this.attributeName) + this.elementObserver = new ElementObserver(scope, this) this.delegate = delegate this.matchesByElement = new Multimap() } @@ -28,68 +37,119 @@ export class SelectorObserver implements ElementObserverDelegate { start() { this.elementObserver.start() + this.attributeObserver.start() } pause(callback: () => void) { - this.elementObserver.pause(callback) + this.elementObserver.pause(() => this.attributeObserver.pause(callback)) } stop() { + this.attributeObserver.stop() this.elementObserver.stop() } refresh() { + this.attributeObserver.refresh() + this.elementObserver.refresh() + } + + // Attribute observer delegate + + elementMatchedAttribute(controllerElement: Element) { + this.selector = controllerElement.getAttribute(this.attributeName) + this.elementObserver.refresh() } - get element(): Element { - return this.elementObserver.element + elementUnmatchedAttribute() { + if (this.selector) { + const matchedElements = this.matchesByElement.getValuesForKey(this.selector) + + for (const matchedElement of matchedElements) { + this.selectorUnmatched(matchedElement, this.selector) + } + } + + this.selector = null + } + + elementAttributeValueChanged(controllerElement: Element) { + this.elementMatchedAttribute(controllerElement) } // 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 { selector } = this + + if (selector) { + 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 (selector) { + const matches = this.matchElement(element) + const matchedBefore = this.matchesByElement.has(selector, element) - if (!matches && matchedBefore) { - this.selectorUnmatched(element) + if (!matches && matchedBefore) { + this.selectorUnmatched(element, selector) + } } } - private selectorMatched(element: Element) { + private selectorMatched(element: Element, selector: string) { if (this.delegate.selectorMatched) { - this.delegate.selectorMatched(element, this.selector, this.details) - this.matchesByElement.add(this.selector, element) + 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) + } + + private get attributeName() { + return this.attributeObserver.attributeName } } diff --git a/src/tests/cases/dom_test_case.ts b/src/tests/cases/dom_test_case.ts index abc04d04..b028b2f7 100644 --- a/src/tests/cases/dom_test_case.ts +++ b/src/tests/cases/dom_test_case.ts @@ -88,8 +88,8 @@ export class DOMTestCase extends TestCase { await this.nextFrame } - findElement(selector: string) { - const element = this.fixtureElement.querySelector(selector) + findElement(selector: string) { + const element = this.fixtureElement.querySelector(selector) if (element) { return element } else { diff --git a/src/tests/controllers/outlet_controller.ts b/src/tests/controllers/outlet_controller.ts index c3a5b840..4c5088ea 100644 --- a/src/tests/controllers/outlet_controller.ts +++ b/src/tests/controllers/outlet_controller.ts @@ -19,6 +19,7 @@ export class OutletController extends BaseOutletController { alphaOutletDisconnectedCallCount: Number, betaOutletConnectedCallCount: Number, betaOutletDisconnectedCallCount: Number, + gammaOutletConnectedCallCount: Number, namespacedEpsilonOutletConnectedCallCount: Number, namespacedEpsilonOutletDisconnectedCallCount: Number, } @@ -44,6 +45,7 @@ export class OutletController extends BaseOutletController { alphaOutletDisconnectedCallCountValue = 0 betaOutletConnectedCallCountValue = 0 betaOutletDisconnectedCallCountValue = 0 + gammaOutletConnectedCallCountValue = 0 namespacedEpsilonOutletConnectedCallCountValue = 0 namespacedEpsilonOutletDisconnectedCallCountValue = 0 @@ -67,6 +69,11 @@ export class OutletController extends BaseOutletController { this.betaOutletDisconnectedCallCountValue++ } + gammaOutletConnected(_outlet: Controller, element: Element) { + if (this.hasConnectedClass) element.classList.add(this.connectedClass) + this.gammaOutletConnectedCallCountValue++ + } + namespacedEpsilonOutletConnected(_outlet: Controller, element: Element) { if (this.hasConnectedClass) element.classList.add(this.connectedClass) this.namespacedEpsilonOutletConnectedCallCountValue++ diff --git a/src/tests/modules/core/outlet_tests.ts b/src/tests/modules/core/outlet_tests.ts index 1bbccfaa..9ddcfffc 100644 --- a/src/tests/modules/core/outlet_tests.ts +++ b/src/tests/modules/core/outlet_tests.ts @@ -297,4 +297,54 @@ export default class OutletTests extends ControllerTestCase(OutletController) { ) this.assert.ok(element.isConnected, "element is still present in document") } + + async "test outlet connect callback when the controlled element's outlet attribute is added"() { + const gamma2 = this.findElement("#gamma2") + + await this.setAttribute(this.controller.element, `data-${this.identifier}-gamma-outlet`, "#gamma2") + + this.assert.equal(this.controller.gammaOutletConnectedCallCountValue, 1) + this.assert.ok(gamma2.isConnected, "#gamma2 is still present in document") + this.assert.ok(gamma2.classList.contains("connected"), `expected "${gamma2.className}" to contain "connected"`) + } + + async "test outlet connect callback when the controlled element's outlet attribute is changed"() { + const alpha1 = this.findElement("#alpha1") + const alpha2 = this.findElement("#alpha2") + + await this.setAttribute(this.controller.element, `data-${this.identifier}-alpha-outlet`, "#alpha1") + + this.assert.equal(this.controller.alphaOutletConnectedCallCountValue, 2) + this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 1) + this.assert.ok(alpha1.isConnected, "alpha1 is still present in document") + this.assert.ok(alpha2.isConnected, "alpha2 is still present in document") + this.assert.ok(alpha1.classList.contains("connected"), `expected "${alpha1.className}" to contain "connected"`) + this.assert.notOk( + alpha1.classList.contains("disconnected"), + `expected "${alpha1.className}" to contain "disconnected"` + ) + this.assert.ok( + alpha2.classList.contains("disconnected"), + `expected "${alpha2.className}" to contain "disconnected"` + ) + } + + async "test outlet disconnected callback when the controlled element's outlet attribute is removed"() { + const alpha1 = this.findElement("#alpha1") + const alpha2 = this.findElement("#alpha2") + + await this.removeAttribute(this.controller.element, `data-${this.identifier}-alpha-outlet`) + + this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 2) + this.assert.ok(alpha1.isConnected, "#alpha1 is still present in document") + this.assert.ok(alpha2.isConnected, "#alpha2 is still present in document") + this.assert.ok( + alpha1.classList.contains("disconnected"), + `expected "${alpha1.className}" to contain "disconnected"` + ) + this.assert.ok( + alpha2.classList.contains("disconnected"), + `expected "${alpha2.className}" to contain "disconnected"` + ) + } } diff --git a/src/tests/modules/mutation-observers/selector_observer_tests.ts b/src/tests/modules/mutation-observers/selector_observer_tests.ts index 2e29b71d..aac8ec3e 100644 --- a/src/tests/modules/mutation-observers/selector_observer_tests.ts +++ b/src/tests/modules/mutation-observers/selector_observer_tests.ts @@ -2,6 +2,7 @@ import { SelectorObserver, SelectorObserverDelegate } from "../../../mutation-ob import { ObserverTestCase } from "../../cases/observer_test_case" export default class SelectorObserverTests extends ObserverTestCase implements SelectorObserverDelegate { + selectorAttribute = "data-selector" attributeName = "data-test" selector = "div[data-test~=two]" details = { some: "details" } @@ -14,7 +15,13 @@ export default class SelectorObserverTests extends ObserverTestCase implements S ` - observer = new SelectorObserver(this.fixtureElement, this.selector, this, this.details) + + observer = new SelectorObserver(this.fixtureElement, this.selectorAttribute, this.fixtureElement, this, this.details) + + async setup() { + await this.setAttribute(this.fixtureElement, this.selectorAttribute, this.selector) + await super.setup() + } async "test should match when observer starts"() { this.assert.deepEqual(this.calls, [ From 492555a143c75f8e83000edb94097c6f131bab68 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 22 Dec 2022 12:59:04 -0500 Subject: [PATCH 3/6] rename `scope` to `controllerElement` --- src/mutation-observers/selector_observer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mutation-observers/selector_observer.ts b/src/mutation-observers/selector_observer.ts index 2a3ff65d..4ca5d317 100644 --- a/src/mutation-observers/selector_observer.ts +++ b/src/mutation-observers/selector_observer.ts @@ -19,14 +19,14 @@ export class SelectorObserver implements AttributeObserverDelegate, ElementObser constructor( element: Element, attributeName: string, - scope: Element, + controllerElement: Element, delegate: SelectorObserverDelegate, details: object ) { this.details = details this.attributeObserver = new AttributeObserver(element, attributeName, this) this.selector = element.getAttribute(this.attributeName) - this.elementObserver = new ElementObserver(scope, this) + this.elementObserver = new ElementObserver(controllerElement, this) this.delegate = delegate this.matchesByElement = new Multimap() } From 971004a94e95a2edf8be5f7cc64f5cd9e4977f5d Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 22 Dec 2022 12:59:52 -0500 Subject: [PATCH 4/6] destructur `selector` from `this` --- src/mutation-observers/selector_observer.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/mutation-observers/selector_observer.ts b/src/mutation-observers/selector_observer.ts index 4ca5d317..9a75bf83 100644 --- a/src/mutation-observers/selector_observer.ts +++ b/src/mutation-observers/selector_observer.ts @@ -63,11 +63,13 @@ export class SelectorObserver implements AttributeObserverDelegate, ElementObser } elementUnmatchedAttribute() { - if (this.selector) { - const matchedElements = this.matchesByElement.getValuesForKey(this.selector) + const { selector } = this + + if (selector) { + const matchedElements = this.matchesByElement.getValuesForKey(selector) for (const matchedElement of matchedElements) { - this.selectorUnmatched(matchedElement, this.selector) + this.selectorUnmatched(matchedElement, selector) } } From 4ff1250a04685ca9471f38cb37e3a06e750c1bf9 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Mon, 30 Jan 2023 04:05:53 +0100 Subject: [PATCH 5/6] Move `AttributeObserver` from `SelectorObserver` to `OutletObserver` --- src/core/outlet_observer.ts | 134 ++++++++++++++---- src/mutation-observers/element_observer.ts | 7 +- src/mutation-observers/selector_observer.ts | 81 ++++------- src/tests/controllers/outlet_controller.ts | 2 + src/tests/modules/core/outlet_tests.ts | 21 ++- .../selector_observer_tests.ts | 9 +- 6 files changed, 163 insertions(+), 91 deletions(-) diff --git a/src/core/outlet_observer.ts b/src/core/outlet_observer.ts index 818db001..b0956f47 100644 --- a/src/core/outlet_observer.ts +++ b/src/core/outlet_observer.ts @@ -1,64 +1,79 @@ 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 readonly outletElementsByName: Multimap private selectorObserverMap: Map + private attributeObserverMap: Map 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 element = this.scope.element - const attributeName = this.outletAttributeFor(outletName) - const details: SelectorObserverDetails = { outletName } - const selectorObserver = new SelectorObserver(element, attributeName, document.body, this, details) - - this.selectorObserverMap.set(outletName, selectorObserver) + 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) { @@ -66,7 +81,7 @@ export class OutletObserver implements SelectorObserverDelegate { } } - selectorUnmatched(element: Element, _selector: string, { outletName }: SelectorObserverDetails) { + selectorUnmatched(element: Element, _selector: string, { outletName }: OutletObserverDetails) { const outlet = this.getOutletFromMap(element, outletName) if (outlet) { @@ -74,11 +89,42 @@ export class OutletObserver implements SelectorObserverDelegate { } } - 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,10 +157,46 @@ 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 outletAttributeFor(outletName: string) { - return this.context.schema.outletAttributeForScope(this.identifier, outletName) + 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() { @@ -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 } diff --git a/src/mutation-observers/element_observer.ts b/src/mutation-observers/element_observer.ts index 2902cd00..544de6e4 100644 --- a/src/mutation-observers/element_observer.ts +++ b/src/mutation-observers/element_observer.ts @@ -14,7 +14,7 @@ export class ElementObserver { private elements: Set private mutationObserver: MutationObserver - private mutationObserverInit = { attributes: true, childList: true, subtree: true } + private mutationObserverInit: MutationObserverInit = { attributes: true, childList: true, subtree: true } constructor(element: Element, delegate: ElementObserverDelegate) { this.element = element @@ -83,15 +83,14 @@ export class ElementObserver { private processMutation(mutation: MutationRecord) { if (mutation.type == "attributes") { - this.processAttributeChange(mutation.target, mutation.attributeName!) + this.processAttributeChange(mutation.target as Element, mutation.attributeName!) } else if (mutation.type == "childList") { this.processRemovedNodes(mutation.removedNodes) this.processAddedNodes(mutation.addedNodes) } } - private processAttributeChange(node: Node, attributeName: string) { - const element = node as Element + private processAttributeChange(element: Element, attributeName: string) { if (this.elements.has(element)) { if (this.delegate.elementAttributeChanged && this.matchElement(element)) { this.delegate.elementAttributeChanged(element, attributeName) diff --git a/src/mutation-observers/selector_observer.ts b/src/mutation-observers/selector_observer.ts index 9a75bf83..6321de64 100644 --- a/src/mutation-observers/selector_observer.ts +++ b/src/mutation-observers/selector_observer.ts @@ -1,4 +1,3 @@ -import { AttributeObserver, AttributeObserverDelegate } from "./attribute_observer" import { ElementObserver, ElementObserverDelegate } from "./element_observer" import { Multimap } from "../multimap" @@ -8,25 +7,17 @@ export interface SelectorObserverDelegate { selectorMatchElement?(element: Element, details: object): boolean } -export class SelectorObserver implements AttributeObserverDelegate, ElementObserverDelegate { - private readonly attributeObserver: AttributeObserver +export class SelectorObserver implements ElementObserverDelegate { private readonly elementObserver: ElementObserver private readonly delegate: SelectorObserverDelegate private readonly matchesByElement: Multimap private readonly details: object - private selector: string | null - - constructor( - element: Element, - attributeName: string, - controllerElement: Element, - delegate: SelectorObserverDelegate, - details: object - ) { + _selector: string | null + + constructor(element: Element, selector: string, delegate: SelectorObserverDelegate, details: object) { + this._selector = selector this.details = details - this.attributeObserver = new AttributeObserver(element, attributeName, this) - this.selector = element.getAttribute(this.attributeName) - this.elementObserver = new ElementObserver(controllerElement, this) + this.elementObserver = new ElementObserver(element, this) this.delegate = delegate this.matchesByElement = new Multimap() } @@ -35,49 +26,33 @@ export class SelectorObserver implements AttributeObserverDelegate, ElementObser return this.elementObserver.started } + get selector() { + return this._selector + } + + set selector(selector: string | null) { + this._selector = selector + this.refresh() + } + start() { this.elementObserver.start() - this.attributeObserver.start() } pause(callback: () => void) { - this.elementObserver.pause(() => this.attributeObserver.pause(callback)) + this.elementObserver.pause(callback) } stop() { - this.attributeObserver.stop() this.elementObserver.stop() } refresh() { - this.attributeObserver.refresh() this.elementObserver.refresh() } - // Attribute observer delegate - - elementMatchedAttribute(controllerElement: Element) { - this.selector = controllerElement.getAttribute(this.attributeName) - - this.elementObserver.refresh() - } - - elementUnmatchedAttribute() { - const { selector } = this - - if (selector) { - const matchedElements = this.matchesByElement.getValuesForKey(selector) - - for (const matchedElement of matchedElements) { - this.selectorUnmatched(matchedElement, selector) - } - } - - this.selector = null - } - - elementAttributeValueChanged(controllerElement: Element) { - this.elementMatchedAttribute(controllerElement) + get element(): Element { + return this.elementObserver.element } // Element observer delegate @@ -119,9 +94,9 @@ export class SelectorObserver implements AttributeObserverDelegate, ElementObser } elementUnmatched(element: Element) { - const { selector } = this + const selectors = this.matchesByElement.getKeysForValue(element) - if (selector) { + for (const selector of selectors) { this.selectorUnmatched(element, selector) } } @@ -133,25 +108,23 @@ export class SelectorObserver implements AttributeObserverDelegate, ElementObser const matches = this.matchElement(element) const matchedBefore = this.matchesByElement.has(selector, element) - if (!matches && matchedBefore) { + if (matches && !matchedBefore) { + this.selectorMatched(element, selector) + } else if (!matches && matchedBefore) { this.selectorUnmatched(element, selector) } } } + // Selector management + private selectorMatched(element: Element, selector: string) { - if (this.delegate.selectorMatched) { - this.delegate.selectorMatched(element, selector, this.details) - this.matchesByElement.add(selector, element) - } + this.delegate.selectorMatched(element, selector, this.details) + this.matchesByElement.add(selector, element) } private selectorUnmatched(element: Element, selector: string) { this.delegate.selectorUnmatched(element, selector, this.details) this.matchesByElement.delete(selector, element) } - - private get attributeName() { - return this.attributeObserver.attributeName - } } diff --git a/src/tests/controllers/outlet_controller.ts b/src/tests/controllers/outlet_controller.ts index 4c5088ea..261a4734 100644 --- a/src/tests/controllers/outlet_controller.ts +++ b/src/tests/controllers/outlet_controller.ts @@ -20,6 +20,7 @@ export class OutletController extends BaseOutletController { betaOutletConnectedCallCount: Number, betaOutletDisconnectedCallCount: Number, gammaOutletConnectedCallCount: Number, + gammaOutletDisconnectedCallCount: Number, namespacedEpsilonOutletConnectedCallCount: Number, namespacedEpsilonOutletDisconnectedCallCount: Number, } @@ -46,6 +47,7 @@ export class OutletController extends BaseOutletController { betaOutletConnectedCallCountValue = 0 betaOutletDisconnectedCallCountValue = 0 gammaOutletConnectedCallCountValue = 0 + gammaOutletDisconnectedCallCountValue = 0 namespacedEpsilonOutletConnectedCallCountValue = 0 namespacedEpsilonOutletDisconnectedCallCountValue = 0 diff --git a/src/tests/modules/core/outlet_tests.ts b/src/tests/modules/core/outlet_tests.ts index 9ddcfffc..fb87ba12 100644 --- a/src/tests/modules/core/outlet_tests.ts +++ b/src/tests/modules/core/outlet_tests.ts @@ -132,7 +132,7 @@ export default class OutletTests extends ControllerTestCase(OutletController) { this.assert.throws(() => this.controller.alphaOutletElement) } - "test outlet connected callback fires"() { + async "test outlet connected callback fires"() { const alphaOutlets = this.controller.alphaOutletElements.filter((outlet) => outlet.classList.contains("connected")) this.assert.equal(alphaOutlets.length, 2) @@ -308,6 +308,25 @@ export default class OutletTests extends ControllerTestCase(OutletController) { this.assert.ok(gamma2.classList.contains("connected"), `expected "${gamma2.className}" to contain "connected"`) } + async "test outlet connect callback doesn't get trigged when any attribute gets added to the controller element"() { + this.assert.equal(this.controller.alphaOutletConnectedCallCountValue, 2) + this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) + this.assert.equal(this.controller.gammaOutletConnectedCallCountValue, 0) + this.assert.equal(this.controller.namespacedEpsilonOutletConnectedCallCountValue, 2) + + await this.setAttribute(this.controller.element, "data-some-random-attribute", "#alpha1") + + this.assert.equal(this.controller.alphaOutletConnectedCallCountValue, 2) + this.assert.equal(this.controller.betaOutletConnectedCallCountValue, 2) + this.assert.equal(this.controller.gammaOutletConnectedCallCountValue, 0) + this.assert.equal(this.controller.namespacedEpsilonOutletConnectedCallCountValue, 2) + + this.assert.equal(this.controller.alphaOutletDisconnectedCallCountValue, 0) + this.assert.equal(this.controller.betaOutletDisconnectedCallCountValue, 0) + this.assert.equal(this.controller.gammaOutletDisconnectedCallCountValue, 0) + this.assert.equal(this.controller.namespacedEpsilonOutletDisconnectedCallCountValue, 0) + } + async "test outlet connect callback when the controlled element's outlet attribute is changed"() { const alpha1 = this.findElement("#alpha1") const alpha2 = this.findElement("#alpha2") diff --git a/src/tests/modules/mutation-observers/selector_observer_tests.ts b/src/tests/modules/mutation-observers/selector_observer_tests.ts index aac8ec3e..2e29b71d 100644 --- a/src/tests/modules/mutation-observers/selector_observer_tests.ts +++ b/src/tests/modules/mutation-observers/selector_observer_tests.ts @@ -2,7 +2,6 @@ import { SelectorObserver, SelectorObserverDelegate } from "../../../mutation-ob import { ObserverTestCase } from "../../cases/observer_test_case" export default class SelectorObserverTests extends ObserverTestCase implements SelectorObserverDelegate { - selectorAttribute = "data-selector" attributeName = "data-test" selector = "div[data-test~=two]" details = { some: "details" } @@ -15,13 +14,7 @@ export default class SelectorObserverTests extends ObserverTestCase implements S ` - - observer = new SelectorObserver(this.fixtureElement, this.selectorAttribute, this.fixtureElement, this, this.details) - - async setup() { - await this.setAttribute(this.fixtureElement, this.selectorAttribute, this.selector) - await super.setup() - } + observer = new SelectorObserver(this.fixtureElement, this.selector, this, this.details) async "test should match when observer starts"() { this.assert.deepEqual(this.calls, [ From 700c67bc2cd7b2412800c4ecf89c1826026573c4 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Mon, 30 Jan 2023 04:12:16 +0100 Subject: [PATCH 6/6] remove unrelated changes in `ElementObserver` --- src/mutation-observers/element_observer.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/mutation-observers/element_observer.ts b/src/mutation-observers/element_observer.ts index 544de6e4..2902cd00 100644 --- a/src/mutation-observers/element_observer.ts +++ b/src/mutation-observers/element_observer.ts @@ -14,7 +14,7 @@ export class ElementObserver { private elements: Set private mutationObserver: MutationObserver - private mutationObserverInit: MutationObserverInit = { attributes: true, childList: true, subtree: true } + private mutationObserverInit = { attributes: true, childList: true, subtree: true } constructor(element: Element, delegate: ElementObserverDelegate) { this.element = element @@ -83,14 +83,15 @@ export class ElementObserver { private processMutation(mutation: MutationRecord) { if (mutation.type == "attributes") { - this.processAttributeChange(mutation.target as Element, mutation.attributeName!) + this.processAttributeChange(mutation.target, mutation.attributeName!) } else if (mutation.type == "childList") { this.processRemovedNodes(mutation.removedNodes) this.processAddedNodes(mutation.addedNodes) } } - private processAttributeChange(element: Element, attributeName: string) { + private processAttributeChange(node: Node, attributeName: string) { + const element = node as Element if (this.elements.has(element)) { if (this.delegate.elementAttributeChanged && this.matchElement(element)) { this.delegate.elementAttributeChanged(element, attributeName)