Bug - Multiple morph calls clash (#2772)

* Add failing test

* Allow multiple Alpine.morph calls one after the other

* Rework comment

Co-authored-by: Caleb Porzio <calebporzio@gmail.com>

packages/morph/src/dom.js

@@ -0,0 +1,66 @@
+class DomManager {
+    el = undefined
+
+    constructor(el) {
+        this.el = el
+    }
+
+    traversals = {
+        'first': 'firstElementChild',
+        'next': 'nextElementSibling',
+        'parent': 'parentElement',
+    }
+
+    nodes() {
+        this.traversals = {
+            'first': 'firstChild',
+            'next': 'nextSibling',
+            'parent': 'parentNode',
+        }; return this
+    }
+
+    first() {
+        return this.teleportTo(this.el[this.traversals['first']])
+    }
+
+    next() {
+        return this.teleportTo(this.teleportBack(this.el[this.traversals['next']]))
+    }
+
+    before(insertee) {
+        this.el[this.traversals['parent']].insertBefore(insertee, this.el); return insertee
+    }
+
+    replace(replacement) {
+        this.el[this.traversals['parent']].replaceChild(replacement, this.el); return replacement
+    }
+
+    append(appendee) {
+        this.el.appendChild(appendee); return appendee
+    }
+
+    teleportTo(el) {
+        if (! el) return el
+        if (el._x_teleport) return el._x_teleport
+        return el
+    }
+
+    teleportBack(el) {
+        if (! el) return el
+        if (el._x_teleportBack) return el._x_teleportBack
+        return el
+    }
+}
+
+export function dom(el) {
+    return new DomManager(el)
+}
+
+export function createElement(html) {
+    return document.createRange().createContextualFragment(html).firstElementChild
+}
+
+export function textOrComment(el) {
+    return el.nodeType === 3
+        || el.nodeType === 8
+}

packages/morph/src/morph.js

@@ -1,435 +1,373 @@
+import { dom, createElement, textOrComment} from './dom.js'
+
 let resolveStep = () => {}
 
 let logger = () => {}
 
-// Keep these global so that we can access them
-// from hooks while debugging.
-let fromEl 
-let toEl
-
-function breakpoint(message) {
-    if (! debug) return
-
-    logger((message || '').replace('\n', '\\n'), fromEl, toEl)
-
-    return new Promise(resolve => resolveStep = () => resolve())
-}
-
 export async function morph(from, toHtml, options) {
-    assignOptions(options)
-    
-    fromEl = from
-    toEl = createElement(toHtml)
+    // We're defining these globals and methods inside this function (instead of outside)
+    // because it's an async function and if run twice, they would overwrite
+    // each other.
 
-    // If there is no x-data on the element we're morphing,
-    // let's seed it with the outer Alpine scope on the page.
-    if (window.Alpine && window.Alpine.closestDataStack && ! from._x_dataStack) {
-        toEl._x_dataStack = window.Alpine.closestDataStack(from)
+    let fromEl
+    let toEl
+    let key
+        ,lookahead
+        ,updating
+        ,updated
+        ,removing
+        ,removed
+        ,adding
+        ,added
+        ,debug
 
-        toEl._x_dataStack && window.Alpine.clone(from, toEl)
-    }
 
-    await breakpoint()
+    function breakpoint(message) {
+        if (! debug) return
 
-    await patch(from, toEl)
+        logger((message || '').replace('\n', '\\n'), fromEl, toEl)
 
-    // Release these for the garbage collector.
-    fromEl = undefined
-    toEl = undefined
+        return new Promise(resolve => resolveStep = () => resolve())
+    }
 
-    return from
-}
+    function assignOptions(options = {}) {
+        let defaultGetKey = el => el.getAttribute('key')
+        let noop = () => {}
+
+        updating = options.updating || noop
+        updated = options.updated || noop
+        removing = options.removing || noop
+        removed = options.removed || noop
+        adding = options.adding || noop
+        added = options.added || noop
+        key = options.key || defaultGetKey
+        lookahead = options.lookahead || false
+        debug = options.debug || false
+    }
 
-morph.step = () => resolveStep()
-morph.log = (theLogger) => {
-    logger = theLogger
-}
+    async function patch(from, to) {
+        // This is a time saver, however, it won't catch differences in nested <template> tags.
+        // I'm leaving this here as I believe it's an important speed improvement, I just
+        // don't see a way to enable it currently:
+        //
+        // if (from.isEqualNode(to)) return
 
-let key
-,lookahead
-,updating
-,updated
-,removing
-,removed
-,adding
-,added
-,debug
-
-let noop = () => {}
-
-function assignOptions(options = {}) {
-    let defaultGetKey = el => el.getAttribute('key')
-
-    updating = options.updating || noop
-    updated = options.updated || noop
-    removing = options.removing || noop
-    removed = options.removed || noop
-    adding = options.adding || noop
-    added = options.added || noop
-    key = options.key || defaultGetKey
-    lookahead = options.lookahead || false
-    debug = options.debug || false
-}
+        if (differentElementNamesTypesOrKeys(from, to)) {
+            let result = patchElement(from, to)
 
-function createElement(html) {
-    return document.createRange().createContextualFragment(html).firstElementChild
-}
+            await breakpoint('Swap elements')
 
-async function patch(from, to) {
-    // This is a time saver, however, it won't catch differences in nested <template> tags.
-    // I'm leaving this here as I believe it's an important speed improvement, I just
-    // don't see a way to enable it currently:
-    //
-    // if (from.isEqualNode(to)) return
+            return result
+        }
 
-    if (differentElementNamesTypesOrKeys(from, to)) {
-        let result = patchElement(from, to)
+        let updateChildrenOnly = false
 
-        await breakpoint('Swap elements')
+        if (shouldSkip(updating, from, to, () => updateChildrenOnly = true)) return
 
-        return result
-    }
+        window.Alpine && initializeAlpineOnTo(from, to, () => updateChildrenOnly = true)
 
-    let updateChildrenOnly = false
+        if (textOrComment(to)) {
+            await patchNodeValue(from, to)
+            updated(from, to)
 
-    if (shouldSkip(updating, from, to, () => updateChildrenOnly = true)) return
+            return
+        }
 
-    window.Alpine && initializeAlpineOnTo(from, to, () => updateChildrenOnly = true)
+        if (! updateChildrenOnly) {
+            await patchAttributes(from, to)
+        }
 
-    if (textOrComment(to)) {
-        await patchNodeValue(from, to)
         updated(from, to)
 
-        return
+        await patchChildren(from, to)
     }
 
-    if (! updateChildrenOnly) {
-        await patchAttributes(from, to)
+    function differentElementNamesTypesOrKeys(from, to) {
+        return from.nodeType != to.nodeType
+            || from.nodeName != to.nodeName
+            || getKey(from) != getKey(to)
     }
 
-    updated(from, to)
+    function patchElement(from, to) {
+        if (shouldSkip(removing, from)) return
 
-    await patchChildren(from, to)
-}
-
-function differentElementNamesTypesOrKeys(from, to) {
-    return from.nodeType != to.nodeType
-        || from.nodeName != to.nodeName
-        || getKey(from) != getKey(to)
-}
+        let toCloned = to.cloneNode(true)
 
-function textOrComment(el) {
-    return el.nodeType === 3
-        || el.nodeType === 8
-}
+        if (shouldSkip(adding, toCloned)) return
 
-function patchElement(from, to) {
-    if (shouldSkip(removing, from)) return
+        dom(from).replace(toCloned)
 
-    let toCloned = to.cloneNode(true)
-
-    if (shouldSkip(adding, toCloned)) return
-
-    dom(from).replace(toCloned)
-
-    removed(from)
-    added(toCloned)
-}
+        removed(from)
+        added(toCloned)
+    }
 
-async function patchNodeValue(from, to) {
-    let value = to.nodeValue
+    async function patchNodeValue(from, to) {
+        let value = to.nodeValue
 
-    if (from.nodeValue !== value) {
-        from.nodeValue = value
+        if (from.nodeValue !== value) {
+            from.nodeValue = value
 
-        await breakpoint('Change text node to: ' + value)
+            await breakpoint('Change text node to: ' + value)
+        }
     }
-}
 
-async function patchAttributes(from, to) {
-    if (from._x_isShown && ! to._x_isShown) {
-        return
-    }
-    if (! from._x_isShown && to._x_isShown) {
-        return
-    }
+    async function patchAttributes(from, to) {
+        if (from._x_isShown && ! to._x_isShown) {
+            return
+        }
+        if (! from._x_isShown && to._x_isShown) {
+            return
+        }
 
-    let domAttributes = Array.from(from.attributes)
-    let toAttributes = Array.from(to.attributes)
+        let domAttributes = Array.from(from.attributes)
+        let toAttributes = Array.from(to.attributes)
 
-    for (let i = domAttributes.length - 1; i >= 0; i--) {
-        let name = domAttributes[i].name;
+        for (let i = domAttributes.length - 1; i >= 0; i--) {
+            let name = domAttributes[i].name;
 
-        if (! to.hasAttribute(name)) {
-            from.removeAttribute(name)
+            if (! to.hasAttribute(name)) {
+                from.removeAttribute(name)
 
-            await breakpoint('Remove attribute')
+                await breakpoint('Remove attribute')
+            }
         }
-    }
 
-    for (let i = toAttributes.length - 1; i >= 0; i--) {
-        let name = toAttributes[i].name
-        let value = toAttributes[i].value
+        for (let i = toAttributes.length - 1; i >= 0; i--) {
+            let name = toAttributes[i].name
+            let value = toAttributes[i].value
 
-        if (from.getAttribute(name) !== value) {
-            from.setAttribute(name, value)
+            if (from.getAttribute(name) !== value) {
+                from.setAttribute(name, value)
 
-            await breakpoint(`Set [${name}] attribute to: "${value}"`)
+                await breakpoint(`Set [${name}] attribute to: "${value}"`)
+            }
         }
     }
-}
 
-async function patchChildren(from, to) {
-    let domChildren = from.childNodes
-    let toChildren = to.childNodes
+    async function patchChildren(from, to) {
+        let domChildren = from.childNodes
+        let toChildren = to.childNodes
 
-    let toKeyToNodeMap = keyToMap(toChildren)
-    let domKeyDomNodeMap = keyToMap(domChildren)
+        let toKeyToNodeMap = keyToMap(toChildren)
+        let domKeyDomNodeMap = keyToMap(domChildren)
 
-    let currentTo = dom(to).nodes().first()
-    let currentFrom = dom(from).nodes().first()
+        let currentTo = dom(to).nodes().first()
+        let currentFrom = dom(from).nodes().first()
 
-    let domKeyHoldovers = {}
+        let domKeyHoldovers = {}
 
-    while (currentTo) {
-        let toKey = getKey(currentTo)
-        let domKey = getKey(currentFrom)
+        while (currentTo) {
+            let toKey = getKey(currentTo)
+            let domKey = getKey(currentFrom)
 
-        // Add new elements
-        if (! currentFrom) {
-            if (toKey && domKeyHoldovers[toKey]) {
-                let holdover = domKeyHoldovers[toKey]
+            // Add new elements
+            if (! currentFrom) {
+                if (toKey && domKeyHoldovers[toKey]) {
+                    let holdover = domKeyHoldovers[toKey]
 
-                dom(from).append(holdover)
-                currentFrom = holdover
+                    dom(from).append(holdover)
+                    currentFrom = holdover
 
-                await breakpoint('Add element (from key)')
-            } else {
-                let added = addNodeTo(currentTo, from) || {}
+                    await breakpoint('Add element (from key)')
+                } else {
+                    let added = addNodeTo(currentTo, from) || {}
 
-                await breakpoint('Add element: ' + (added.outerHTML || added.nodeValue))
+                    await breakpoint('Add element: ' + (added.outerHTML || added.nodeValue))
 
-                currentTo = dom(currentTo).nodes().next()
+                    currentTo = dom(currentTo).nodes().next()
 
-                continue
+                    continue
+                }
             }
-        }
 
-        if (lookahead) {
-            let nextToElementSibling = dom(currentTo).next()
+            if (lookahead) {
+                let nextToElementSibling = dom(currentTo).next()
 
-            let found = false
+                let found = false
 
-            while (!found && nextToElementSibling) {
-                if (currentFrom.isEqualNode(nextToElementSibling)) {
-                    found = true
+                while (!found && nextToElementSibling) {
+                    if (currentFrom.isEqualNode(nextToElementSibling)) {
+                        found = true
 
-                    currentFrom = addNodeBefore(currentTo, currentFrom)
-
-                    domKey = getKey(currentFrom)
-
-                    await breakpoint('Move element (lookahead)')
-                }
-
-                nextToElementSibling = dom(nextToElementSibling).next()
-            }
-        }
-
-        if (toKey !== domKey) {
-            if (! toKey && domKey) {
-                domKeyHoldovers[domKey] = currentFrom
-                currentFrom = addNodeBefore(currentTo, currentFrom)
-                domKeyHoldovers[domKey].remove()
-                currentFrom = dom(currentFrom).nodes().next()
-                currentTo = dom(currentTo).nodes().next()
-
-                await breakpoint('No "to" key')
+                        currentFrom = addNodeBefore(currentTo, currentFrom)
 
-                continue
-            }
+                        domKey = getKey(currentFrom)
 
-            if (toKey && ! domKey) {
-                if (domKeyDomNodeMap[toKey]) {
-                    currentFrom = dom(currentFrom).replace(domKeyDomNodeMap[toKey])
+                        await breakpoint('Move element (lookahead)')
+                    }
 
-                    await breakpoint('No "from" key')
+                    nextToElementSibling = dom(nextToElementSibling).next()
                 }
             }
 
-            if (toKey && domKey) {
-                domKeyHoldovers[domKey] = currentFrom
-                let domKeyNode = domKeyDomNodeMap[toKey]
-
-                if (domKeyNode) {
-                    currentFrom = dom(currentFrom).replace(domKeyNode)
-
-                    await breakpoint('Move "from" key')
-                } else {
+            if (toKey !== domKey) {
+                if (! toKey && domKey) {
                     domKeyHoldovers[domKey] = currentFrom
                     currentFrom = addNodeBefore(currentTo, currentFrom)
                     domKeyHoldovers[domKey].remove()
-                    currentFrom = dom(currentFrom).next()
-                    currentTo = dom(currentTo).next()
+                    currentFrom = dom(currentFrom).nodes().next()
+                    currentTo = dom(currentTo).nodes().next()
 
-                    await breakpoint('Swap elements with keys')
+                    await breakpoint('No "to" key')
 
                     continue
                 }
-            }
-        }
 
-        // Get next from sibling before patching in case the node is replaced
-        let currentFromNext = currentFrom && dom(currentFrom).nodes().next()
+                if (toKey && ! domKey) {
+                    if (domKeyDomNodeMap[toKey]) {
+                        currentFrom = dom(currentFrom).replace(domKeyDomNodeMap[toKey])
 
-        // Patch elements
-        await patch(currentFrom, currentTo)
+                        await breakpoint('No "from" key')
+                    }
+                }
 
-        currentTo = currentTo && dom(currentTo).nodes().next()
-        currentFrom = currentFromNext
-    }
+                if (toKey && domKey) {
+                    domKeyHoldovers[domKey] = currentFrom
+                    let domKeyNode = domKeyDomNodeMap[toKey]
 
-    // Cleanup extra froms.
-    let removals = []
-    
-    // We need to collect the "removals" first before actually
-    // removing them so we don't mess with the order of things.
-    while (currentFrom) {
-        if(! shouldSkip(removing, currentFrom)) removals.push(currentFrom)
+                    if (domKeyNode) {
+                        currentFrom = dom(currentFrom).replace(domKeyNode)
 
-        currentFrom = dom(currentFrom).nodes().next()
-    }
+                        await breakpoint('Move "from" key')
+                    } else {
+                        domKeyHoldovers[domKey] = currentFrom
+                        currentFrom = addNodeBefore(currentTo, currentFrom)
+                        domKeyHoldovers[domKey].remove()
+                        currentFrom = dom(currentFrom).next()
+                        currentTo = dom(currentTo).next()
 
-    // Now we can do the actual removals.
-    while (removals.length) {
-        let domForRemoval = removals.shift()
+                        await breakpoint('Swap elements with keys')
 
-        domForRemoval.remove()
+                        continue
+                    }
+                }
+            }
 
-        await breakpoint('remove el')
+            // Get next from sibling before patching in case the node is replaced
+            let currentFromNext = currentFrom && dom(currentFrom).nodes().next()
 
-        removed(domForRemoval)
-    }
-}
+            // Patch elements
+            await patch(currentFrom, currentTo)
 
-function getKey(el) {
-    return el && el.nodeType === 1 && key(el)
-}
+            currentTo = currentTo && dom(currentTo).nodes().next()
+            currentFrom = currentFromNext
+        }
 
-function keyToMap(els) {
-    let map = {}
+        // Cleanup extra froms.
+        let removals = []
 
-    els.forEach(el => {
-        let theKey = getKey(el)
+        // We need to collect the "removals" first before actually
+        // removing them so we don't mess with the order of things.
+        while (currentFrom) {
+            if(! shouldSkip(removing, currentFrom)) removals.push(currentFrom)
 
-        if (theKey) {
-            map[theKey] = el
+            currentFrom = dom(currentFrom).nodes().next()
         }
-    })
 
-    return map
-}
+        // Now we can do the actual removals.
+        while (removals.length) {
+            let domForRemoval = removals.shift()
 
-function shouldSkip(hook, ...args) {
-    let skip = false
+            domForRemoval.remove()
 
-    hook(...args, () => skip = true)
+            await breakpoint('remove el')
 
-    return skip
-}
+            removed(domForRemoval)
+        }
+    }
 
-function addNodeTo(node, parent) {
-    if(! shouldSkip(adding, node)) {
-        let clone = node.cloneNode(true)
+    function getKey(el) {
+        return el && el.nodeType === 1 && key(el)
+    }
+
+    function keyToMap(els) {
+        let map = {}
 
-        dom(parent).append(clone)
+        els.forEach(el => {
+            let theKey = getKey(el)
 
-        added(clone)
+            if (theKey) {
+                map[theKey] = el
+            }
+        })
 
-        return clone
+        return map
     }
 
-    return null;
-}
+    function addNodeTo(node, parent) {
+        if(! shouldSkip(adding, node)) {
+            let clone = node.cloneNode(true)
 
-function addNodeBefore(node, beforeMe) {
-    if(! shouldSkip(adding, node)) {
-        let clone = node.cloneNode(true)
+            dom(parent).append(clone)
 
-        dom(beforeMe).before(clone)
+            added(clone)
 
-        added(clone)
+            return clone
+        }
 
-        return clone
+        return null;
     }
 
-    return beforeMe
-}
+    function addNodeBefore(node, beforeMe) {
+        if(! shouldSkip(adding, node)) {
+            let clone = node.cloneNode(true)
 
-function initializeAlpineOnTo(from, to, childrenOnly) {
-    if (from.nodeType !== 1) return
+            dom(beforeMe).before(clone)
 
-    // If the element we are updating is an Alpine component...
-    if (from._x_dataStack) {
-        // Then temporarily clone it (with it's data) to the "to" element.
-        // This should simulate backend Livewire being aware of Alpine changes.
-        window.Alpine.clone(from, to)
+            added(clone)
+
+            return clone
+        }
+
+        return beforeMe
     }
-}
 
-function dom(el) {
-    return new DomManager(el)
-}
+    // Finally we morph the element
 
-class DomManager {
-    el = undefined
+    assignOptions(options)
 
-    constructor(el) {
-        this.el = el
-    }
+    fromEl = from
+    toEl = createElement(toHtml)
 
-    traversals = {
-        'first': 'firstElementChild',
-        'next': 'nextElementSibling',
-        'parent': 'parentElement',
-    }
+    // If there is no x-data on the element we're morphing,
+    // let's seed it with the outer Alpine scope on the page.
+    if (window.Alpine && window.Alpine.closestDataStack && ! from._x_dataStack) {
+        toEl._x_dataStack = window.Alpine.closestDataStack(from)
 
-    nodes() {
-        this.traversals = {
-            'first': 'firstChild',
-            'next': 'nextSibling',
-            'parent': 'parentNode',
-        }; return this
+        toEl._x_dataStack && window.Alpine.clone(from, toEl)
     }
 
-    first() {
-        return this.teleportTo(this.el[this.traversals['first']])
-    }
+    await breakpoint()
 
-    next() {
-        return this.teleportTo(this.teleportBack(this.el[this.traversals['next']]))
-    }
+    await patch(from, toEl)
 
-    before(insertee) {
-        this.el[this.traversals['parent']].insertBefore(insertee, this.el); return insertee
-    }
+    // Release these for the garbage collector.
+    fromEl = undefined
+    toEl = undefined
 
-    replace(replacement) {
-        this.el[this.traversals['parent']].replaceChild(replacement, this.el); return replacement
-    }
+    return from
+}
 
-    append(appendee) {
-        this.el.appendChild(appendee); return appendee
-    }
+morph.step = () => resolveStep()
+morph.log = (theLogger) => {
+    logger = theLogger
+}
 
-    teleportTo(el) {
-        if (! el) return el
-        if (el._x_teleport) return el._x_teleport
-        return el
-    }
+function shouldSkip(hook, ...args) {
+    let skip = false
+
+    hook(...args, () => skip = true)
+
+    return skip
+}
 
-    teleportBack(el) {
-        if (! el) return el
-        if (el._x_teleportBack) return el._x_teleportBack
-        return el
+function initializeAlpineOnTo(from, to, childrenOnly) {
+    if (from.nodeType !== 1) return
+
+    // If the element we are updating is an Alpine component...
+    if (from._x_dataStack) {
+        // Then temporarily clone it (with it's data) to the "to" element.
+        // This should simulate backend Livewire being aware of Alpine changes.
+        window.Alpine.clone(from, to)
     }
 }

tests/cypress/integration/plugins/morph.spec.js

@@ -326,3 +326,19 @@ test('can morph different inline nodes',
         get('div').should(haveHtml('\n            Welcome <b>Person</b>!\n        '))
     },
 )
+
+test('can morph multiple nodes',
+    [html`
+        <div x-data>
+            <p></p>
+            <p></p>
+        </div>
+    `],
+    ({ get }, reload, window, document) => {
+        let paragraphs = document.querySelectorAll('p')
+        window.Alpine.morph(paragraphs[0], '<p>1</p')
+        window.Alpine.morph(paragraphs[1], '<p>2</p')
+        get('p:nth-of-type(1)').should(haveText('1'))
+        get('p:nth-of-type(2)').should(haveText('2'))
+    },
+)