Issues with mutations and third party libs (#2399)

* Fix issue with mutations running in the wrong order when triggered by third party libs

* Revert "Fix issue with mutations running in the wrong order when triggered by third party libs"

This reverts commit c3e2f581fce93b498a7ae6e22b9200a3e597764c.

* Add failing test

* Fix issue with mutations running in the wrong order when triggered by third party libs

* Add failing test for regression

* Fix regression

packages/alpinejs/src/lifecycle.js

@@ -14,7 +14,7 @@ export function start() {
     startObservingMutations()
 
     onElAdded(el => initTree(el, walk))
-    onElRemoved(el => nextTick(() => destroyTree(el)))
+    onElRemoved(el => destroyTree(el))
 
     onAttributesAdded((el, attrs) => {
         directives(el, attrs).forEach(handle => handle())

packages/alpinejs/src/mutation.js

@@ -160,6 +160,14 @@ function onMutate(mutations) {
         onAttributeAddeds.forEach(i => i(el, attrs))
     })
 
+    for (let node of removedNodes) {
+        // If an element gets moved on a page, it's registered
+        // as both an "add" and "remove", so we want to skip those.
+        if (addedNodes.includes(node)) continue
+
+        onElRemoveds.forEach(i => i(node))
+    }
+
     // Mutations are bundled together by the browser but sometimes
     // for complex cases, there may be javascript code adding a wrapper
     // and then an alpine component as a child of that wrapper in the same
@@ -177,6 +185,10 @@ function onMutate(mutations) {
         // as both an "add" and "remove", so we want to skip those.
         if (removedNodes.includes(node)) continue
 
+        // If the node was eventually removed as part of one of his
+        // parent mutations, skip it
+        if (! node.isConnected) continue
+
         delete node._x_ignoreSelf
         delete node._x_ignore
         onElAddeds.forEach(i => i(node))
@@ -188,14 +200,6 @@ function onMutate(mutations) {
         delete node._x_ignore
     })
 
-    for (let node of removedNodes) {
-        // If an element gets moved on a page, it's registered
-        // as both an "add" and "remove", so we want to skip those.
-        if (addedNodes.includes(node)) continue
-
-        onElRemoveds.forEach(i => i(node))
-    }
-
     addedNodes = null
     removedNodes = null
     addedAttributes = null

tests/cypress/integration/mutation.spec.js

@@ -1,4 +1,4 @@
-import { haveText, html, test } from '../utils'
+import { beVisible, haveText, html, test } from '../utils'
 
 test('element side effects are cleaned up after the elements are removed',
     html`
@@ -138,3 +138,59 @@ test('does not initialise components twice when contained in multiple mutations'
         get('span#two').should(haveText('1'))
     }
 )
+
+test('directives keep working when node is moved into a different one',
+    html`
+        <div x-data="{
+            foo: 0,
+            mutate() {
+                let button = document.getElementById('one')
+                button.remove()
+                let container = document.createElement('p')
+                container.appendChild(button)
+                this.$root.appendChild(container)
+            }
+        }">
+            <button id="one" @click="foo++">increment</button>
+            <button id="two" @click="mutate()">Mutate</button>
+
+            <span x-text="foo"></span>
+        </div>
+    `,
+    ({ get }) => {
+        get('span').should(haveText('0'))
+        get('button#one').click()
+        get('span').should(haveText('1'))
+        get('button#two').click()
+        get('p').should(beVisible())
+        get('button#one').click()
+        get('span').should(haveText('2'))
+    }
+)
+
+test('no side effects when directives are added to an element that is removed afterwards',
+    html`
+        <div x-data="{
+            foo: 0,
+            mutate() {
+                let span = document.createElement('span')
+                span.setAttribute('x-on:keydown.a.window', 'foo = foo+1')
+                let container = document.getElementById('container')
+                container.appendChild(span)
+                container.remove()
+            }
+        }">
+            <button @click="mutate()">Mutate</button>
+            <p id="container"></p>
+            <input type="text">
+
+            <span x-text="foo"></span>
+        </div>
+    `,
+    ({ get }) => {
+        get('span').should(haveText('0'))
+        get('button').click()
+        get('input').type('a')
+        get('span').should(haveText('0'))
+    }
+)