bugfix: keyed FC children should now handle reordering correctly, ref…

…actor E2E tests

e2e/cypress/e2e/hmr.cy.ts

@@ -13,7 +13,7 @@ describe("hot module reload", () => {
   )
   beforeEach(() => {
     const port = Cypress.env("port")
-    cy.visit(`http://localhost:${port}`)
+    cy.visit(`http://localhost:${port}/counter`)
   })
   afterEach(() => cy.writeFile("src/Counter.tsx", counterTsx))
 

e2e/cypress/e2e/reactivity.cy.ts

@@ -1,7 +1,7 @@
 describe("basic reactivity & state", () => {
   beforeEach(() => {
     const port = Cypress.env("port")
-    cy.visit(`http://localhost:${port}`)
+    cy.visit(`http://localhost:${port}/counter`)
   })
 
   it("updates text in the dom that was derived from state when the state changes", () => {
@@ -25,7 +25,7 @@ describe("basic reactivity & state", () => {
 describe("hooks & data", () => {
   beforeEach(() => {
     const port = Cypress.env("port")
-    cy.visit(`http://localhost:${port}`)
+    cy.visit(`http://localhost:${port}/todos`)
   })
 
   it("can render a dynamic state-driven list", () => {

e2e/cypress/e2e/rendering.cy.ts

@@ -1,7 +1,7 @@
 describe("rendering", () => {
   beforeEach(() => {
     const port = Cypress.env("port")
-    cy.visit(`http://localhost:${port}`)
+    cy.visit(`http://localhost:${port}/counter`)
   })
   it("displays the correct text in the site heading", () => {
     cy.get("header h1").should("have.text", "Hello World")

e2e/src/App.tsx

@@ -1,19 +1,14 @@
-import { Link, Route, Router, memo, useModel, useState } from "kaioken"
+import { Link, Route, Router } from "kaioken"
 import { Counter } from "./Counter"
+import { MemoTest } from "./memo"
+import { TodoList } from "./Todos"
 
 export function App() {
-  const [toggled, setToggled] = useState(false)
   return (
     <main>
       <header>
         <h1>Hello World</h1>
       </header>
-      {toggled && <p>Toggled</p>}
-      <Counter />
-      <TodoList />
-      <button id="toggle-btn" onclick={() => setToggled(!toggled)}>
-        toggle
-      </button>
       <nav>
         <ul>
           <li>
@@ -22,72 +17,26 @@ export function App() {
           <li>
             <Link to="/about">About</Link>
           </li>
+          <li>
+            <Link to="/counter">Counter</Link>
+          </li>
+          <li>
+            <Link to="/todos">Todos</Link>
+          </li>
+          <li>
+            <Link to="/memo">Memo</Link>
+          </li>
         </ul>
       </nav>
       <div id="router-outlet">
         <Router>
           <Route path="/" element={() => <h2>Home</h2>} />
           <Route path="/about" element={() => <h2>About</h2>} />
-          <Route path="/memo" element={SimpleCounter} />
+          <Route path="/counter" element={Counter} />
+          <Route path="/todos" element={TodoList} />
+          <Route path="/memo" element={MemoTest} />
         </Router>
       </div>
     </main>
   )
 }
-
-function TodoList() {
-  const [inputRef, inputValue, setInputValue] = useModel("")
-  const [items, setItems] = useState<{ text: string }[]>([
-    { text: "buy coffee" },
-    { text: "walk the dog" },
-    { text: "push the latest commits" },
-  ])
-
-  function addItem() {
-    setItems((items) => [...items, { text: inputValue }])
-    setInputValue("")
-  }
-
-  return (
-    <div id="todos">
-      <input ref={inputRef} />
-      <button onclick={addItem} />
-      <ul>
-        {items.map((item) => (
-          <li>{item.text}</li>
-        ))}
-      </ul>
-    </div>
-  )
-}
-
-function SimpleCounter() {
-  const [count, setCount] = useState(0)
-  return (
-    <div id="memo">
-      <span>Count: {count}</span>
-      <button onclick={() => setCount((prev) => prev + 1)}>Increment</button>
-      <WhenPropsChangeMemo count={1} />
-      {count % 2 === 0 && <DynamicRenderMemo />}
-    </div>
-  )
-}
-
-let renders = 0
-const DynamicRenderMemo = memo(() => {
-  return (
-    <div id="memo-dynamic" className="flex flex-col">
-      <span className="text-red-500">Render Count: {++renders}</span>
-    </div>
-  )
-})
-
-let renders2 = 0
-const WhenPropsChangeMemo = memo(({ count }: { count: number }) => {
-  return (
-    <div id="memo-props" className="flex flex-col">
-      <div>Memo Demo {count}</div>
-      <span>Render Count: {++renders2}</span>
-    </div>
-  )
-})

e2e/src/Counter.tsx

@@ -1,9 +1,24 @@
 import { useState } from "kaioken"
 
 export function Counter() {
+  const [toggled, setToggled] = useState(false)
+  return (
+    <>
+      {/* used for checking that counter persists state after reordering these children */}
+      {toggled && <p>Toggled</p>}
+      <ActualCounter />
+      <button id="toggle-btn" onclick={() => setToggled(!toggled)}>
+        toggle
+      </button>
+    </>
+  )
+}
+
+export const ActualCounter = () => {
   const [count, setCount] = useState(0)
   return (
     <div id="counter">
+      {/** rendering.cy.ts - toggle attr check */}
       {count % 2 === 0 ? (
         <span data-even={true}>{count}</span>
       ) : (

e2e/src/KeyedList.tsx

@@ -0,0 +1,57 @@
+import { useState } from "kaioken"
+
+export function KeyedList() {
+  const [counters, setCounters] = useState<string[]>(
+    "abcdefghijklmnopqrstuvwxyz".split("")
+  )
+
+  const randomize = () => {
+    const newCounters = [...counters]
+    newCounters.sort(() => Math.random() - 0.5)
+    setCounters(newCounters)
+  }
+  const remove = (id: string) => {
+    setCounters(counters.filter((c) => c !== id))
+  }
+
+  return (
+    <div id="keyed-list">
+      <ul>
+        {counters.map((c) => (
+          <li key={"item-" + c} data-key={c} className="flex gap-2">
+            <KeyedCounterItem id={c} remove={() => remove(c)} />
+          </li>
+        ))}
+      </ul>
+
+      <button id="randomize" onclick={randomize}>
+        Randomize
+      </button>
+    </div>
+  )
+}
+
+interface KeyedCounterProps {
+  id: string
+  remove: () => void
+}
+
+function KeyedCounterItem({ id, remove }: KeyedCounterProps) {
+  const [count, setCount] = useState(0)
+  return (
+    <>
+      id : {id}
+      <div className="flex gap-2 px-2 bg-black bg-opacity-30">
+        <button
+          className="increment btn btn-primary"
+          onclick={() => setCount((c) => c + 1)}
+        >
+          {count}
+        </button>
+      </div>
+      <button className="remove btn btn-primary" onclick={remove}>
+        Remove
+      </button>
+    </>
+  )
+}

e2e/src/Todos.tsx

@@ -0,0 +1,27 @@
+import { useModel, useState } from "kaioken"
+
+export function TodoList() {
+  const [inputRef, inputValue, setInputValue] = useModel("")
+  const [items, setItems] = useState<{ text: string }[]>([
+    { text: "buy coffee" },
+    { text: "walk the dog" },
+    { text: "push the latest commits" },
+  ])
+
+  function addItem() {
+    setItems((items) => [...items, { text: inputValue }])
+    setInputValue("")
+  }
+
+  return (
+    <div id="todos">
+      <input ref={inputRef} />
+      <button onclick={addItem} />
+      <ul>
+        {items.map((item) => (
+          <li>{item.text}</li>
+        ))}
+      </ul>
+    </div>
+  )
+}

e2e/src/memo.tsx

@@ -0,0 +1,32 @@
+import { memo, useState } from "kaioken"
+
+export function MemoTest() {
+  const [count, setCount] = useState(0)
+  return (
+    <div id="memo">
+      <span>Count: {count}</span>
+      <button onclick={() => setCount((prev) => prev + 1)}>Increment</button>
+      <WhenPropsChangeMemo count={1} />
+      {count % 2 === 0 && <DynamicRenderMemo />}
+    </div>
+  )
+}
+
+let renders = 0
+const DynamicRenderMemo = memo(() => {
+  return (
+    <div id="memo-dynamic" className="flex flex-col">
+      <span className="text-red-500">Render Count: {++renders}</span>
+    </div>
+  )
+})
+
+let renders2 = 0
+const WhenPropsChangeMemo = memo(({ count }: { count: number }) => {
+  return (
+    <div id="memo-props" className="flex flex-col">
+      <div>Memo Demo {count}</div>
+      <span>Render Count: {++renders2}</span>
+    </div>
+  )
+})

packages/lib/src/dom.ts

@@ -287,6 +287,12 @@ function commitWork(ctx: AppContext, vNode: VNode) {
         c.effectTag = EffectTag.PLACEMENT
         c = c.sibling
       }
+      // if we're trying to place a non-html-producing node, process sibling first
+      // this prepares us to properly take advantage of the algorithm used by placeDom
+      if (commitSibling && n.sibling?.effectTag === EffectTag.PLACEMENT) {
+        stack.push([n, prevSiblingDom, mntParent], [n.sibling, dom, mntParent])
+        continue
+      }
     }
 
     if (commitSibling && n.sibling) {

packages/lib/src/reconciler.ts

@@ -270,6 +270,7 @@ function updateFromMap(
       oldChild.props = newChild.props
       oldChild.frozen = newChild.frozen
       oldChild.sibling = undefined
+      oldChild.index = index
       return oldChild
     } else {
       const n = createElement(newChild.type, newChild.props)