Skip to content

Commit a7343b8

Browse files
Fix removing multiple v-on event handlers
If we manipulate the DOM while iterating over it, the iteration may skip elements. At the moment the library uses two different workarounds for this: iterate over a shallow copy of the list using iterator_to_array() (handleNode(), handleAttributeBinding()), or defer the manipulation until after the loop (handleIf()). stripEventHandlers() did neither and therefore failed to remove consecutive event handlers; fix that using the defer approach, as I think that’s slightly more efficient in this case. Also use removeAttributeNode() while we’re at it so that PHP doesn’t have to look up the attribute by its name again.
1 parent 5a9cb60 commit a7343b8

File tree

2 files changed

+12
-1
lines changed

2 files changed

+12
-1
lines changed

src/Component.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,17 @@ private function stripEventHandlers( DOMNode $node ) {
7474
if ( $this->isTextNode( $node ) ) {
7575
return;
7676
}
77+
// Removing items while iterating breaks iteration, so defer attribute removal
78+
$attributesToRemove = [];
7779
/** @var DOMAttr $attribute */
7880
foreach ( $node->attributes as $attribute ) {
7981
if ( str_starts_with( $attribute->name, 'v-on:' ) ) {
80-
$node->removeAttribute( $attribute->name );
82+
$attributesToRemove[] = $attribute;
8183
}
8284
}
85+
foreach ( $attributesToRemove as $attribute ) {
86+
$node->removeAttributeNode( $attribute );
87+
}
8388
}
8489

8590
/**

tests/php/TemplatingTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ public function testTemplateHasOnClickHandlerInGrandChildNode_RemoveHandlerFormO
6363
$this->assertSame( '<p><b><a></a></b></p>', $result );
6464
}
6565

66+
public function testTemplateHasMultipleEventHandlers_RemoveAll(): void {
67+
$result = $this->createAndRender( '<p v-on:click="x" v-on:keypress="y"></p>', [] );
68+
69+
$this->assertSame( '<p></p>', $result );
70+
}
71+
6672
public function testTemplateWithSingleMustacheVariable_ReplacesVariableWithGivenValue() {
6773
$result = $this->createAndRender( '<p>{{value}}</p>', [ 'value' => 'some value' ] );
6874

0 commit comments

Comments
 (0)