소스 검색

Fix morhdom sibling cleanup bug

Caleb Porzio 3 년 전
부모
커밋
acc6fdc388
3개의 변경된 파일129개의 추가작업 그리고 11개의 파일을 삭제
  1. 70 0
      morph.html
  2. 28 11
      packages/morph/src/morph.js
  3. 31 0
      tests/cypress/integration/plugins/morph.spec.js

+ 70 - 0
morph.html

@@ -0,0 +1,70 @@
+<html>
+    <script src="./packages/morph/dist/cdn.js" defer></script>
+    <script src="./packages/alpinejs/dist/cdn.js" defer></script>
+    <!-- <script src="https://unpkg.com/alpinejs@3.0.0/dist/cdn.min.js" defer></script> -->
+
+<div id="before">
+<!-- Before markup goes here: -->
+<button>
+    <div>
+        <div>second</div>
+        <div>third</div>
+    </div>
+</button>
+</div>
+
+<div id="after" style="display: none;">
+<!-- After markup goes here: -->
+<button>
+    <div>first</div>
+    <div>
+        <div>second</div>
+        <div>third</div>
+    </div>
+</button>
+</div>
+
+    <div style="display: flex;">
+        <pre id="log-from"></pre>
+        <pre id="log-to"></pre>
+        <ul id="log-message"></ul>
+    </div>
+
+    <div style="position: absolute; bottom: 0; left: 0; padding: 1rem; width: 100vw; background: gray; box-sizing: border-box;">
+        <button onclick="start()">Start</button>
+        <button onclick="next()">Next Step</button>
+    </div>
+
+    <script>
+        function start() {
+            Alpine.morph.log((message, from, to) => {
+                document.querySelector('#log-from').innerHTML = escape(from.outerHTML)
+                document.querySelector('#log-to').innerHTML = escape(to.outerHTML)
+                let li = document.createElement('li')
+                li.textContent = message
+                message && document.querySelector('#log-message').appendChild(li)
+            })
+
+            Alpine.morph(
+                document.querySelector('#before').firstElementChild,
+                document.querySelector('#after').firstElementChild.outerHTML,
+                { debug: true }
+            )
+        }
+
+        function next() {
+            Alpine.morph.step()
+            setTimeout(() => window.dispatchEvent(new CustomEvent('refresh', { bubbles: true })))
+        }
+
+        function escape(unsafe) {
+            return unsafe
+                .replace(/\n/g, "⬎\n")
+                .replace(/&/g, "&amp;")
+                .replace(/</g, "&lt;")
+                .replace(/>/g, "&gt;")
+                .replace(/"/g, "&quot;")
+                .replace(/'/g, "&#039;");
+        }
+    </script>
+</html>

+ 28 - 11
packages/morph/src/morph.js

@@ -2,18 +2,24 @@ 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
 
-    message && logger(message.replace('\n', '\\n'))
+    logger((message || '').replace('\n', '\\n'), fromEl, toEl)
 
     return new Promise(resolve => resolveStep = () => resolve())
 }
 
 export async function morph(from, toHtml, options) {
     assignOptions(options)
-
-    let toEl = createElement(toHtml)
+    
+    fromEl = from
+    toEl = createElement(toHtml)
 
     // If there is no x-data on the element we're morphing,
     // let's seed it with the outer Alpine scope on the page.
@@ -27,6 +33,10 @@ export async function morph(from, toHtml, options) {
 
     await patch(from, toEl)
 
+    // Release these for the garbage collector.
+    fromEl = undefined
+    toEl = undefined
+
     return from
 }
 
@@ -275,19 +285,26 @@ async function patchChildren(from, to) {
         currentFrom = currentFrom && dom(currentFrom).nodes().next()
     }
 
-    // Cleanup extra froms
+    // 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)) {
-            let domForRemoval = currentFrom
+        if(! shouldSkip(removing, currentFrom)) removals.push(currentFrom)
+
+        currentFrom = dom(currentFrom).nodes().next()
+    }
 
-            domForRemoval.remove()
+    // Now we can do the actual removals.
+    while (removals.length) {
+        let domForRemoval = removals.pop()
 
-            await breakpoint('remove el')
+        domForRemoval.remove()
 
-            removed(domForRemoval)
-        }
+        await breakpoint('remove el')
 
-        currentFrom = dom(currentFrom).nodes().next()
+        removed(domForRemoval)
     }
 }
 

+ 31 - 0
tests/cypress/integration/plugins/morph.spec.js

@@ -255,3 +255,34 @@ test('can morph text nodes',
         get('h2').should(haveHtml('Foo <br> Baz'))
     },
 )
+
+test.only('can morph with added element before and siblings are different',
+    [html`
+        <button>
+            <div>
+                <div>second</div>
+                <div data="false">third</div>
+            </div>
+        </button>
+    `],
+    ({ get }, reload, window, document) => {
+        let toHtml = html`
+        <button>
+            <div>first</div>
+            <div>
+                <div>second</div>
+                <div data="true">third</div>
+            </div>
+        </button>
+        `
+
+        get('button').then(([el]) => window.Alpine.morph(el, toHtml))
+
+        get('button > div').should(haveLength(2))
+        get('button > div:nth-of-type(1)').should(haveText('first'))
+        get('button > div:nth-of-type(2)').should(haveHtml(`
+                <div>second</div>
+                <div data="true">third</div>
+            `))
+    },
+)