a small incremental UI library for the web
javascript web ui
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Unmount deleted fiber trees and fix sibling pointer on last child

garrison b31faf76 cfdb0530

+69 -19
+8 -6
demos/todo/todo.tsx
··· 1 1 import * as Noir from '../../js/noir.ts'; 2 - import { useState, useEffect } from '../../js/hooks.ts'; 2 + import { useState, useEffect, useFiber } from '../../js/hooks.ts'; 3 3 4 4 function App(props) { 5 5 return ( ··· 11 11 12 12 function List(props) { 13 13 const [count, setCount] = useState(3); 14 - 15 - useEffect(() => { 16 - console.log('in'); 17 - return () => console.log('out'); 18 - }, [count]); 19 14 20 15 const items = []; 21 16 for (i = 0; i < count; i++) { ··· 25 20 return ( 26 21 <div class={`foo-${count}`}> 27 22 <button onClick={ev => setCount(c => c + 1)}>Add</button> 23 + <button onClick={ev => setCount(c => c - 1)}>Remove</button> 28 24 <div id="items">{items}</div> 29 25 </div> 30 26 ); 31 27 } 32 28 33 29 function Item(props) { 30 + const id = useFiber().id; 34 31 const [count, setCount] = useState(0); 32 + 33 + useEffect(() => { 34 + console.log(`in (id=${id}, count=${count})`); 35 + return () => console.log(`out (id=${id}, count=${count})`); 36 + }, [count]); 35 37 36 38 return ( 37 39 <div>
+33 -4
js/commit.ts
··· 1 1 import { FiberTag, EffectTag } from './fiber.ts'; 2 + import { cleanUpEffects } from './hooks'; 2 3 import { assert } from './utils.ts'; 3 4 4 5 const { ··· 49 50 let fiber = forFiber; 50 51 while (true) { 51 52 if (fiber.tag === HostFiber || fiber.tag === HostTextFiber) { 52 - // TODO: recursively unmount all child fibers 53 + // There is no need to remove DOM nodes for ancestors of this node 54 + // because the DOM will remove them for us, but we do still need to 55 + // traverse the subtree and unmount its components, 56 + // and that happens in `unmountFiberTree()` 57 + unmountFiberTree(fiber); 53 58 54 59 // A host fiber which is being deleted must have already been placed, 55 60 // so it must have a DOM node 56 61 assert(fiber.dom); 57 - // It is sufficient to delete the DOM node without traversing the host fiber's children, 58 - // as the DOM will remove all DOM children for us 59 62 domParent.removeChild(fiber.dom); 60 63 } 61 64 else { 62 - // TODO: unmount `fiber` 65 + // Unlike the above Host case, it is okay to unmount only this fiber 66 + // because we are going to traverse its children within this loop 67 + unmountFiber(fiber); 63 68 64 69 // This is a non-host fiber so it has no DOM node, but its children might, 65 70 // so we must traverse them looking for host fibers ··· 89 94 // Continue with the next sibling (which must be a descendant of `forFiber`) 90 95 fiber = fiber.sibling; 91 96 } 97 + } 98 + 99 + function unmountFiberTree(rootFiber) { 100 + let fiber = rootFiber; 101 + while (true) { 102 + unmountFiber(fiber); 103 + 104 + if (fiber.child) { 105 + fiber = fiber.child; 106 + continue; 107 + } 108 + if (fiber === rootFiber) return; 109 + 110 + while (!fiber.sibling) { 111 + if (!fiber.parent) return; 112 + if (fiber.parent === rootFiber) return; 113 + fiber = fiber.parent; 114 + } 115 + fiber = fiber.sibling; 116 + } 117 + } 118 + 119 + function unmountFiber(fiber) { 120 + cleanUpEffects(fiber); 92 121 } 93 122 94 123 function commitTextPlacement(fiber) {
+15 -8
js/fiber.ts
··· 1 - import { assertUnreachable } from './utils.ts'; 1 + import { assert, assertUnreachable } from './utils.ts'; 2 2 3 3 export const FiberTag = { 4 4 RootFiber: 'root', ··· 9 9 10 10 export const EffectTag = { 11 11 Placement: 'placement', 12 + Delete: 'delete', 12 13 }; 13 14 14 15 const { ··· 20 21 21 22 const { 22 23 Placement, 24 + Delete, 23 25 } = EffectTag; 24 26 25 27 let nextFiberId = 0; ··· 94 96 if (!childElementOrElements) return; 95 97 96 98 if (Array.isArray(childElementOrElements)) { 97 - // TODO: is flattening correct? 98 99 const elementsFlat = childElementOrElements.flat(); 99 100 return reconcileArray(parentFiber, elementsFlat); 100 101 } ··· 104 105 } 105 106 106 107 function reconcileArray(parentFiber, elements) { 108 + assert(parentFiber.effect !== Delete); 107 109 const oldFiberLookup = buildLookup(parentFiber.child); 108 110 109 111 parentFiber.deletedChildren = []; ··· 125 127 if (newFiber.alternate) { 126 128 const oldIndex = oldFiber.index; 127 129 if (oldIndex < lastKeptIndex) { 128 - newFiber.effect = 'placement'; 130 + newFiber.effect = Placement; 129 131 } 130 132 else { 131 133 lastKeptIndex = i; 132 134 } 133 135 } 134 136 else { 135 - newFiber.effect = 'placement'; 137 + newFiber.effect = Placement; 136 138 } 137 - if (oldFiber && oldFiber.effect === 'delete') { 139 + if (oldFiber && oldFiber.effect === Delete) { 138 140 parentFiber.deletedChildren.push(oldFiber); 139 141 } 140 142 ··· 142 144 else firstNewChild = newFiber; 143 145 prevChild = newFiber; 144 146 } 147 + // Remove any existing sibling pointer from the last child 148 + // TODO: maybe this should be done by cloneFiber, 149 + // it doesn't seem right to have to worry about it here 150 + // (and it has already caused a bug) 151 + if (prevChild) prevChild.sibling = null; 145 152 146 153 oldFiberLookup.forEach(deadFiber => { 147 - deadFiber.effect = 'delete'; 154 + deadFiber.effect = Delete; 148 155 parentFiber.deletedChildren.push(deadFiber); 149 156 }); 150 157 ··· 165 172 const nodeValue = element.toString(); 166 173 167 174 if (oldFiber === null || oldFiber.tag !== HostTextFiber) { 168 - if (oldFiber) oldFiber.effect = 'delete'; 175 + if (oldFiber) oldFiber.effect = Delete; 169 176 const newFiber = makeFiber(HostTextFiber); 170 177 newFiber.props = { nodeValue }; 171 178 return newFiber; ··· 179 186 180 187 function updateElement(oldFiber, element, index) { 181 188 if (oldFiber === null || oldFiber.type !== element.type) { 182 - if (oldFiber) oldFiber.effect = 'delete'; 189 + if (oldFiber) oldFiber.effect = Delete; 183 190 const newFiber = makeFiber(fiberTagForType(element.type)); 184 191 newFiber.type = element.type; 185 192 newFiber.props = element.props;
+13 -1
js/hooks.ts
··· 12 12 Effect, 13 13 } = Hook; 14 14 15 - function useFiber() { 15 + export function useFiber() { 16 16 const currentFiber = getCurrentFiber(); 17 17 if (!currentFiber) throw new Error('Hooks can only be called from within a function component.'); 18 18 return currentFiber; ··· 72 72 const newCleanup = callback(); 73 73 fiber.hooks[pointer + 1] = newCleanup; 74 74 }); 75 + } 76 + } 77 + 78 + export function cleanUpEffects(fiber) { 79 + const hooks = fiber.hooks; 80 + if (!hooks) return; 81 + 82 + for (let i = 0; i < hooks.length; i += 3) { 83 + if (hooks[i] === Effect) { 84 + const cleanup = hooks[i + 1]; 85 + cleanup(); 86 + } 75 87 } 76 88 } 77 89