Fix x-if sub expressions still being evaluated after x-if evals to fa…

…lse (#2556)

* Fix x-if sub-expressions still being evaluated after x-if evals to false

* Extend fix for queued effects to x-for

* Update x-for test description.

* formatting + code cleanup

packages/alpinejs/src/directives/x-for.js

@@ -6,6 +6,7 @@ import { initTree } from '../lifecycle'
 import { mutateDom } from '../mutation'
 import { flushJobs } from '../scheduler'
 import { warn } from '../utils/warn'
+import { dequeueJob } from '../scheduler'
 
 directive('for', (el, { expression }, { effect, cleanup }) => {
     let iteratorNames = parseForExpression(expression)
@@ -135,6 +136,10 @@ function loop(el, iteratorNames, evaluateItems, evaluateKey) {
         for (let i = 0; i < removes.length; i++) {
             let key = removes[i]
 
+            //Remove any queued effects that might run after the DOM node has been removed.
+            if (!!lookup[key]._x_effects) {
+                lookup[key]._x_effects.forEach(dequeueJob)
+            }
             lookup[key].remove()
 
             lookup[key] = null

packages/alpinejs/src/directives/x-if.js

@@ -3,6 +3,8 @@ import { addScopeToNode } from '../scope'
 import { directive } from '../directives'
 import { initTree } from '../lifecycle'
 import { mutateDom } from '../mutation'
+import { walk } from "../utils/walk"
+import { dequeueJob } from '../scheduler'
 
 directive('if', (el, { expression }, { effect, cleanup }) => {
     let evaluate = evaluateLater(el, expression)
@@ -22,7 +24,16 @@ directive('if', (el, { expression }, { effect, cleanup }) => {
 
         el._x_currentIfEl = clone
 
-        el._x_undoIf = () => { clone.remove(); delete el._x_currentIfEl }
+        el._x_undoIf = () => {
+            walk(clone, (node) => {
+                if (!!node._x_effects) {
+                    node._x_effects.forEach(dequeueJob)
+                }
+            })
+            clone.remove();
+            delete el._x_currentIfEl
+
+        }
 
         return clone
     }

packages/alpinejs/src/scheduler.js

@@ -10,6 +10,12 @@ function queueJob(job) {
 
     queueFlush()
 }
+export function dequeueJob(job) {
+    const index = queue.indexOf(job)
+    if (index !== -1) {
+        queue.splice(index, 1)
+    }
+}
 
 function queueFlush() {
     if (! flushing && ! flushPending) {

tests/cypress/integration/directives/x-for.spec.js

@@ -524,3 +524,24 @@ test('correctly renders x-if children when reordered',
         get('span:nth-of-type(2)').should(haveText('foo'))
     }
 )
+//If an x-for element is removed from DOM, expectation is that the removed DOM element will not have any of its reactive expressions evaluated after removal.
+test('x-for removed dom node does not evaluate child expressions after being removed',
+    html`
+        <div x-data="{ users: [{ name: 'lebowski' }] }">
+            <template x-for="(user, idx) in users">
+                <span x-text="users[idx].name"></span>
+            </template>
+            <button @click="users = []">Reset</button>
+        </div>
+    `,
+    ({ get }) => {
+        get('span').should(haveText('lebowski'))
+
+        /** Clicking button sets users=[] and thus x-for loop will remove all children.
+            If the sub-expression x-text="users[idx].name" is evaluated, the button click  
+            will produce an error because users[idx] is no longer defined and the test will fail
+        **/
+        get('button').click()
+        get('span').should('not.exist')
+    }
+)

tests/cypress/integration/directives/x-if.spec.js

@@ -51,3 +51,26 @@ test('x-if initializes after being added to the DOM to allow x-ref to work',
         get('li').should(haveText('bar'))
     }
 )
+
+//If x-if evaluates to false, the expectation is that no sub-expressions will be evaluated.
+test('x-if removed dom does not evaluate reactive expressions in dom tree',
+    html`
+    <div x-data="{user: {name: 'lebowski'}}">
+        <button @click="user = null">Log out</button>
+        <template x-if="user">
+            <span x-text="user.name"></span>
+        </template>
+
+    </div>
+    `,
+    ({ get }) => {
+        get('span').should(haveText('lebowski'))
+
+        /** Clicking button sets user=null and thus x-if="user" will evaluate to false. 
+            If the sub-expression x-text="user.name" is evaluated, the button click  
+            will produce an error because user is no longer defined and the test will fail
+        **/
+        get('button').click()
+        get('span').should('not.exist')
+    }
+)