Component: attached handles are called top-down (ancestor → descendant) (BC break)

Implementation handles tree mutations during listener execution:
- Listeners can modify tree (remove self, siblings, parent)
- Validity check before processing children
- Deduplication prevents calling same listener twice
- Reentry guard prevents infinite loops

Author: dg

phpstan.neon

@@ -14,7 +14,7 @@ parameters:
 		- # complex tuple type of $monitors is hard to express precisely for all mutation paths
 			message: '#^Property Nette\\ComponentModel\\Component\:\:\$monitors .+ does not accept .+\.$#'
 			identifier: assign.propertyType
-			count: 5
+			count: 4
 			path: src/ComponentModel/Component.php
 
 		- # IContainer::getComponent() has optional $throw parameter in implementation

src/ComponentModel/Component.php

@@ -28,9 +28,6 @@ abstract class Component implements IComponent
 	 */
 	private array $monitors = [];
 
-	/** Prevents nested listener execution during refreshMonitors */
-	private bool $callingListeners = false;
-
 
 	/**
 	 * Finds the closest ancestor of specified type.
@@ -201,33 +198,19 @@ protected function validateParent(IContainer $parent): void
 	/**
 	 * Refreshes monitors when attaching/detaching from component tree.
 	 * @param  ?array<string, true>  $missing  null = detaching, array = attaching
-	 * @param  array<int, array{\Closure(IComponent): void, IComponent}>  $listeners
+	 * @param  array<array{\Closure(IComponent): void, int}>  $called  deduplication tracking
+	 * @param  array<int, true>  $processed  prevents reentry
 	 */
-	private function refreshMonitors(int $depth, ?array &$missing = null, array &$listeners = []): void
+	private function refreshMonitors(
+		int $depth,
+		?array &$missing = null,
+		array &$called = [],
+		array &$processed = [],
+	): void
 	{
-		if ($this instanceof IContainer) {
-			foreach ($this->getComponents() as $component) {
-				if ($component instanceof self) {
-					$component->refreshMonitors($depth + 1, $missing, $listeners);
-				}
-			}
-		}
+		$processed[spl_object_id($this)] = true;
 
-		if ($missing === null) { // detaching
-			foreach ($this->monitors as $type => [$ancestor, $inDepth, , $callbacks]) {
-				if (isset($inDepth) && $inDepth > $depth) { // only process if ancestor was deeper than current detachment point
-					assert($ancestor !== null);
-					if ($callbacks) {
-						$this->monitors[$type] = [null, null, null, $callbacks]; // clear cached object, keep listener registrations
-						foreach ($callbacks[1] as $detached) {
-							$listeners[] = [$detached, $ancestor];
-						}
-					} else { // no listeners, just cached lookup result - clear it
-						unset($this->monitors[$type]);
-					}
-				}
-			}
-		} else { // attaching
+		if ($missing !== null) { // attaching
 			foreach ($this->monitors as $type => [$ancestor, , , $callbacks]) {
 				if (isset($ancestor)) { // already cached and valid - skip
 					continue;
@@ -243,7 +226,10 @@ private function refreshMonitors(int $depth, ?array &$missing = null, array &$li
 					assert($type !== '');
 					if ($ancestor = $this->lookup($type, throw: false)) {
 						foreach ($callbacks[0] as $attached) {
-							$listeners[] = [$attached, $ancestor];
+							if (!in_array($key = [$attached, spl_object_id($ancestor)], $called, strict: false)) { // deduplicate: same callback + same object = call once
+								$attached($ancestor);
+								$called[] = $key;
+							}
 						}
 					} else {
 						$missing[$type] = true; // ancestor not found - remember so we don't check again
@@ -254,18 +240,33 @@ private function refreshMonitors(int $depth, ?array &$missing = null, array &$li
 			}
 		}
 
-		if ($depth === 0 && !$this->callingListeners) { // call listeners
-			$this->callingListeners = true;
-			try {
-				$called = [];
-				foreach ($listeners as [$callback, $component]) {
-					if (!in_array($key = [$callback, $component], $called, strict: false)) { // deduplicate: same callback + same object = call once
-						$callback($component);
-						$called[] = $key;
+		if ($this instanceof IContainer) {
+			foreach ($this->getComponents() as $component) {
+				if ($component instanceof self
+					&& !isset($processed[spl_object_id($component)]) // component may have been processed already
+					&& $component->getParent() === $this  // may have been removed by previous sibling's listener
+				) {
+					$component->refreshMonitors($depth + 1, $missing, $called, $processed);
+				}
+			}
+		}
+
+		if ($missing === null) { // detaching
+			foreach ($this->monitors as $type => [$ancestor, $inDepth, , $callbacks]) {
+				if (isset($inDepth) && $inDepth > $depth) { // only process if ancestor was deeper than current detachment point
+					assert($ancestor !== null);
+					if ($callbacks) {
+						$this->monitors[$type] = [null, null, null, $callbacks]; // clear cached object, keep listener registrations
+						foreach ($callbacks[1] as $detached) {
+							if (!in_array($key = [$detached, spl_object_id($ancestor)], $called, strict: false)) { // deduplicate: same callback + same object = call once
+								$detached($ancestor);
+								$called[] = $key;
+							}
+						}
+					} else { // no listeners, just cached lookup result - clear it
+						unset($this->monitors[$type]);
 					}
 				}
-			} finally {
-				$this->callingListeners = false;
 			}
 		}
 	}

tests/ComponentModel/Component.monitor.phpt

@@ -86,8 +86,8 @@ test('monitor with handlers', function () {
 	$a = new A;
 	$a['b'] = $b;
 	Assert::same([
-		'ATTACHED(A, C)',
 		'ATTACHED(A, B)',
+		'ATTACHED(A, C)',
 	], $log);
 
 	$log = [];

tests/ComponentModel/Component.monitorEdgeCases.phpt

@@ -77,7 +77,7 @@ test('parent removes children before their listeners run', function () {
 	$root->addComponent($parent, 'parent');
 
 	Assert::true($parent->attachedCalled);
-	Assert::true($child->attachedCalled); // problem: child listener is called although parent removed it first
+	Assert::false($child->attachedCalled, 'Child listener NOT called - parent removed it first');
 	Assert::same(0, count($parent->getComponents()));
 });
 
@@ -102,7 +102,7 @@ test('multiple nested children removed by parent', function () {
 	$root->addComponent($parent, 'parent');
 
 	foreach ($children as $i => $child) {
-		Assert::true($child->attachedCalled); // problem: child listener is called although parent removed it first
+		Assert::false($child->attachedCalled, "Child $i listener NOT called - parent removed it first");
 	}
 
 	Assert::same(0, count($parent->getComponents()));
@@ -129,7 +129,7 @@ test('child removes sibling before its listener runs', function () {
 	$root->addComponent($container, 'container');
 
 	Assert::true($child1->attachedCalled);
-	Assert::true($child2->attachedCalled); // problem: child2 listener is called although child1 removed it first
+	Assert::false($child2->attachedCalled, 'child2 listener NOT called - child1 removed it first');
 	Assert::same(1, count($container->getComponents()));
 });
 
@@ -251,6 +251,6 @@ test('component moved to later-visited container is not reprocessed', function (
 	// 4. Without $processed tracking, mover would be processed again
 	$root->addComponent($parent, 'parent');
 
-	Assert::same(2, $callCount); // problem: callback must be called exactly once
+	Assert::same(1, $callCount, 'Callback must be called exactly once');
 	Assert::same($secondContainer, $mover->getParent());
 });

tests/ComponentModel/Container.clone.monitor.phpt

@@ -184,8 +184,8 @@ test('reattach cloned component', function () {
 	$a['dolly'] = $dolly;
 
 	Assert::same([
-		'ATTACHED(A, C)',
 		'ATTACHED(A, B)',
+		'ATTACHED(A, C)',
 	], $log);
 
 	Assert::same([