diff --git a/compat/src/forwardRef.js b/compat/src/forwardRef.js index f72a011b49..75be7f26d6 100644 --- a/compat/src/forwardRef.js +++ b/compat/src/forwardRef.js @@ -2,10 +2,18 @@ import { options } from 'preact'; let oldDiffHook = options._diff; options._diff = (internal, vnode) => { - if (internal.type && internal.type._forwarded && vnode.ref) { - vnode.props.ref = vnode.ref; - vnode.ref = null; - internal.ref = null; + if ( + internal.type && + internal.type._forwarded && + (internal.ref || (vnode && vnode.ref)) + ) { + if (vnode) { + vnode.props.ref = vnode.ref; + vnode.ref = null; + } else { + internal.props.ref = internal.ref; + internal.ref = null; + } } if (oldDiffHook) oldDiffHook(internal, vnode); }; diff --git a/compat/src/index.js b/compat/src/index.js index 587dcd6c4e..86d2c7237d 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -32,7 +32,7 @@ import { REACT_ELEMENT_TYPE, __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED } from './render'; -import { getChildDom } from '../../src/tree'; +import { getFirstDom } from '../../src/tree'; export * from './scheduler'; const version = '17.0.2'; // trick libraries to think we are react @@ -72,7 +72,7 @@ function cloneElement(element) { * @returns {boolean} */ function unmountComponentAtNode(container) { - if (container._children) { + if (container._child) { preactRender(null, container); return true; } @@ -89,9 +89,11 @@ function findDOMNode(component) { return null; } else if (component.nodeType == 1) { return component; + } else if (component._internal._child == null) { + return null; } - return getChildDom(component._internal, 0); + return getFirstDom(component._internal._child, 0); } /** @@ -112,11 +114,11 @@ const StrictMode = Fragment; /** * In React, `flushSync` flushes the entire tree and forces a rerender. It's - * implmented here as a no-op. + * implemented here as a no-op. * @template Arg * @template Result * @param {(arg: Arg) => Result} callback function that runs before the flush - * @param {Arg} [arg] Optional arugment that can be passed to the callback + * @param {Arg} [arg] Optional argument that can be passed to the callback * @returns */ const flushSync = (callback, arg) => callback(arg); diff --git a/compat/src/render.js b/compat/src/render.js index a59411e331..12cff033a2 100644 --- a/compat/src/render.js +++ b/compat/src/render.js @@ -56,14 +56,14 @@ Component.prototype.isReactComponent = {}; export function render(vnode, parent, callback) { // React destroys any existing DOM nodes, see #1727 // ...but only on the first render, see #1828 - if (parent._children == null) { + if (parent._child == null) { parent.textContent = ''; } preactRender(vnode, parent); if (typeof callback == 'function') callback(); - const internal = parent._children._children[0]; + const internal = parent._child._child; return internal ? internal._component : null; } diff --git a/debug/src/component-stack.js b/debug/src/component-stack.js index bcba04d60f..0eef9ebbe7 100644 --- a/debug/src/component-stack.js +++ b/debug/src/component-stack.js @@ -1,18 +1,21 @@ -import { options, Fragment } from 'preact'; +import { options as rawOptions, Fragment } from 'preact'; + +const options = /** @type {import('../../src/internal').Options} */ (rawOptions); /** * Get human readable name of the component/dom node - * @param {import('./internal').VNode} vnode - * @param {import('./internal').VNode} vnode + * @param {import('../../src/internal').ComponentChild} vnode * @returns {string} */ export function getDisplayName(vnode) { - if (vnode.type === Fragment) { - return 'Fragment'; - } else if (typeof vnode.type == 'function') { - return vnode.type.displayName || vnode.type.name; - } else if (typeof vnode.type == 'string') { - return vnode.type; + if (vnode != null && typeof vnode === 'object') { + if (vnode.type === Fragment) { + return 'Fragment'; + } else if (typeof vnode.type == 'function') { + return vnode.type.displayName || vnode.type.name; + } else if (typeof vnode.type == 'string') { + return vnode.type; + } } return '#text'; diff --git a/debug/src/debug.js b/debug/src/debug.js index 61ecb46ae5..0aa0573ccd 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -1,5 +1,5 @@ import { checkPropTypes } from './check-props'; -import { options, Component } from 'preact'; +import { options as rawOptions, Component } from 'preact'; import { ELEMENT_NODE, DOCUMENT_NODE, @@ -12,16 +12,9 @@ import { getDisplayName } from './component-stack'; import { IS_NON_DIMENSIONAL } from '../../compat/src/util'; +import { validateTableMarkup } from './validateMarkup'; -const isWeakMapSupported = typeof WeakMap == 'function'; - -function getClosestDomNodeParent(parent) { - if (!parent) return {}; - if (typeof parent.type == 'function') { - return getClosestDomNodeParent(parent._parent); - } - return parent; -} +const options = /** @type {import('../../src/internal').Options} */ (rawOptions); export function initDebug() { setupComponentStack(); @@ -35,24 +28,32 @@ export function initDebug() { let oldCatchError = options._catchError; let oldRoot = options._root; let oldHook = options._hook; - const warnedComponents = !isWeakMapSupported - ? null - : { - useEffect: new WeakMap(), - useLayoutEffect: new WeakMap(), - lazyPropTypes: new WeakMap() - }; + const warnedComponents = { + useEffect: new WeakMap(), + useLayoutEffect: new WeakMap(), + lazyPropTypes: new WeakMap() + }; const deprecations = []; - options._catchError = (error, vnode, oldVNode) => { - let component = vnode && vnode._component; + options._catchError = catchErrorHook; + options._root = rootHook; + options._diff = diffHook; + options._hook = hookHook; + options.vnode = vnodeHook; + options.diffed = diffedHook; + + /** @type {typeof options["_catchError"]} */ + function catchErrorHook(error, internal) { + let component = internal && internal._component; if (component && typeof error.then == 'function') { const promise = error; error = new Error( - `Missing Suspense. The throwing component was: ${getDisplayName(vnode)}` + `Missing Suspense. The throwing component was: ${getDisplayName( + internal + )}` ); - let parent = vnode; + let parent = internal; for (; parent; parent = parent._parent) { if (parent._component && parent._component._childDidSuspend) { error = promise; @@ -68,7 +69,7 @@ export function initDebug() { } try { - oldCatchError(error, vnode, oldVNode); + oldCatchError(error, internal); // when an error was handled by an ErrorBoundary we will nontheless emit an error // event on the window object. This is to make up for react compatibility in dev mode @@ -81,9 +82,10 @@ export function initDebug() { } catch (e) { throw e; } - }; + } - options._root = (vnode, parentNode) => { + /** @type {typeof options["_root"]} */ + function rootHook(vnode, parentNode) { if (!parentNode) { throw new Error( 'Undefined parent passed to render(), this is the second argument.\n' + @@ -110,10 +112,16 @@ export function initDebug() { } if (oldRoot) oldRoot(vnode, parentNode); - }; + } + + /** @type {typeof options["_diff"]} */ + function diffHook(internal, vnode) { + if (vnode === null || typeof vnode !== 'object') { + // TODO: This isn't correct. We need these checks to run on mount + oldBeforeDiff(internal, vnode); + return; + } - options._diff = (internal, vnode) => { - if (vnode === null || typeof vnode !== 'object') return; // Check if the user passed plain objects as children. Note that we cannot // move this check into `options.vnode` because components can receive // children in any shape they want (e.g. @@ -153,44 +161,10 @@ export function initDebug() { ); } - let parentVNode = getClosestDomNodeParent(parent); + validateTableMarkup(internal); hooksAllowed = true; - if ( - (type === 'thead' || type === 'tfoot' || type === 'tbody') && - parentVNode.type !== 'table' - ) { - console.error( - 'Improper nesting of table. Your should have a parent.' + - serializeVNode(internal) + - `\n\n${getOwnerStack(internal)}` - ); - } else if ( - type === 'tr' && - parentVNode.type !== 'thead' && - parentVNode.type !== 'tfoot' && - parentVNode.type !== 'tbody' && - parentVNode.type !== 'table' - ) { - console.error( - 'Improper nesting of table. Your should have a parent.' + - serializeVNode(internal) + - `\n\n${getOwnerStack(internal)}` - ); - } else if (type === 'td' && parentVNode.type !== 'tr') { - console.error( - 'Improper nesting of table. Your parent.' + - serializeVNode(internal) + - `\n\n${getOwnerStack(internal)}` - ); - } else if (type === 'th' && parentVNode.type !== 'tr') { - console.error( - 'Improper nesting of table. Your .' + - serializeVNode(internal) + - `\n\n${getOwnerStack(internal)}` - ); - } let isCompatNode = '$$typeof' in vnode; if ( internal.ref !== undefined && @@ -244,7 +218,6 @@ export function initDebug() { if (typeof internal.type == 'function' && internal.type.propTypes) { if ( internal.type.displayName === 'Lazy' && - warnedComponents && !warnedComponents.lazyPropTypes.has(internal.type) ) { const m = @@ -279,15 +252,16 @@ export function initDebug() { } if (oldBeforeDiff) oldBeforeDiff(internal, vnode); - }; + } - options._hook = (internal, index, type) => { + /** @type {typeof options["_hook"]} */ + function hookHook(internal, index, type) { if (!internal || !hooksAllowed) { throw new Error('Hook can only be invoked from render methods.'); } if (oldHook) oldHook(internal, index, type); - }; + } // Ideally we'd want to print a warning once per component, but we // don't have access to the vnode that triggered it here. As a @@ -327,7 +301,8 @@ export function initDebug() { // https://esbench.com/bench/6021ebd7d9c27600a7bfdba3 const deprecatedProto = Object.create({}, deprecatedAttributes); - options.vnode = vnode => { + /** @type {typeof options["vnode"]} */ + function vnodeHook(vnode) { const props = vnode.props; if (props != null && ('__source' in props || '__self' in props)) { Object.defineProperties(props, debugProps); @@ -338,17 +313,18 @@ export function initDebug() { // eslint-disable-next-line vnode.__proto__ = deprecatedProto; if (oldVnode) oldVnode(vnode); - }; + } - options.diffed = vnode => { + /** @type {typeof options["diffed"]} */ + function diffedHook(internal) { hooksAllowed = false; - if (oldDiffed) oldDiffed(vnode); + if (oldDiffed) oldDiffed(internal); - if (vnode._children != null) { + if (internal._children != null) { const keys = []; - for (let i = 0; i < vnode._children.length; i++) { - const child = vnode._children[i]; + for (let i = 0; i < internal._children.length; i++) { + const child = internal._children[i]; if (!child || child.key == null) continue; const key = child.key; @@ -357,8 +333,8 @@ export function initDebug() { 'Following component has two or more children with the ' + `same key attribute: "${key}". This may cause glitches and misbehavior ` + 'in rendering process. Component: \n\n' + - serializeVNode(vnode) + - `\n\n${getOwnerStack(vnode)}` + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` ); // Break early to not spam the console @@ -368,7 +344,7 @@ export function initDebug() { keys.push(key); } } - }; + } } const setState = Component.prototype.setState; diff --git a/debug/src/validateMarkup.js b/debug/src/validateMarkup.js new file mode 100644 index 0000000000..7040ab5ec2 --- /dev/null +++ b/debug/src/validateMarkup.js @@ -0,0 +1,58 @@ +import { getOwnerStack } from './component-stack'; +import { serializeVNode } from './debug'; + +/** + * @param {import('./internal').Internal} parent + * @returns {import('./internal').Internal | null} + */ +function getClosestDomNodeParent(parent) { + if (!parent) return null; + if (typeof parent.type == 'function') { + return getClosestDomNodeParent(parent._parent); + } + return parent; +} + +/** + * @param {import('./internal').Internal} internal + * @return {void} + */ +export function validateTableMarkup(internal) { + const { type, _parent: parent } = internal; + const parentDomInternal = getClosestDomNodeParent(parent); + + if ( + (type === 'thead' || type === 'tfoot' || type === 'tbody') && + parentDomInternal.type !== 'table' + ) { + console.error( + 'Improper nesting of table. Your should have a
should have a
should have a
parent.' + + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` + ); + } else if ( + type === 'tr' && + parentDomInternal.type !== 'thead' && + parentDomInternal.type !== 'tfoot' && + parentDomInternal.type !== 'tbody' && + parentDomInternal.type !== 'table' + ) { + console.error( + 'Improper nesting of table. Your should have a parent.' + + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` + ); + } else if (type === 'td' && parentDomInternal.type !== 'tr') { + console.error( + 'Improper nesting of table. Your parent.' + + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` + ); + } else if (type === 'th' && parentDomInternal.type !== 'tr') { + console.error( + 'Improper nesting of table. Your .' + + serializeVNode(internal) + + `\n\n${getOwnerStack(internal)}` + ); + } +} diff --git a/jsconfig.json b/jsconfig.json index cd6a2a5b3c..ccd3f47e1d 100644 --- a/jsconfig.json +++ b/jsconfig.json @@ -3,14 +3,14 @@ "baseUrl": ".", "checkJs": true, "jsx": "react", + "jsxFactory": "createElement", "lib": ["dom", "es5"], "moduleResolution": "node", "paths": { "preact": ["."], "preact/*": ["./*"] }, - "reactNamespace": "createElement", - "target": "es5" + "target": "es2015" }, "exclude": ["node_modules", "dist", "demo"] } diff --git a/package.json b/package.json index 0488234160..d2e68489f2 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "preact", "amdName": "preact", - "version": "11.0.0-beta", + "version": "11.0.0-alpha", "private": false, "description": "Fast 3kb React-compatible Virtual DOM library.", "main": "dist/preact.js", @@ -21,65 +21,65 @@ "Samsung>=8.2", "not dead" ], - "sideEffects": [ - "./compat/**/*", - "./debug/**/*", - "./devtools/**/*", - "./hooks/**/*", - "./test-utils/**/*" - ], + "sideEffects": [ + "./compat/**/*", + "./debug/**/*", + "./devtools/**/*", + "./hooks/**/*", + "./test-utils/**/*" + ], "exports": { ".": { - "types": "./src/index.d.ts", + "types": "./src/index.d.ts", "module": "./dist/preact.mjs", "import": "./dist/preact.mjs", "require": "./dist/preact.js", "umd": "./dist/preact.umd.js" }, "./compat": { - "types": "./compat/src/index.d.ts", + "types": "./compat/src/index.d.ts", "module": "./compat/dist/compat.mjs", "import": "./compat/dist/compat.mjs", "require": "./compat/dist/compat.js", "umd": "./compat/dist/compat.umd.js" }, "./debug": { - "types": "./debug/src/index.d.ts", + "types": "./debug/src/index.d.ts", "module": "./debug/dist/debug.mjs", "import": "./debug/dist/debug.mjs", "require": "./debug/dist/debug.js", "umd": "./debug/dist/debug.umd.js" }, "./devtools": { - "types": "./devtools/src/index.d.ts", + "types": "./devtools/src/index.d.ts", "module": "./devtools/dist/devtools.mjs", "import": "./devtools/dist/devtools.mjs", "require": "./devtools/dist/devtools.js", "umd": "./devtools/dist/devtools.umd.js" }, "./hooks": { - "types": "./hooks/src/index.d.ts", + "types": "./hooks/src/index.d.ts", "module": "./hooks/dist/hooks.mjs", "import": "./hooks/dist/hooks.mjs", "require": "./hooks/dist/hooks.js", "umd": "./hooks/dist/hooks.umd.js" }, "./test-utils": { - "types": "./test-utils/src/index.d.ts", + "types": "./test-utils/src/index.d.ts", "module": "./test-utils/dist/testUtils.mjs", "import": "./test-utils/dist/testUtils.mjs", "require": "./test-utils/dist/testUtils.js", "umd": "./test-utils/dist/testUtils.umd.js" }, "./jsx-runtime": { - "types": "./jsx-runtime/src/index.d.ts", + "types": "./jsx-runtime/src/index.d.ts", "module": "./jsx-runtime/dist/jsxRuntime.mjs", "import": "./jsx-runtime/dist/jsxRuntime.mjs", "require": "./jsx-runtime/dist/jsxRuntime.js", "umd": "./jsx-runtime/dist/jsxRuntime.umd.js" }, "./jsx-dev-runtime": { - "types": "./jsx-runtime/src/index.d.ts", + "types": "./jsx-runtime/src/index.d.ts", "module": "./jsx-runtime/dist/jsxRuntime.mjs", "import": "./jsx-runtime/dist/jsxRuntime.mjs", "require": "./jsx-runtime/dist/jsxRuntime.js", @@ -91,12 +91,12 @@ "require": "./server/dist/server.js", "umd": "./server/dist/server.umd.js" }, - "./compat/client": { + "./compat/client": { "import": "./compat/client.mjs", "require": "./compat/client.js" }, "./compat/server": { - "browser": "./compat/server.browser.js", + "browser": "./compat/server.browser.js", "module": "./compat/server.mjs", "import": "./compat/server.mjs", "require": "./compat/server.js" @@ -219,7 +219,7 @@ "compat/server.js", "compat/server.browser.js", "compat/server.mjs", - "compat/client.js", + "compat/client.js", "compat/client.mjs", "compat/test-utils.js", "compat/jsx-runtime.js", @@ -318,5 +318,8 @@ }, "dependencies": { "pretty-format": "^27.5.1" + }, + "volta": { + "node": "18.14.0" } } diff --git a/src/component.js b/src/component.js index daa916699a..e495a1847e 100644 --- a/src/component.js +++ b/src/component.js @@ -4,6 +4,7 @@ import { createElement, Fragment } from './create-element'; import { patch } from './diff/patch'; import { DIRTY_BIT, FORCE_UPDATE, MODE_UNMOUNTING } from './constants'; import { getParentDom } from './tree'; +import { inEvent } from './diff/props'; export let ENABLE_CLASSES = false; @@ -122,6 +123,19 @@ let renderQueue = []; let prevDebounce; +const microTick = + typeof Promise == 'function' + ? Promise.prototype.then.bind(Promise.resolve()) + : setTimeout; + +function defer(cb) { + if (inEvent) { + setTimeout(cb); + } else { + microTick(cb); + } +} + /** * Enqueue a rerender of an internal * @param {import('./internal').Internal} internal The internal to rerender @@ -135,7 +149,7 @@ export function enqueueRender(internal) { prevDebounce !== options.debounceRendering ) { prevDebounce = options.debounceRendering; - (prevDebounce || queueMicrotask)(processRenderQueue); + (prevDebounce || defer)(processRenderQueue); } } diff --git a/src/constants.js b/src/constants.js index a2e805effa..ea627adba7 100644 --- a/src/constants.js +++ b/src/constants.js @@ -48,6 +48,9 @@ export const DIRTY_BIT = 1 << 14; /** Signals the component can skip children due to a non-update */ export const SKIP_CHILDREN = 1 << 15; +/** Indicates that this node needs to be inserted while patching children */ +export const INSERT_INTERNAL = 1 << 16; + /** Reset all mode flags */ export const RESET_MODE = ~( MODE_HYDRATE | @@ -63,4 +66,5 @@ export const RESET_MODE = ~( export const INHERITED_MODES = MODE_HYDRATE | MODE_MUTATIVE_HYDRATE | MODE_SVG; export const EMPTY_ARR = []; +/** @type {undefined} */ export const UNDEFINED = undefined; diff --git a/src/create-root.js b/src/create-root.js index 3d2de048a9..c40327efbe 100644 --- a/src/create-root.js +++ b/src/create-root.js @@ -35,7 +35,7 @@ export function createRoot(parentDom) { rootInternal = createInternal(vnode); // Store the VDOM tree root on the DOM element in a (minified) property: - parentDom._children = rootInternal; + parentDom._child = rootInternal; // Calling createRoot().render() on an Element with existing children triggers mutative hydrate mode: if (firstChild) { @@ -49,7 +49,7 @@ export function createRoot(parentDom) { rootInternal._context = {}; - mount(rootInternal, vnode, parentDom, firstChild); + mount(rootInternal, parentDom, firstChild); } // Flush all queued effects diff --git a/src/diff/children.js b/src/diff/children.js index 091a6765f8..4e48e60266 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -1,252 +1,290 @@ import { applyRef } from './refs'; -import { normalizeToVNode } from '../create-element'; +import { createElement, Fragment } from '../create-element'; import { TYPE_COMPONENT, + TYPE_TEXT, MODE_HYDRATE, MODE_SUSPENDED, - EMPTY_ARR, TYPE_DOM, - UNDEFINED + TYPE_ELEMENT, + INSERT_INTERNAL } from '../constants'; import { mount } from './mount'; import { patch } from './patch'; import { unmount } from './unmount'; import { createInternal, getDomSibling } from '../tree'; +/** + * Scenarios: + * + * 1. Unchanged: no ordering changes, walk new+old children and update Internals in-place + * 2. All removed: walk old child Internals and unmount + * 3. All added: walk over new child vnodes and create Internals, assign `.next`, mount + */ + +/** @typedef {import('../internal').Internal} Internal */ +/** @typedef {import('../internal').VNode} VNode */ +/** @typedef {import('../internal').PreactElement} PreactElement */ +/** @typedef {import('../internal').ComponentChildren} ComponentChildren */ + /** * Update an internal with new children. - * @param {import('../internal').Internal} internal The internal whose children should be patched - * @param {import('../internal').ComponentChild[]} children The new children, represented as VNodes - * @param {import('../internal').PreactElement} parentDom The element into which this subtree is rendered + * @param {Internal} internal The internal whose children should be patched + * @param {ComponentChildren[]} children The new children, represented as VNodes + * @param {PreactElement} parentDom The element into which this subtree is rendered */ export function patchChildren(internal, children, parentDom) { - let oldChildren = - (internal._children && internal._children.slice()) || EMPTY_ARR; + // Find matches and set up _next pointers. Unmount unused/unclaimed Internal + findMatches(internal._child, children, internal); - let oldChildrenLength = oldChildren.length; - let remainingOldChildren = oldChildrenLength; + // Walk forwards over the newly-assigned _next properties, inserting Internals + // that require insertion. + let index = 0; + internal = internal._child; + while (internal) { + let vnode = children[index]; + while (vnode == null || vnode === true || vnode === false) { + vnode = children[++index]; + } - let skew = 0; - let i; + if (internal._index === -1) { + let nextDomSibling = getDomSibling(internal); + mount(internal, parentDom, nextDomSibling); + if (internal.flags & TYPE_DOM) { + // If we are mounting a component, it's DOM children will get inserted + // into the DOM in mountChildren. If we are mounting a DOM node, then + // it's children will be mounted into itself and we need to insert this + // DOM in place. + insert(internal, parentDom, nextDomSibling); + } + } else if ( + (internal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) === + (MODE_HYDRATE | MODE_SUSPENDED) + ) { + mount(internal, parentDom, internal.data); + } else { + patch( + internal, + Array.isArray(vnode) ? createElement(Fragment, null, vnode) : vnode, + parentDom + ); + if (internal.flags & INSERT_INTERNAL) { + insert(internal, parentDom, getDomSibling(internal)); + } + } - /** @type {import('../internal').Internal} */ - let childInternal; + let oldRef = internal._prevRef; + if (internal.ref != oldRef) { + if (oldRef) applyRef(oldRef, null, internal); + if (internal.ref) + applyRef(internal.ref, internal._component || internal.data, internal); + } - /** @type {import('../internal').ComponentChild} */ - let childVNode; + internal.flags &= ~INSERT_INTERNAL; + internal._index = index++; + internal = internal._next; + } +} - /** @type {import('../internal').Internal[]} */ - const newChildren = []; +/** + * @param {Internal | null} internal + * @param {ComponentChildren[]} children + * @param {Internal} parentInternal + */ +function findMatches(internal, children, parentInternal) { + parentInternal._child = null; - for (i = 0; i < children.length; i++) { - childVNode = normalizeToVNode(children[i]); + /** @type {Internal | undefined} The last matched internal */ + let prevMatchedInternal; - // Terser removes the `continue` here and wraps the loop body - // in a `if (childVNode) { ... } condition - if (childVNode == null) { - newChildren[i] = null; - continue; - } + /** @type {Map | undefined} */ + let keyMap; - let skewedIndex = i + skew; + /** @type {number} Tracks the index of the last Internal we decided was "in-place" and did not need insertion */ + let lastPlacedIndex = 0; - /// TODO: Reconsider if we should bring back the "not moving text nodes" logic? - let matchingIndex = findMatchingIndex( - childVNode, - oldChildren, - skewedIndex, - remainingOldChildren - ); + for (let index = 0; index < children.length; index++) { + let vnode = children[index]; - if (matchingIndex === -1) { - childInternal = UNDEFINED; - } else { - childInternal = oldChildren[matchingIndex]; - oldChildren[matchingIndex] = UNDEFINED; - remainingOldChildren--; - } + // holes get accounted for in the index property: + if (vnode == null || vnode === true || vnode === false) { + if (internal && index == internal._index && internal.key == null) { + // The current internal is unkeyed, has the same index as this VNode + // child, and the VNode is now null. So we'll unmount the Internal and + // treat this slot in the children array as a null placeholder. We'll + // eagerly unmount this node to prevent it from being used in future + // searches for matching internals + unmount(internal, internal, 0); - let mountingChild = childInternal == null; + internal = internal._next; + } + continue; + } - if (mountingChild) { - childInternal = createInternal(childVNode, internal); + let type = null; + let typeFlag = 0; + let key; + /** @type {Internal | undefined} */ + let matchedInternal; - // We are mounting a new VNode - mount( - childInternal, - childVNode, - parentDom, - getDomSibling(internal, skewedIndex) - ); - } - // If this node suspended during hydration, and no other flags are set: - // @TODO: might be better to explicitly check for MODE_ERRORED here. - else if ( - (childInternal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) === - (MODE_HYDRATE | MODE_SUSPENDED) - ) { - // We are resuming the hydration of a VNode - mount(childInternal, childVNode, parentDom, childInternal.data); + // text VNodes (strings, numbers, bigints, etc): + if (typeof vnode !== 'object') { + typeFlag = TYPE_TEXT; + vnode = '' + vnode; } else { - // Morph the old element into the new one, but don't append it to the dom yet - patch(childInternal, childVNode, parentDom); - } + // TODO: Investigate avoiding this VNode allocation (and the one below in + // the call to `patch`) by passing through the raw VNode type and handling + // nested arrays directly in mount, patch, createInternal, etc. + vnode = Array.isArray(vnode) + ? createElement(Fragment, null, vnode) + : vnode; - go: if (mountingChild) { - if (matchingIndex == -1) { - skew--; - } + type = vnode.type; + typeFlag = typeof type === 'function' ? TYPE_COMPONENT : TYPE_ELEMENT; + key = vnode.key; + } - // Perform insert of new dom - if (childInternal.flags & TYPE_DOM) { - parentDom.insertBefore( - childInternal.data, - getDomSibling(internal, skewedIndex) - ); + if (key == null && internal && index < internal._index) { + // If we are doing an unkeyed diff, and the old index of the current + // internal in the old list of children is greater than the current VNode + // index, then this vnode represents a new element that is mounting into + // what was previous a null placeholder slot. We should create a new + // internal to mount this VNode. + } else if ( + !keyMap && + internal && + internal.flags & typeFlag && + internal.type === type && + internal.key == key + ) { + // Fast path checking if this current vnode matches the first unused + // Internal. By doing this we can avoid the search loop and setting the + // move flag, which allows us to skip the LDS algorithm if no Internals + // moved + matchedInternal = internal; + internal = internal._next; + } else if (internal) { + /* Keyed search */ + /** @type {any} */ + let search; + if (!keyMap) { + keyMap = buildMap(internal); } - } else if (matchingIndex !== skewedIndex) { - // Move this DOM into its correct place - if (matchingIndex === skewedIndex + 1) { - skew++; - break go; - } else if (matchingIndex > skewedIndex) { - if (remainingOldChildren > children.length - skewedIndex) { - skew += matchingIndex - skewedIndex; - break go; - } else { - // ### Change from keyed: I think this was missing from the algo... - skew--; - } - } else if (matchingIndex < skewedIndex) { - if (matchingIndex == skewedIndex - 1) { - skew = matchingIndex - skewedIndex; - } else { - skew = 0; + if (key == null) { + search = keyMap.get(type); + if (search && search.length) { + matchedInternal = search.shift(); } } else { - skew = 0; + search = keyMap.get(key); + if (search && search.type == type) { + keyMap.delete(key); + matchedInternal = search; + } } + } - skewedIndex = i + skew; - - if (matchingIndex == i) break go; - - let nextSibling = getDomSibling(internal, skewedIndex + 1); - if (childInternal.flags & TYPE_DOM) { - parentDom.insertBefore(childInternal.data, nextSibling); - } else { - insertComponentDom(childInternal, nextSibling, parentDom); - } + // No match, create a new Internal: + if (!matchedInternal) { + matchedInternal = createInternal(vnode, parentInternal); + } else if (matchedInternal._index < lastPlacedIndex) { + // If the matched internal has moved such that it is now after the last + // internal we determined was "in-place", mark it for insertion to move it + // into the correct place + matchedInternal.flags |= INSERT_INTERNAL; + } else { + // If the matched internal's oldIndex is greater the index of the last + // internal we determined was "in-place", make this internal the new + // "in-place" internal. Doing this (only moving the index forward when + // matching internals old index is grater) better accommodates more + // scenarios such as unmounting Internals at the beginning and middle of + // lists + lastPlacedIndex = matchedInternal._index; } - newChildren[i] = childInternal; + // Put matched or new internal into the new list of children + if (prevMatchedInternal) prevMatchedInternal._next = matchedInternal; + else parentInternal._child = matchedInternal; + prevMatchedInternal = matchedInternal; + + // TODO: Consider detecting if an internal is of TYPE_ROOT, whether or not + // it is a PORTAL, and setting a flag as such to use in getDomSibling and + // getFirstDom } - internal._children = newChildren; + // Ensure the last node of the last matched internal has a null _next pointer. + // Its possible that it still points to it's old sibling at the end of this loop, + // so we'll manually clear it here. + if (prevMatchedInternal) prevMatchedInternal._next = null; - // Remove remaining oldChildren if there are any. - if (remainingOldChildren > 0) { - for (i = oldChildrenLength; i--; ) { - if (oldChildren[i] != null) { - unmount(oldChildren[i], oldChildren[i]); - } - } + // Walk over the unused children and unmount: + if (keyMap) { + unmountUnusedKeyedChildren(keyMap); + } else if (internal) { + unmountUnusedChildren(internal); } +} - // Set refs only after unmount - for (i = 0; i < newChildren.length; i++) { - childInternal = newChildren[i]; - if (childInternal) { - let oldRef = childInternal._prevRef; - if (childInternal.ref != oldRef) { - if (oldRef) applyRef(oldRef, null, childInternal); - if (childInternal.ref) - applyRef( - childInternal.ref, - childInternal._component || childInternal.data, - childInternal - ); - } +/** + * @param {Internal | null} internal + * @returns {Map} + */ +function buildMap(internal) { + let keyMap = new Map(); + while (internal) { + if (internal.key) { + keyMap.set(internal.key, internal); + } else if (!keyMap.has(internal.type)) { + keyMap.set(internal.type, [internal]); + } else { + keyMap.get(internal.type).push(internal); } + internal = internal._next; } + + return keyMap; } /** - * @param {import('../internal').VNode | string} childVNode - * @param {import('../internal').Internal[]} oldChildren - * @param {number} skewedIndex - * @param {number} remainingOldChildren - * @returns {number} + * @param {Map} keyMap */ -function findMatchingIndex( - childVNode, - oldChildren, - skewedIndex, - remainingOldChildren -) { - const type = typeof childVNode == 'string' ? null : childVNode.type; - const key = type !== null ? childVNode.key : UNDEFINED; - let match = -1; - let x = skewedIndex - 1; // i - 1; - let y = skewedIndex + 1; // i + 1; - let oldChild = oldChildren[skewedIndex]; // i - - if ( - // ### Change from keyed: support for matching null placeholders - oldChild === null || - (oldChild != null && oldChild.type === type && oldChild.key == key) - ) { - match = skewedIndex; // i - } - // If there are any unused children left (ignoring an available in-place child which we just checked) - else if (remainingOldChildren > (oldChild != null ? 1 : 0)) { - // eslint-disable-next-line no-constant-condition - while (true) { - if (x >= 0) { - oldChild = oldChildren[x]; - if (oldChild != null && oldChild.type === type && oldChild.key == key) { - match = x; - break; - } - x--; - } - if (y < oldChildren.length) { - oldChild = oldChildren[y]; - if (oldChild != null && oldChild.type === type && oldChild.key == key) { - match = y; - break; - } - y++; - } else if (x < 0) { - break; +function unmountUnusedKeyedChildren(keyMap) { + for (let internal of keyMap.values()) { + if (Array.isArray(internal)) { + for (let i of internal) { + unmount(i, i, 0); } + } else { + unmount(internal, internal, 0); } } +} - return match; +/** + * @param {Internal | null} internal + */ +function unmountUnusedChildren(internal) { + while (internal) { + unmount(internal, internal, 0); + internal = internal._next; + } } /** * @param {import('../internal').Internal} internal - * @param {import('../internal').PreactNode} nextSibling * @param {import('../internal').PreactNode} parentDom + * @param {import('../internal').PreactNode} nextSibling */ -export function insertComponentDom(internal, nextSibling, parentDom) { - if (internal._children == null) { - return; - } - - for (let i = 0; i < internal._children.length; i++) { - let childInternal = internal._children[i]; - if (childInternal) { - childInternal._parent = internal; - - if (childInternal.flags & TYPE_COMPONENT) { - insertComponentDom(childInternal, nextSibling, parentDom); - } else if (childInternal.data != nextSibling) { - parentDom.insertBefore(childInternal.data, nextSibling); - } +export function insert(internal, parentDom, nextSibling) { + if (internal.flags & TYPE_COMPONENT) { + let child = internal._child; + while (child) { + insert(child, parentDom, nextSibling); + child = child._next; } + } else if (internal.data != nextSibling) { + // @ts-ignore .data is a Node + parentDom.insertBefore(internal.data, nextSibling); } } diff --git a/src/diff/mount.js b/src/diff/mount.js index 34e081dd7f..e17d20deeb 100644 --- a/src/diff/mount.js +++ b/src/diff/mount.js @@ -13,7 +13,7 @@ import { MODE_SVG, DIRTY_BIT } from '../constants'; -import { normalizeToVNode, Fragment } from '../create-element'; +import { normalizeToVNode, createElement, Fragment } from '../create-element'; import { setProperty } from './props'; import { createInternal, getParentContext } from '../tree'; import options from '../options'; @@ -22,13 +22,12 @@ import { commitQueue } from './commit'; /** * Diff two virtual nodes and apply proper changes to the DOM * @param {import('../internal').Internal} internal The Internal node to mount - * @param {import('../internal').VNode | string} newVNode The new virtual node * @param {import('../internal').PreactElement} parentDom The element into which this subtree is rendered * @param {import('../internal').PreactNode} startDom * @returns {import('../internal').PreactNode | null} pointer to the next DOM node to be hydrated (or null) */ -export function mount(internal, newVNode, parentDom, startDom) { - if (options._diff) options._diff(internal, newVNode); +export function mount(internal, parentDom, startDom) { + if (options._diff) options._diff(internal, null); /** @type {import('../internal').PreactNode} */ let nextDomSibling, prevStartDom; @@ -40,15 +39,16 @@ export function mount(internal, newVNode, parentDom, startDom) { // top. if ( internal.flags & TYPE_ROOT && - newVNode.props._parentDom !== parentDom + internal.props._parentDom !== parentDom ) { - parentDom = newVNode.props._parentDom; + parentDom = internal.props._parentDom; prevStartDom = startDom; startDom = null; } const renderResult = mountComponent(internal, startDom); - if (renderResult === startDom) { + // if (renderResult === startDom) { + if (renderResult === null) { nextDomSibling = startDom; } else { nextDomSibling = mountChildren( @@ -218,38 +218,41 @@ function mountElement(internal, dom) { /** * Mount all children of an Internal - * @param {import('../internal').Internal} internal The parent Internal of the given children + * @param {import('../internal').Internal} parentInternal The parent Internal of the given children * @param {import('../internal').ComponentChild[]} children * @param {import('../internal').PreactElement} parentDom The element into which this subtree is rendered * @param {import('../internal').PreactNode} startDom */ -export function mountChildren(internal, children, parentDom, startDom) { - let internalChildren = (internal._children = []), - i, - childVNode, - childInternal, +export function mountChildren(parentInternal, children, parentDom, startDom) { + let i, + /** @type {import('../internal').Internal} */ + prevInternal, + /** @type {import('../internal').Internal} */ + internal, + vnode, newDom, mountedNextChild; for (i = 0; i < children.length; i++) { - childVNode = normalizeToVNode(children[i]); + vnode = children[i]; - // Terser removes the `continue` here and wraps the loop body - // in a `if (childVNode) { ... } condition - if (childVNode == null) { - internalChildren[i] = null; - continue; - } + // account for holes by incrementing the index: + if (vnode == null || vnode === true || vnode === false) continue; + else if (Array.isArray(vnode)) vnode = createElement(Fragment, null, vnode); + else if (typeof vnode !== 'object') vnode = String(vnode); + + internal = createInternal(vnode, parentInternal); + internal._index = i; - childInternal = createInternal(childVNode, internal); - internalChildren[i] = childInternal; + if (prevInternal) prevInternal._next = internal; + else parentInternal._child = internal; // Morph the old element into the new one, but don't append it to the dom yet - mountedNextChild = mount(childInternal, childVNode, parentDom, startDom); + mountedNextChild = mount(internal, parentDom, startDom); - newDom = childInternal.data; + newDom = internal.data; - if (childInternal.flags & TYPE_COMPONENT || newDom == startDom) { + if (internal.flags & TYPE_COMPONENT || newDom == startDom) { // If the child is a Fragment-like or if it is DOM VNode and its _dom // property matches the dom we are diffing (i.e. startDom), just // continue with the mountedNextChild @@ -261,19 +264,17 @@ export function mountChildren(internal, children, parentDom, startDom) { parentDom.insertBefore(newDom, startDom); } - if (childInternal.ref) { - applyRef( - childInternal.ref, - childInternal._component || newDom, - childInternal - ); + if (internal.ref) { + applyRef(internal.ref, internal._component || newDom, internal); } + + prevInternal = internal; } // Remove children that are not part of any vnode. if ( - internal.flags & (MODE_HYDRATE | MODE_MUTATIVE_HYDRATE) && - internal.flags & TYPE_ELEMENT + parentInternal.flags & (MODE_HYDRATE | MODE_MUTATIVE_HYDRATE) && + parentInternal.flags & TYPE_ELEMENT ) { // TODO: Would it be simpler to just clear the pre-existing DOM in top-level // render if render is called with no oldVNode & existing children & no @@ -292,7 +293,7 @@ export function mountChildren(internal, children, parentDom, startDom) { /** * @param {import('../internal').Internal} internal The component's backing Internal node * @param {import('../internal').PreactNode} startDom the preceding node - * @returns {import('../internal').PreactNode} the component's children + * @returns {import('../internal').ComponentChild[]} the component's children */ function mountComponent(internal, startDom) { /** @type {import('../internal').Component} */ @@ -393,7 +394,8 @@ function mountComponent(internal, startDom) { } if (renderResult == null) { - return startDom; + // return startDom; + return null; } if (typeof renderResult == 'object') { diff --git a/src/diff/patch.js b/src/diff/patch.js index 0dcec97e5c..f0c522719e 100644 --- a/src/diff/patch.js +++ b/src/diff/patch.js @@ -1,4 +1,4 @@ -import { patchChildren, insertComponentDom } from './children'; +import { patchChildren, insert } from './children'; import { setProperty } from './props'; import options from '../options'; import { @@ -27,7 +27,7 @@ import { commitQueue } from './commit'; /** * Diff two virtual nodes and apply proper changes to the DOM * @param {import('../internal').Internal} internal The Internal node to patch - * @param {import('../internal').VNode | string} vnode The new virtual node + * @param {import('../internal').ComponentChild} vnode The new virtual node * @param {import('../internal').PreactElement} parentDom The element into which this subtree is rendered */ export function patch(internal, vnode, parentDom) { @@ -56,10 +56,10 @@ export function patch(internal, vnode, parentDom) { if (flags & TYPE_ROOT) { parentDom = vnode.props._parentDom; - if (internal.props._parentDom !== vnode.props._parentDom) { + if (internal.props._parentDom !== parentDom) { let nextSibling = parentDom == prevParentDom ? getDomSibling(internal) : null; - insertComponentDom(internal, nextSibling, parentDom); + insert(internal, parentDom, nextSibling); } } @@ -90,7 +90,7 @@ export function patch(internal, vnode, parentDom) { if (vnode._vnodeId !== internal._vnodeId) { internal.flags &= ~DIRTY_BIT; } - } else if (internal._children == null) { + } else if (internal._child == null) { let siblingDom = (internal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) === (MODE_HYDRATE | MODE_SUSPENDED) @@ -180,7 +180,8 @@ function patchElement(internal, vnode) { if (!oldHtml || (value !== oldHtml.__html && value !== dom.innerHTML)) { dom.innerHTML = value; } - internal._children = null; + // @TODO - unmount?? + internal._child = null; } else { if (oldHtml) dom.innerHTML = ''; patchChildren( diff --git a/src/diff/props.js b/src/diff/props.js index 24cc19ba23..c904a70bdb 100644 --- a/src/diff/props.js +++ b/src/diff/props.js @@ -102,15 +102,29 @@ export function setProperty(dom, name, value, oldValue, isSvg) { } } +export let inEvent = false; + /** * Proxy an event to hooked event handlers * @param {Event} e The event object from the browser * @private */ function eventProxy(e) { - return this._listeners[e.type + false](options.event ? options.event(e) : e); + inEvent = true; + try { + return this._listeners[e.type + false]( + options.event ? options.event(e) : e + ); + } finally { + inEvent = false; + } } function eventProxyCapture(e) { - return this._listeners[e.type + true](options.event ? options.event(e) : e); + inEvent = true; + try { + return this._listeners[e.type + true](options.event ? options.event(e) : e); + } finally { + inEvent = false; + } } diff --git a/src/diff/unmount.js b/src/diff/unmount.js index 0b28db91da..84793a09f3 100644 --- a/src/diff/unmount.js +++ b/src/diff/unmount.js @@ -7,19 +7,19 @@ import { ENABLE_CLASSES } from '../component'; /** * Unmount a virtual node from the tree and apply DOM changes * @param {import('../internal').Internal} internal The virtual node to unmount - * @param {import('../internal').Internal} parentInternal The parent of the VNode that - * initiated the unmount + * @param {import('../internal').Internal} topUnmountedInternal The top of the + * subtree that is being unmounted * @param {number} [skipRemove] Flag that indicates that a parent node of the * current element is already detached from the DOM. */ -export function unmount(internal, parentInternal, skipRemove) { +export function unmount(internal, topUnmountedInternal, skipRemove) { let r, i = 0; if (options.unmount) options.unmount(internal); internal.flags |= MODE_UNMOUNTING; if ((r = internal.ref)) { - applyRef(r, null, parentInternal); + applyRef(r, null, topUnmountedInternal); } if ((r = internal._component)) { @@ -29,20 +29,19 @@ export function unmount(internal, parentInternal, skipRemove) { try { r.componentWillUnmount(); } catch (e) { - options._catchError(e, parentInternal); + options._catchError(e, topUnmountedInternal); } } } - if ((r = internal._children)) { - for (; i < r.length; i++) { - if (r[i]) { - unmount( - r[i], - parentInternal, - skipRemove ? ~internal.flags & TYPE_ROOT : internal.flags & TYPE_DOM - ); - } + if ((r = internal._child)) { + while (r) { + unmount( + r, + topUnmountedInternal, + skipRemove ? ~internal.flags & TYPE_ROOT : internal.flags & TYPE_DOM + ); + r = r._next; } } diff --git a/src/diff/vdom-children-diffing.md b/src/diff/vdom-children-diffing.md new file mode 100644 index 0000000000..7530137061 --- /dev/null +++ b/src/diff/vdom-children-diffing.md @@ -0,0 +1,236 @@ +# Virtual DOM Children diffing + +## Introduction + +One problem all virtual dom implementations must solve is diffing children: Given the list of the current children, and the list of the new children, which children moved? To do this, there are generally 3 steps: + +1. Match up nodes between current and new children +2. Determine which nodes are added, removed, or changed positions +3. Apply changes to the DOM + +For the first step, virtual dom libraries need a way to match the current children to the new children. When children are a dynamically generated array, this matching is typically done using user-provided keys on the each child. If a user doesn't provide keys on children, then the key is typically the index of the child. Our discussion today will focus on the algorithms to determine which children moved when each child is identified using a key. + +For example, you may have a virtual tree such as: + +```jsx +
    +
  • a
    • +
  • b
    • +
  • c
    • +
  • d
    • +
+``` + +And a next render might produce this tree instead: + +```jsx +
    +
  • d
    • +
  • a
    • +
  • b
    • +
  • c
    • +
+``` + +Determining how to morph the original children of the `ul` element into the newly rendered children is the job your virtual DOM library. + +For here one out, will will simplify our discussion of virtual dom children and just refer to the list of keys. So we would represent the first JSX example as `0 1 2 3` and the new rendered output as `3 0 1 2`. + +## Determining movement + +Given a list of current children: `0 1 2 3` and new children `3 0 1 2`, what nodes would you move to make the current children match the new children? One approach could be to append `0` after `3` (`1 2 3 0`), append `1` after `0` (`2 3 0 1`), and then append `2` after `1` (`3 0 1 2`). However, a simpler approach would be to just insert `3` before `0` (`3 0 1 2`). + +When determining which nodes to move, our goal is minimize the number of nodes that move. Compared to other operations virtual dom libraries do, moving DOM nodes is an expensive operation, so reducing DOM node moves increases performance. + +How do we determine the fewest number of movements? Looking at our previous example (`0 1 2 3` -> `3 0 1 2`), the key to seeing that we can just move the `3` is noticing that `0 1 2` doesn't change between the old and new children. Only the `3` changes! `0 1 2` is the _longest common substring_ between the children (here, "substring" refers to contiguous sequence of items in our children array, i.e. a subset of items that are next to each other). Holding the longest common substring constant and moving elements around it helped us identify smaller number of movements to morph `0 1 2 3` into `3 0 1 2`. + +But let's look at another example: `0 1 2 3 4 5` -> `0 3 1 4 2 5`. Here, there are no common substrings! But if you look closely we can morph the old children into the new children in just 2 moves: insert `3` before `1` and insert `4` before `2`. How did we determine that? + +While there aren't any common substrings between the children arrays, there are common _subsequences_. A subsequence is set of elements from an array that are in the same order as the original sequence, but aren't necessarily next to each other. So while in a _substring_ the items must be next to each other, in a _subsequence_ they do not. For example, `0 3 5`, `0 1 5`, and `2 4` are a subsequences of `0 1 2 3 4 5`. `0 2 1` is not a subsequence of `0 1 2 3 4 5` because the `2` does not occur after `0` and before `1` in the original sequence. + +In our example above (`0 1 2 3 4 5` -> `0 3 1 4 2 5`), let's use our understanding of subsequences to examine the differences between the two children arrays. If we remove the two nodes that moved (`3` and `4`) from the arrays, we are left a common subsequence between the two arrays: `0 1 2 5`. This subsequence is actually the _longest common subsequence_ between the two arrays! + +If we can identify _longest common subsequence_ between two arrays, we have identified the most nodes that we can hold in place (i.e. **not move**). Every other node has changed positions and can move around these nodes. This approach leads to the minimal number of moves because we have found the most nodes that we don't have to move. Said another way, we have found the longest sequence of _relatively_ in place nodes, i.e. nodes that are in the same order with relationship to other nodes in the array. + +The longest common subsequence algorithm is a well-known algorithm in computer science. You can read more about [the longest common subsequence algorithm on Wikipedia](https://en.wikipedia.org/wiki/Longest_common_subsequence). The best time complexity for determining the longest common subsequence is $O(n^2)$. This time complexity is acceptable, but can we do better for our specific use case of diffing virtual DOM children? + +## Longest increasing subsequence + +Typically, virtual dom keys are strings. A more realistic example might look something like trying to morph `a c b e d f` -> `a b c d e f`. Let's map this to numbers using the indices of each key in the original array. Let's assign to each key its index in the original array: `{a: 0, c: 1, b: 2, e: 3, d: 4, f: 5}`. Now, let's replace each key with this number in both arrays. So the original array becomes `0 1 2 3 4 5` and the new array becomes `0 2 1 4 3 5`. + +```text +Original array + key: a c b e d f +index: 0 1 2 3 4 5 + +New array + key: a b c d e f +original index: 0 2 1 4 3 5 +``` + +Two interesting properties emerge from doing this: + +1. The original array will always be an array of numbers from `0` to `array.length - 1`, and will always be numbers sorted in increasing order. +2. The new array is a mapping of an item's new index to it's old index in the original array. Said another way, the item at index `0` in `0 2 1 4 3 5` (`a`) was at index `0` in the original way, the item at index `1` in the new array (`b`) was at index `2` in the original array, and so on. + +We can use these properties to simplify the problem we are trying to solve. Remember, we are searching for items that are in the relatively same order between the two arrays (in the same order relative to each other). Since the original array will always be in increasing sorted order, any subsequence of the original array that is present in the new array will also be in increasing order. + +This insight is big! We can simplify what we need to search for now. Instead of solving the general _longest common subsequence_ problem, we now only need to look for the longest subsequence of increasing numbers in the new array. Again, any subsequence of the new array that is in increasing order is also a subsequence of the original array which was in increasing sorted order. + +Finding the longest increasing subsequence is an easier algorithm because we are only looking at one array (the new array with the mapped indices) and has a better time complexity: $O(n\log(n))$. Let's take a look at the algorithm to do this. + +### Basic dynamic programming algorithm + +Let's start by writing a simple loop to find and track the first increasing subsequence to get us started. In this code we'll use the acronym LIS (or `lis`) to represent "longest increasing subsequence". + +```js +/** + * Longest increasing subsequence (LIS) algorithm + * @param {number[]} nums An array of numbers to find the longest increasing subsequence + * @returns {number[]} The items in the longest increasing subsequence of nums + */ +function findLIS(nums) { + // If there are no items, just return an empty array + if (nums.length < 0) { + return []; + } + + // There is at least one item in the array, so let's get started by adding it to + // start our longest increasing subsequence. + /** @type {number[]} The indicies of the items in the longest increasing subsequence */ + const lisIndices = [0]; + + for (let i = 1; i < nums.length; i++) { + /** The next value from nums to consider */ + const nextValue = nums[i]; + + /** Last index of current longest increasing subsequence */ + const lastIndex = lisIndices[lisIndices.length - 1]; + /** Last value in our current longest increasing subsequence */ + const lastValue = nums[lastIndex]; + + if (lastValue < nextValue) { + // The next value in nums is greater than the last value in our current + // increasing subsequence. Let's tack this index at the end of the + // sequence + lisIndices.push(i); + } else { + // We'll come back to this part + } + } + + // Now that lisIndices has all the indices of our increasing subsequence, + // let's add those items to an array and return the result. + const result = []; + for (let index of lisIndices) { + result.push(nums[index]); + } + + return result; +} +``` + +This code currently only finds the first increasing subsequence starting at the first item. To fix it to find the longest increasing subsequence, let's walk through some examples and talk about what we should change. + +#### Example 1: 0 8 3 6 + +Let's run the array `0 8 3 6` through our algorithm: + +1. `nums.length` is `4` so we continue on +2. We initialize `lisIndices` to `[0]` and start our loop at `1` +3. Loop iteration `i=1`: + `lisIndices` is currently `[0]` + 1. We initialize some variables: + `nextValue = 8` (`nums[i]`) + `lastIndex = 0` (`lisIndices[lisIndices.length - 1]`) + `lastValue = 0` (`nums[lasIndex]`) + 2. `0 < 8 == true` (`lastValue < nextValue`) + The next value in our array is greater than the last value in our current increasing subsequence, so let's add it to the subsequence: + 3. `lisIndices.push(1)` +4. Loop iteration `i=2` + `lisIndices` is currently `[0, 1]` + + 1. Initialize variables + `nextValue = 3` (`nums[i]`) + `lastIndex = 1` (`lisIndices[lisIndices.length - 1]`) + `lastValue = 8` (`nums[lasIndex]`) + 2. `8 < 3 == false` (`lastValue < nextValue`) + + TODO: Here we've reached our first + +#### Example 2: 1 4 5 6 2 3 + +## Algorithm + +Design decisions: + +1. Use linked list for perf and memory reasons +2. Iterate backwards cuz `insertBefore` is the API available on `Node` in DOM. We can't use `after` or `before` cuz DocumentFragment and Document don't support it. + +TODO: Talk about `insertBefore` and how DOM objects such as Document and DocumentFragment only support `insertBefore` and not newer methods such as `after` or `before` since they are only containers. You can't place a node after the document which is the root container. This limitation motivates using `prev` pointers in our algorithm. + +For understanding this algorithm, we are going to use a linked list to represent our array of children. Here is the structure of the linked list node we will use: + +```ts +interface Node { + /** The position of this node in the original list of children */ + index: number; + next: Node | null; +} +``` + +TODO: finish + +## Adding new nodes and deletions + +TODO: fill out + +## Summary + +Basic algorithm for diffing virtual dom children: + +1. Find matches between current and new children +2. Determine which children should move +3. Unmount, mount, and move children to match the new order + +When determining which children should move, we want to minimize the number of children that move (moving DOM nodes can be expensive). To do this, we need to determine longest subsequence of children that didn't move. One way to do this is to find the longest common subsequence between the current children and next children. However, the longest common subsequence runs in $O(n^2)$ time. + +If we map the array of current and next children into new arrays using their indices from the current children array, we can run a more efficient algorithm. For example, given current children of `a c b e d f` and next children of `a b c d e f`, we use the current children array to create a map of item to index in that array: `{a: 0, c: 1, b: 2, e: 3, d: 4, f: 5}`. Now create arrays using this map. + +```text +Current children + key: a c b e d f +index: 0 1 2 3 4 5 + +New children + key: a b c d e f +original index: 0 2 1 4 3 5 +``` + +Because the original array will always be sorted in increasing order, any common subsequence between the current and new children will also be increasing. Said another way, any increasing subsequence of the new children array is also a subsequence in the current children. So if we find the longest increasing subsequence in the new children array, we've found the longest common subsequence between the two arrays! Looking for the longest increasing subsequence, we no longer need to compare the two arrays and can instead just look at the new children array. + +To find the longest increasing subsequence, we need to keep track of two things as we traverse the array: + +1. The node/index that ends the smallest (in value) subsequence of each length + + For example, the subsequence `0 1 2` is smaller than `4 5 6` since the values in the subsequence are smaller + +2. The complete subsequence for each length + +For #1, let's take a look at two examples, `0 8 3 6` and `4 5 3 2`. Upon traversing over the first two elements (`0 8` in the first example and `4 5` in the second), we add them to our current increasing subsequence since they are all increasing. However, when we get to `3`, we need to decide whether to throw away our existing subsequence to include `3` or not. In first example, we should use `3` because using `3` gives us a longer subsequence `0 3 6`. But in the second example, we should not because `4 5` is the longest increasing subsequence. + +This decision is where #1 comes into play. After traversing the first two elements we end up with array `[0, 1]` signaling that the smallest increasing subsequence of length 1 starts at index `0`, the smallest subsequence of length 2 starts at index `2`, and so on. Upon reaching the value `3`, we search through our existing array to see if it can form start a smaller subsequence. + +In the first example, `3 < 8` so we replace the `8`'s index (`1`) with `3`'s index: `[0, 2]`. When we get to `6`, the tip of our subsequence is the value at index `2` (`3`). Since `6 > 3`, we add to our subsequence. + +In the second example, `3 < 4` so we replace `4`'s index (`0`) with `3`'s index: `[2, 1]`. However, we have now broken our subsequence. `3` doesn't come before `5` in the original array. Doing this is okay though because this array we are using is only keeping track of the index that starts the subsequence at that length. + +And this is where #2 comes into play. Since #1 only keeps tracks of the start of an increasing subsequence, we use an additional data structure to keep track of what the actual subsequence that starts at that index is. This data structure can be an array of the index that comes before current index in its increasing subsequence, or if using a linked list, a pointer to the previous node in its increasing subsequence. + +In Preact, we use linked lists to traverse the virtual DOM tree for better memory and performance. Also, to support DOM objects such as Document and DocumentFragment, we only use the `insertBefore` method to move DOM nodes around. Because `insertBefore` requires knowing the node that comes after the node to insert, we loop through children backwards, setting up a nodes next sibling before itself. + +## Acknowledgements + +- @localvoid and his work in `ivi` +- Domenic and his work in inferno +- leetcode editors for the great descriptions of these algorithms on their website diff --git a/src/internal.d.ts b/src/internal.d.ts index 379a28fdaa..7b1037e229 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -31,14 +31,14 @@ export interface Options extends preact.Options { parent: Element | Document | ShadowRoot | DocumentFragment ): void; /** Attach a hook that is invoked before a vnode is diffed. */ - _diff?(internal: Internal, vnode?: VNode | string): void; + _diff?(internal: Internal, vnode: VNode | string | null): void; /** Attach a hook that is invoked after a tree was mounted or was updated. */ _commit?(internal: Internal, commitQueue: CommitQueue): void; /** Attach a hook that is invoked before a vnode has rendered. */ _render?(internal: Internal): void; /** Attach a hook that is invoked before a hook's state is queried. */ _hook?(component: Component, index: number, type: HookType): void; - /** Bypass effect execution. Currenty only used in devtools for hooks inspection */ + /** Bypass effect execution. Currently only used in devtools for hooks inspection */ _skipEffects?: boolean; /** Attach a hook that is invoked after an error is caught in a component but before calling lifecycle hooks */ _catchError?(error: any, internal: Internal): void; @@ -93,7 +93,7 @@ export type Root = { }; export interface PreactElement extends HTMLElement { - _children?: Internal | null; + _child?: Internal | null; _root?: Root | null; /** Event listeners to support event delegation */ _listeners?: Record void>; @@ -145,10 +145,13 @@ export interface Internal

{ /** The function that triggers in-place re-renders for an internal */ rerender: (internal: Internal) => void; - /** children Internal nodes */ - _children: Internal[]; - /** next sibling Internal node */ + /** parent Internal node */ _parent: Internal; + /** first child Internal node */ + _child: Internal | null; + /** next sibling Internal node */ + _next: Internal | null; + _index: number; /** most recent vnode ID */ _vnodeId: number; /** diff --git a/src/render.js b/src/render.js index af50960452..21323efcdd 100644 --- a/src/render.js +++ b/src/render.js @@ -11,8 +11,8 @@ export function render(vnode, parentDom) { if (!root) { root = createRoot(parentDom); } - root.render(vnode); parentDom._root = root; + root.render(vnode); } /** @@ -26,6 +26,6 @@ export function hydrate(vnode, parentDom) { if (!root) { root = createRoot(parentDom); } - root.hydrate(vnode); parentDom._root = root; + root.hydrate(vnode); } diff --git a/src/tree.js b/src/tree.js index cd0ec2e36b..cb53501139 100644 --- a/src/tree.js +++ b/src/tree.js @@ -9,7 +9,8 @@ import { TYPE_COMPONENT, TYPE_DOM, MODE_SVG, - UNDEFINED + UNDEFINED, + INSERT_INTERNAL } from './constants'; import { enqueueRender } from './component'; @@ -81,90 +82,133 @@ export function createInternal(vnode, parentInternal) { } /** @type {import('./internal').Internal} */ - const internal = { + // const internal = { + // type, + // props, + // key, + // ref, + // _prevRef: null, + // data: + // flags & TYPE_COMPONENT + // ? { _commitCallbacks: [], _stateCallbacks: [] } + // : null, + // rerender: enqueueRender, + // flags, + // _parent: parentInternal, + // _child: null, + // _next: null, + // _index: -1, + // _vnodeId: vnodeId, + // _component: null, + // _context: null, + // _depth: parentInternal ? parentInternal._depth + 1 : 0 + // }; + const internal = new Internal( type, props, key, ref, - _prevRef: null, - data: - flags & TYPE_COMPONENT - ? { _commitCallbacks: [], _stateCallbacks: [] } - : null, - rerender: enqueueRender, flags, - _children: null, - _parent: parentInternal, - _vnodeId: vnodeId, - _component: null, - _context: null, - _depth: parentInternal ? parentInternal._depth + 1 : 0 - }; + parentInternal, + vnodeId + ); if (options._internal) options._internal(internal, vnode); return internal; } +class Internal { + constructor(type, props, key, ref, flags, parentInternal, vnodeId) { + this.type = type; + this.props = props; + this.key = key; + this.ref = ref; + this._prevRef = null; + this.data = + flags & TYPE_COMPONENT + ? { _commitCallbacks: [], _stateCallbacks: [] } + : null; + this.rerender = enqueueRender; + this.flags = flags; + this._parent = parentInternal; + this._child = null; + this._next = null; + this._index = -1; + this._vnodeId = vnodeId; + this._component = null; + this._context = null; + this._depth = parentInternal ? parentInternal._depth + 1 : 0; + } +} + +/** @type {(internal: import('./internal').Internal) => boolean} */ const shouldSearchComponent = internal => internal.flags & TYPE_COMPONENT && (!(internal.flags & TYPE_ROOT) || - internal.props._parentDom == getParentDom(internal._parent)); + (internal._parent && + internal.props._parentDom == getParentDom(internal._parent))); /** + * Get the next DOM Internal after a given index within a parent Internal. + * If `childIndex` is `null`, finds the next DOM Internal sibling of the given Internal. * @param {import('./internal').Internal} internal - * @param {number | null} [childIndex] + * @param {never} [childIndex] todo - replace parent+index with child internal reference * @returns {import('./internal').PreactNode} */ export function getDomSibling(internal, childIndex) { - if (childIndex == null) { - // Use childIndex==null as a signal to resume the search from the vnode's sibling - return getDomSibling( - internal._parent, - internal._parent._children.indexOf(internal) + 1 - ); - } - - let childDom = getChildDom(internal, childIndex); - if (childDom) { - return childDom; + if (internal._next) { + let childDom = getFirstDom(internal._next); + if (childDom) { + return childDom; + } } - // If we get here, we have not found a DOM node in this vnode's children. We - // must resume from this vnode's sibling (in it's parent _children array). - // Only climb up and search the parent if we aren't searching through a DOM - // VNode (meaning we reached the DOM parent of the original vnode that began - // the search). Note, the top of the tree has _parent == null so avoiding that - // here. - return internal._parent && shouldSearchComponent(internal) - ? getDomSibling(internal) + // If we get here, we have not found a DOM node in this internal's siblings so + // we need to search up through the parent's sibling's sub tree for a DOM + // Node, assuming the parent's siblings should be searched. If the parent is + // DOM node or a root node with a different parentDOM, we shouldn't search + // through it since any siblings we'd find wouldn't be siblings of the current + // internal's DOM + return internal._parent && shouldSearchComponent(internal._parent) + ? getDomSibling(internal._parent) : null; } /** - * @param {import('./internal').Internal} internal - * @param {number} offset + * Search the given internal and it's next siblings for a DOM element. If the + * provided Internal _is_ a DOM Internal, its DOM will be returned. If you want + * to find the first DOM node of an Internal's subtree, than pass in + * `internal._child` + * @param {import('./internal').Internal} internal The internal to begin the + * search * @returns {import('./internal').PreactElement} */ -export function getChildDom(internal, offset) { - if (internal._children == null) { - return null; - } +export function getFirstDom(internal) { + while (internal) { + if (internal.flags & INSERT_INTERNAL || internal._index == -1) { + internal = internal._next; + continue; + } - for (; offset < internal._children.length; offset++) { - let child = internal._children[offset]; - if (child != null) { - if (child.flags & TYPE_DOM) { - return child.data; - } - - if (shouldSearchComponent(child)) { - let childDom = getChildDom(child, 0); - if (childDom) { - return childDom; - } - } + // this is a DOM internal + if (internal.flags & TYPE_DOM) { + // @ts-ignore this is an Element Internal, .data is a PreactElement. + return internal.data; } + + // This is a Component internal (but might be a root/portal). + // Find its first DOM child, unless it's a portal: + // @todo - this is an inlined version of shouldSearchComponent without the type=component guard. + if ( + !(internal.flags & TYPE_ROOT) || + internal.props._parentDom == getParentDom(internal._parent) + ) { + const childDom = getFirstDom(internal._child); + if (childDom) return childDom; + } + + internal = internal._next; } return null; @@ -184,23 +228,23 @@ export function getParentContext(internal) { } /** + * Get the parent DOM for an Internal. If the Internal is a Root, returns its DOM root. * @param {import('./internal').Internal} internal * @returns {import('./internal').PreactElement} */ export function getParentDom(internal) { - let parent = internal; - // if this is a Root internal, return its parent DOM: - if (parent.flags & TYPE_ROOT) { - return parent.props._parentDom; + if (internal.flags & TYPE_ROOT) { + return internal.props._parentDom; } // walk up the tree to find the nearest DOM or Root Internal: - while ((parent = parent._parent)) { - if (parent.flags & TYPE_ROOT) { - return parent.props._parentDom; - } else if (parent.flags & TYPE_ELEMENT) { - return parent.data; + while ((internal = internal._parent)) { + if (internal.flags & TYPE_ELEMENT) { + return internal.data; + } + if (internal.flags & TYPE_ROOT) { + return internal.props._parentDom; } } } diff --git a/test/browser/createContext.test.js b/test/browser/createContext.test.js index 0502e2b557..d7f94ad3b5 100644 --- a/test/browser/createContext.test.js +++ b/test/browser/createContext.test.js @@ -881,8 +881,8 @@ describe('createContext', () => { expect(events).to.deep.equal([ 'render 0', 'mount 0', - 'render 1', 'unmount 0', + 'render 1', 'mount 1' ]); }); diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js index fdaf6d5be3..981f9c05f7 100644 --- a/test/browser/fragments.test.js +++ b/test/browser/fragments.test.js @@ -282,9 +282,9 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(div([div(1), span(2), span(2)])); expectDomLogToBe([ + '1.remove()', '

.insertBefore(#text, Null)', - '
122.insertBefore(
1, 1)', - '1.remove()' + '
22.insertBefore(
1, 2)' ]); }); @@ -404,9 +404,9 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); expect(scratch.innerHTML).to.equal('
Hello
'); expectDomLogToBe([ + '
Hello.remove()', '
.insertBefore(#text, Null)', - '
Hello.insertBefore(
Hello,
Hello)', - '
Hello.remove()' + '
.insertBefore(
Hello, Null)' ]); clearLog(); @@ -415,10 +415,10 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); expect(scratch.innerHTML).to.equal('
Hello
'); expectDomLogToBe([ + '
Hello.remove()', '
.insertBefore(#text, Null)', // Re-append the Stateful DOM since it has been re-parented - '
Hello.insertBefore(
Hello,
Hello)', - '
Hello.remove()' + '
.insertBefore(
Hello, Null)' ]); }); @@ -443,9 +443,9 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); expect(scratch.innerHTML).to.equal('
Hello
'); expectDomLogToBe([ + '
Hello.remove()', '
.insertBefore(#text, Null)', - '
Hello.insertBefore(
Hello,
Hello)', - '
Hello.remove()' + '
.insertBefore(
Hello, Null)' ]); clearLog(); @@ -454,9 +454,9 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); expect(scratch.innerHTML).to.equal('
Hello
'); expectDomLogToBe([ + '
Hello.remove()', '
.insertBefore(#text, Null)', - '
Hello.insertBefore(
Hello,
Hello)', - '
Hello.remove()' + '
.insertBefore(
Hello, Null)' ]); }); @@ -734,10 +734,10 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(htmlForFalse); expectDomLogToBe( [ - '
barHellobeep.insertBefore(
bar,
beep)', - '
Hellobarbeep.insertBefore(
Hello, Null)', + '
barHellobeep.insertBefore(
bar, Null)', + '
Hellobeepbar.insertBefore(
Hello, Null)', // TODO: does one operation too many here - '
barbeepHello.insertBefore(
bar, Null)' + '
beepbarHello.insertBefore(
bar, Null)' ], 'rendering true to false' ); @@ -787,12 +787,12 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); // Component should not have updated (empty op log) expect(scratch.innerHTML).to.equal(html); expectDomLogToBe([ + '1.remove()', + '
Hello.remove()', '.insertBefore(#text, Null)', - '
1Hello2.insertBefore(1, 1)', + '
2.insertBefore(1, 2)', '
.insertBefore(#text, Null)', - '
11Hello2.insertBefore(
Hello, 1)', - '1.remove()', - '
Hello.remove()' + '
12.insertBefore(
Hello, 2)' ]); clearLog(); @@ -801,12 +801,12 @@ describe('Fragment', () => { expect(ops).to.deep.equal([]); // Component should not have updated (empty op log) expect(scratch.innerHTML).to.equal(html); expectDomLogToBe([ + '1.remove()', + '
Hello.remove()', '.insertBefore(#text, Null)', - '
1Hello2.insertBefore(1, 1)', + '
2.insertBefore(1, 2)', '
.insertBefore(#text, Null)', - '
11Hello2.insertBefore(
Hello, 1)', - '1.remove()', - '
Hello.remove()' + '
12.insertBefore(
Hello, 2)' ]); }); @@ -869,9 +869,9 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal('foobar'); expectDomLogToBe([ - '
spamfoobar.insertBefore(#text, #text)', '#text.remove()', - '#text.remove()' + '#text.remove()', + '
foo.insertBefore(#text, Null)' ]); }); @@ -1032,6 +1032,7 @@ describe('Fragment', () => { }); it('should reorder Fragment children', () => { + /** @type {() => void} */ let updateState; class App extends Component { @@ -1324,9 +1325,16 @@ describe('Fragment', () => { htmlForFalse, 'rendering from true to false' ); + // A perfect algo can do this in two moves + // expectDomLogToBe([ + // '
    012345.insertBefore(
  1. 4,
  2. 0)', + // '
      401235.insertBefore(
    1. 5,
    2. 0)' + // ]); expectDomLogToBe([ - '
        012345.insertBefore(
      1. 4,
      2. 0)', - '
          401235.insertBefore(
        1. 5,
        2. 0)' + '
            012345.insertBefore(
          1. 0, Null)', + '
              123450.insertBefore(
            1. 1, Null)', + '
                234501.insertBefore(
              1. 2, Null)', + '
                  345012.insertBefore(
                1. 3, Null)' ]); clearLog(); @@ -1376,14 +1384,14 @@ describe('Fragment', () => { 'rendering from true to false' ); expectDomLogToBe([ + // Remove 1 & 2 (replaced with null) + '
                2. 0.remove()', + '
                3. 1.remove()', // Mount 3 & 4 '
                4. .insertBefore(#text, Null)', - '
                    0122.insertBefore(
                  1. 3, Null)', + '
                      22.insertBefore(
                    1. 3, Null)', '
                    2. .insertBefore(#text, Null)', - '
                        01223.insertBefore(
                      1. 4, Null)', - // Remove 1 & 2 (replaced with null) - '
                      2. 0.remove()', - '
                      3. 1.remove()' + '
                          223.insertBefore(
                        1. 4, Null)' ]); clearLog(); @@ -1393,14 +1401,14 @@ describe('Fragment', () => { 'rendering from false to true' ); expectDomLogToBe([ + // Remove 3 & 4 (replaced by null) + '
                        2. 3.remove()', + '
                        3. 4.remove()', // Insert 0 and 1 '
                        4. .insertBefore(#text, Null)', - '
                            2234.insertBefore(
                          1. 0,
                          2. 2)', + '
                              22.insertBefore(
                            1. 0,
                            2. 2)', '
                            3. .insertBefore(#text, Null)', - '
                                02234.insertBefore(
                              1. 1,
                              2. 2)', - // Remove 3 & 4 (replaced by null) - '
                              3. 3.remove()', - '
                              4. 4.remove()' + '
                                  022.insertBefore(
                                1. 1,
                                2. 2)' ]); }); @@ -1447,14 +1455,14 @@ describe('Fragment', () => { 'rendering from true to false' ); expectDomLogToBe([ + // Remove 1 & 2 (replaced with null) + '
                                3. 0.remove()', + '
                                4. 1.remove()', // Mount 4 & 5 '
                                5. .insertBefore(#text, Null)', - '
                                    0123.insertBefore(
                                  1. 4, Null)', + '
                                      23.insertBefore(
                                    1. 4, Null)', '
                                    2. .insertBefore(#text, Null)', - '
                                        01234.insertBefore(
                                      1. 5, Null)', - // Remove 1 & 2 (replaced with null) - '
                                      2. 0.remove()', - '
                                      3. 1.remove()' + '
                                          234.insertBefore(
                                        1. 5, Null)' ]); clearLog(); @@ -1464,14 +1472,14 @@ describe('Fragment', () => { 'rendering from false to true' ); expectDomLogToBe([ + // Remove 4 & 5 (replaced by null) + '
                                        2. 4.remove()', + '
                                        3. 5.remove()', // Insert 0 and 1 back into the DOM '
                                        4. .insertBefore(#text, Null)', - '
                                            2345.insertBefore(
                                          1. 0,
                                          2. 2)', + '
                                              23.insertBefore(
                                            1. 0,
                                            2. 2)', '
                                            3. .insertBefore(#text, Null)', - '
                                                02345.insertBefore(
                                              1. 1,
                                              2. 2)', - // Remove 4 & 5 (replaced by null) - '
                                              3. 4.remove()', - '
                                              4. 5.remove()' + '
                                                  023.insertBefore(
                                                1. 1,
                                                2. 2)' ]); }); @@ -1525,11 +1533,11 @@ describe('Fragment', () => { ); expectDomLogToBe( [ + '
                                                  boop.remove()', // TODO: this operation is too much - '
                                                  barHellobeepboop.insertBefore(
                                                  bar,
                                                  beep)', - '
                                                  Hellobarbeepboop.insertBefore(
                                                  Hello,
                                                  boop)', - '
                                                  barbeepHelloboop.insertBefore(
                                                  bar,
                                                  boop)', - '
                                                  boop.remove()' + '
                                                  barHellobeep.insertBefore(
                                                  bar, Null)', + '
                                                  Hellobeepbar.insertBefore(
                                                  Hello, Null)', + '
                                                  beepbarHello.insertBefore(
                                                  bar, Null)' ], 'rendering from true to false' ); @@ -1554,6 +1562,7 @@ describe('Fragment', () => { }); it('should swap nested fragments correctly', () => { + /** @type {() => void} */ let swap; class App extends Component { constructor(props) { @@ -1670,9 +1679,9 @@ describe('Fragment', () => { ); expectDomLogToBe( [ - '
                                                  barHellobeepbeepbeep.insertBefore(
                                                  bar,
                                                  beep)', - '
                                                  Hellobarbeepbeepbeep.insertBefore(
                                                  Hello, Null)', - '
                                                  barbeepbeepbeepHello.insertBefore(
                                                  bar, Null)' + '
                                                  barHellobeepbeepbeep.insertBefore(
                                                  bar, Null)', + '
                                                  Hellobeepbeepbeepbar.insertBefore(
                                                  Hello, Null)', + '
                                                  beepbeepbeepbarHello.insertBefore(
                                                  bar, Null)' ], 'rendering from true to false' ); @@ -1686,10 +1695,16 @@ describe('Fragment', () => { 'rendering from false to true' ); expectDomLogToBe( + // [ + // '
                                                  beepbeepbeepHellofoo.insertBefore(
                                                  Hello, Null)', + // '
                                                  beepbeepbeepfooHello.insertBefore(
                                                  foo,
                                                  beep)', + // '
                                                  foobeepbeepbeepHello.insertBefore(
                                                  Hello,
                                                  beep)' + // ], [ '
                                                  beepbeepbeepHellofoo.insertBefore(
                                                  Hello, Null)', - '
                                                  beepbeepbeepfooHello.insertBefore(
                                                  foo,
                                                  beep)', - '
                                                  foobeepbeepbeepHello.insertBefore(
                                                  Hello,
                                                  beep)' + '
                                                  boopbeepbeepfooHello.insertBefore(
                                                  boop, Null)', + '
                                                  boopbeepfooHelloboop.insertBefore(
                                                  boop, Null)', + '
                                                  boopfooHelloboopboop.insertBefore(
                                                  boop, Null)' ], 'rendering from false to true' ); @@ -1697,7 +1712,7 @@ describe('Fragment', () => { it('should correctly append children with siblings', () => { /** - * @type {(props: { values: Array}) => JSX.Element} + * @type {(props: { values: Array}) => preact.VNode} */ const Foo = ({ values }) => (
                                                    @@ -1778,12 +1793,12 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(htmlForFalse); expectDomLogToBe( [ + '
                                                    2.remove()', + '#text.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    1.insertBefore(
                                                    3, #text)', + '
                                                    .insertBefore(
                                                    3, Null)', '
                                                    .insertBefore(#text, Null)', - '
                                                    31.insertBefore(
                                                    4, #text)', - '#text.remove()', - '
                                                    2.remove()' + '
                                                    3.insertBefore(
                                                    4, Null)' ], 'rendering from true to false' ); @@ -1794,9 +1809,9 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(htmlForTrue); expectDomLogToBe( [ - '
                                                    34.insertBefore(#text,
                                                    3)', - '
                                                    4.remove()', '
                                                    3.remove()', + '
                                                    4.remove()', + '
                                                    .insertBefore(#text, Null)', '
                                                    .insertBefore(#text, Null)', '
                                                    1.insertBefore(
                                                    2, Null)' ], @@ -1980,6 +1995,7 @@ describe('Fragment', () => { it('should properly render Fragments whose last child is a component returning null', () => { let Noop = () => null; + /** @type {() => void} */ let update; class App extends Component { constructor(props) { @@ -2025,6 +2041,7 @@ describe('Fragment', () => { }); it('should replace node in-between children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2058,13 +2075,14 @@ describe('Fragment', () => { `
                                                    A
                                                    B2
                                                    C
                                                    ` ); expectDomLogToBe([ + '
                                                    B1.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    AB1C.insertBefore(
                                                    B2,
                                                    B1)', - '
                                                    B1.remove()' + '
                                                    AC.insertBefore(
                                                    B2,
                                                    C)' ]); }); it('should replace Fragment in-between children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2108,16 +2126,17 @@ describe('Fragment', () => { div([div('A'), section('B3'), section('B4'), div('C')]) ); expectDomLogToBe([ + '
                                                    B1.remove()', + '
                                                    B2.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    AB1B2C.insertBefore(
                                                    B3,
                                                    B1)', + '
                                                    AC.insertBefore(
                                                    B3,
                                                    C)', '
                                                    .insertBefore(#text, Null)', - '
                                                    AB3B1B2C.insertBefore(
                                                    B4,
                                                    B1)', - '
                                                    B2.remove()', - '
                                                    B1.remove()' + '
                                                    AB3C.insertBefore(
                                                    B4,
                                                    C)' ]); }); it('should insert in-between children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2155,6 +2174,7 @@ describe('Fragment', () => { }); it('should insert in-between Fragments', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2194,6 +2214,7 @@ describe('Fragment', () => { }); it('should insert in-between null children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2233,6 +2254,7 @@ describe('Fragment', () => { }); it('should insert Fragment in-between null children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2279,6 +2301,7 @@ describe('Fragment', () => { }); it('should insert in-between nested null children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2322,6 +2345,7 @@ describe('Fragment', () => { }); it('should insert Fragment in-between nested null children', () => { + /** @type {() => void} */ let update; class SetState extends Component { constructor(props) { @@ -2372,6 +2396,7 @@ describe('Fragment', () => { }); it('should update at correct place', () => { + /** @type {() => void} */ let updateA; class A extends Component { constructor(props) { @@ -2426,13 +2451,14 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.eql(`
                                                    A2
                                                    C
                                                    `); expectDomLogToBe([ + '
                                                    A.remove()', '.insertBefore(#text, Null)', - '
                                                    AC.insertBefore(A2,
                                                    A)', - '
                                                    A.remove()' + '
                                                    C.insertBefore(A2,
                                                    C)' ]); }); it('should update Fragment at correct place', () => { + /** @type {() => void} */ let updateA; class A extends Component { constructor(props) { @@ -2493,12 +2519,12 @@ describe('Fragment', () => { `
                                                    A3A4
                                                    C
                                                    ` ); expectDomLogToBe([ + '
                                                    A1.remove()', + '
                                                    A2.remove()', '.insertBefore(#text, Null)', - '
                                                    A1A2C.insertBefore(A3,
                                                    A1)', + '
                                                    C.insertBefore(A3,
                                                    C)', '.insertBefore(#text, Null)', - '
                                                    A3A1A2C.insertBefore(A4,
                                                    A1)', - '
                                                    A2.remove()', - '
                                                    A1.remove()' + '
                                                    A3C.insertBefore(A4,
                                                    C)' ]); }); @@ -2574,9 +2600,9 @@ describe('Fragment', () => { 'updateA' ); expectDomLogToBe([ + '
                                                    A.remove()', '.insertBefore(#text, Null)', - '
                                                    ABC.insertBefore(A2,
                                                    A)', - '
                                                    A.remove()' + '
                                                    BC.insertBefore(A2,
                                                    B)' ]); }); @@ -2631,12 +2657,12 @@ describe('Fragment', () => { 'updateA' ); expectDomLogToBe([ + '
                                                    A1.remove()', + '
                                                    A2.remove()', '.insertBefore(#text, Null)', - '
                                                    A1A2.insertBefore(A3,
                                                    A1)', + '
                                                    .insertBefore(A3, Null)', '.insertBefore(#text, Null)', - '
                                                    A3A1A2.insertBefore(A4,
                                                    A1)', - '
                                                    A2.remove()', - '
                                                    A1.remove()' + '
                                                    A3.insertBefore(A4, Null)' ]); clearLog(); @@ -2654,6 +2680,7 @@ describe('Fragment', () => { }); it('should properly place conditional elements around strictly equal vnodes', () => { + /** @type {() => void} */ let set; const Children = () => ( @@ -2705,9 +2732,9 @@ describe('Fragment', () => { rerender(); expect(scratch.innerHTML).to.equal(top); expectDomLogToBe([ + '
                                                    bottom panel.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    NavigationContentbottom panel.insertBefore(
                                                    top panel,
                                                    Navigation)', - '
                                                    bottom panel.remove()' + '
                                                    NavigationContent.insertBefore(
                                                    top panel,
                                                    Navigation)' ]); clearLog(); @@ -2715,9 +2742,9 @@ describe('Fragment', () => { rerender(); expect(scratch.innerHTML).to.equal(bottom); expectDomLogToBe([ + '
                                                    top panel.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    top panelNavigationContent.insertBefore(
                                                    bottom panel, Null)', - '
                                                    top panel.remove()' + '
                                                    NavigationContent.insertBefore(
                                                    bottom panel, Null)' ]); clearLog(); @@ -2725,9 +2752,9 @@ describe('Fragment', () => { rerender(); expect(scratch.innerHTML).to.equal(top); expectDomLogToBe([ + '
                                                    bottom panel.remove()', '
                                                    .insertBefore(#text, Null)', - '
                                                    NavigationContentbottom panel.insertBefore(
                                                    top panel,
                                                    Navigation)', - '
                                                    bottom panel.remove()' + '
                                                    NavigationContent.insertBefore(
                                                    top panel,
                                                    Navigation)' ]); }); @@ -2856,10 +2883,10 @@ describe('Fragment', () => { div([div(1), div(4), div('A'), div('B')]) ); expectDomLogToBe([ - '
                                                    .insertBefore(#text, Null)', - '
                                                    123AB.insertBefore(
                                                    4,
                                                    2)', '
                                                    2.remove()', - '
                                                    3.remove()' + '
                                                    3.remove()', + '
                                                    .insertBefore(#text, Null)', + '
                                                    1AB.insertBefore(
                                                    4,
                                                    A)' ]); }); @@ -2904,11 +2931,11 @@ describe('Fragment', () => { expect(scratch.innerHTML).to.equal(div([span(1), div('A'), div('B')])); expectDomLogToBe([ - '.insertBefore(#text, Null)', - '
                                                    123AB.insertBefore(1,
                                                    1)', + '
                                                    1.remove()', '
                                                    2.remove()', '
                                                    3.remove()', - '
                                                    1.remove()' + '.insertBefore(#text, Null)', + '
                                                    AB.insertBefore(1,
                                                    A)' ]); }); }); diff --git a/test/browser/getDomSibling.test.js b/test/browser/getDomSibling.test.js index 87e6c85f4c..6cde79800e 100644 --- a/test/browser/getDomSibling.test.js +++ b/test/browser/getDomSibling.test.js @@ -4,11 +4,17 @@ import { setupScratch, teardown } from '../_util/helpers'; /** @jsx createElement */ +/** + * @typedef {import('../../src/internal').Internal} Internal + * @typedef {import('../../src/internal').PreactElement} PreactElement + */ + describe('getDomSibling', () => { - /** @type {import('../../src/internal').PreactElement} */ + /** @type {PreactElement} */ let scratch; - const getRoot = dom => dom._children; + /** @type {(dom: PreactElement) => Internal} */ + const getRoot = dom => dom._child; const Root = props => props.children; const createPortal = (vnode, parent) => ( @@ -32,7 +38,7 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -45,7 +51,7 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -61,7 +67,24 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; + expect(getDomSibling(internal)).to.equalNode( + scratch.firstChild.childNodes[1] + ); + }); + + it('should find direct sibling through empty Components', () => { + render( +
                                                    +
                                                    A
                                                    + + +
                                                    B
                                                    +
                                                    , + scratch + ); + let internal = getRoot(scratch)._child._child; + expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -69,7 +92,7 @@ describe('getDomSibling', () => { it('should find text node sibling with placeholder', () => { render(
                                                    A{null}B
                                                    , scratch); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -84,7 +107,8 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; + expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -103,7 +127,8 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0]; + let internal = getRoot(scratch)._child._child._child; + expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -119,7 +144,7 @@ describe('getDomSibling', () => {
                                                    , scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0]; + let internal = getRoot(scratch)._child._child._child; expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -151,9 +176,9 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child._child._child._child; expect(divAInternal.type).to.equal('div'); + expect(divAInternal.key).to.equal('A'); expect(getDomSibling(divAInternal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -178,9 +203,9 @@ describe('getDomSibling', () => { scratch ); - let fragment = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]; + let fragment = getRoot(scratch)._child._child._child._child; expect(fragment.type).to.equal(Fragment); + expect(fragment.key).to.equal('0.0.0.0'); expect(getDomSibling(fragment)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -206,9 +231,9 @@ describe('getDomSibling', () => { scratch ); - let foo = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]; + let foo = getRoot(scratch)._child._child._child._child; expect(foo.type).to.equal(Foo); + expect(foo.key).to.equal('0.0.0.0'); expect(getDomSibling(foo)).to.equalNode(scratch.firstChild.childNodes[1]); }); @@ -229,8 +254,9 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child._child; expect(divAInternal.type).to.equal('div'); + expect(divAInternal.key).to.equal('A'); expect(getDomSibling(divAInternal)).to.equalNode( scratch.firstChild.childNodes[1] ); @@ -248,8 +274,9 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child; expect(divAInternal.type).to.equal('div'); + expect(divAInternal.key).to.equal('A'); let sibling = getDomSibling(divAInternal); expect(sibling).to.equalNode(scratch.firstChild.childNodes[1]); @@ -265,8 +292,9 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child; expect(divAInternal.type).to.equal('div'); + expect(divAInternal.key).to.equal('A'); let sibling = getDomSibling(divAInternal); expect(sibling).to.equalNode(scratch.firstChild.childNodes[1]); @@ -283,7 +311,7 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child._child; expect(divAInternal.key).to.equal('A'); let sibling = getDomSibling(divAInternal); @@ -306,8 +334,8 @@ describe('getDomSibling', () => { scratch ); - const divCInternal = getRoot(scratch)._children[0]._children[2] - ._children[0]; + const divCInternal = getRoot(scratch)._child._child._next._next._child; + expect(divCInternal.key).to.equal('C'); expect(getDomSibling(divCInternal)).to.equal(null); }); @@ -328,8 +356,8 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child._child._child._child; + expect(divAInternal.key).to.equal('A'); expect(getDomSibling(divAInternal)).to.equal(null); }); @@ -357,8 +385,8 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]._children[0]; + let divAInternal = getRoot(scratch)._child._child._child._child._child; + expect(divAInternal.key).to.equal('A'); expect(getDomSibling(divAInternal)).to.equal(null); }); @@ -378,7 +406,7 @@ describe('getDomSibling', () => { scratch ); - let divAInternal = getRoot(scratch)._children[0]._children[0]._children[1]; + let divAInternal = getRoot(scratch)._child._child._child._next; expect(divAInternal.key).to.equal('A'); expect(getDomSibling(divAInternal)).to.equal(null); }); @@ -386,7 +414,7 @@ describe('getDomSibling', () => { it("should return null if it's the only child", () => { render(
                                                    A
                                                    , scratch); - let internal = getRoot(scratch)._children[0]; + let internal = getRoot(scratch)._child; expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.be.null; }); @@ -407,7 +435,7 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0]; + let internal = getRoot(scratch)._child._child._child; expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode( scratch.firstChild.childNodes[1] @@ -428,7 +456,7 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0]; + let internal = getRoot(scratch)._child._child._child; expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode(scratch.childNodes[1]); }); @@ -447,7 +475,7 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0]; + let internal = getRoot(scratch)._child._child._child; expect(internal.key).to.equal('A'); expect(getDomSibling(internal)).to.equalNode(scratch.childNodes[1]); }); @@ -468,7 +496,8 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]; + let internal = getRoot(scratch)._child._child._next; + expect(internal.type).to.equal(Root); expect(internal.props._parentDom).to.equal(portalParent); expect(getDomSibling(internal)).to.equalNode(scratch.firstChild.lastChild); }); @@ -487,7 +516,8 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]; + let internal = getRoot(scratch)._child._child._next; + expect(internal.type).to.equal(Root); expect(internal.props._parentDom).to.equal(scratch); expect(getDomSibling(internal)).to.equalNode(scratch.lastChild); }); @@ -508,7 +538,7 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]._children[0]; + let internal = getRoot(scratch)._child._child._next._child; expect(internal.key).to.equal('B'); expect(getDomSibling(internal)).to.equal(null); }); @@ -537,7 +567,7 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]._children[0]; + let internal = getRoot(scratch)._child._child._next._child; expect(internal.key).to.equal('B'); expect(getDomSibling(internal)).to.equal(portalParent.lastChild); }); @@ -556,7 +586,10 @@ describe('getDomSibling', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]._children[0]; + let portalInternal = getRoot(scratch)._child._child._next; + let internal = portalInternal._child; + expect(portalInternal.type).to.equal(Root); + expect(portalInternal.props._parentDom).to.equalNode(scratch); expect(internal.key).to.equal('B'); expect(getDomSibling(internal)).to.equalNode(scratch.lastChild); }); diff --git a/test/browser/getParentDom.test.js b/test/browser/getParentDom.test.js index 237816ee95..7aa9c29e23 100644 --- a/test/browser/getParentDom.test.js +++ b/test/browser/getParentDom.test.js @@ -4,17 +4,36 @@ import { setupScratch, teardown } from '../_util/helpers'; /** @jsx createElement */ +/** + * @typedef {import('../../src/internal').Internal} Internal + * @typedef {import('../../src/internal').PreactElement} PreactElement + */ + describe('getParentDom', () => { - /** @type {import('../../src/internal').PreactElement} */ + /** @type {PreactElement} */ let scratch; - const getRoot = dom => dom._children; + /** @type {(dom: PreactElement) => Internal} */ + const getRoot = dom => dom._child; const Root = props => props.children; const createPortal = (vnode, parent) => ( {vnode} ); + /** + * @param {Internal} parent + * @param {(internal: Internal, index: number) => void} cb + */ + function forEachChild(parent, cb) { + let index = 0; + let child = parent._child; + while (child) { + cb(child, index++); + child = child._next; + } + } + beforeEach(() => { scratch = setupScratch(); }); @@ -34,11 +53,11 @@ describe('getParentDom', () => { scratch ); - let domInternals = getRoot(scratch)._children[0]._children; - for (let internal of domInternals) { + let parentInternal = getRoot(scratch)._child; + forEachChild(parentInternal, internal => { expect(internal.type).to.equal('div'); expect(getParentDom(internal)).to.equalNode(scratch.firstChild); - } + }); }); it('should find direct parent of text node', () => { @@ -49,12 +68,12 @@ describe('getParentDom', () => { scratch ); - let domInternals = getRoot(scratch)._children[0]._children; + let parent = getRoot(scratch)._child; let expectedTypes = ['div', null, 'div']; - for (let i = 0; i < domInternals.length; i++) { - expect(domInternals[i].type).to.equal(expectedTypes[i]); - expect(getParentDom(domInternals[i])).to.equalNode(scratch.firstChild); - } + forEachChild(parent, (internal, i) => { + expect(internal.type).to.equal(expectedTypes[i]); + expect(getParentDom(internal)).to.equalNode(scratch.firstChild); + }); }); it('should find parent through Fragments', () => { @@ -69,8 +88,8 @@ describe('getParentDom', () => { ); let domInternals = [ - getRoot(scratch)._children[0]._children[0]._children[0], - getRoot(scratch)._children[0]._children[1]._children[0] + getRoot(scratch)._child._child._child, + getRoot(scratch)._child._child._next._child ]; let expectedTypes = ['div', null]; @@ -97,8 +116,8 @@ describe('getParentDom', () => { ); let domInternals = [ - getRoot(scratch)._children[0]._children[0]._children[0]._children[0], - getRoot(scratch)._children[0]._children[1]._children[0]._children[0] + getRoot(scratch)._child._child._child._child, + getRoot(scratch)._child._child._next._child._child ]; let expectedTypes = ['div', null]; @@ -127,8 +146,8 @@ describe('getParentDom', () => { ); let domInternals = [ - getRoot(scratch)._children[0]._children[0]._children[0], - getRoot(scratch)._children[0]._children[1]._children[0] + getRoot(scratch)._child._child._child, + getRoot(scratch)._child._child._next._child ]; let expectedTypes = [Foo, Fragment]; @@ -150,8 +169,7 @@ describe('getParentDom', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[1]._children[0] - ._children[0]; + let internal = getRoot(scratch)._child._child._next._child._child; let parentDom = getParentDom(internal); expect(internal.type).to.equal('span'); @@ -175,8 +193,7 @@ describe('getParentDom', () => { scratch ); - let internal = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]; + let internal = getRoot(scratch)._child._child._child._child; let parent = getParentDom(internal); expect(internal.type).to.equal('p'); @@ -192,7 +209,7 @@ describe('getParentDom', () => { scratch ); - const internal = getRoot(scratch)._children[0]; + const internal = getRoot(scratch)._child; expect(internal.type).to.equal(Foo); expect(getParentDom(internal)).to.equal(scratch); }); @@ -215,7 +232,7 @@ describe('getParentDom', () => { expect(scratch.innerHTML).to.equal('
                                                    '); - let internal = getRoot(scratch)._children[0]._children[0]; + let internal = getRoot(scratch)._child._child; expect(internal.type).to.equal(Root); expect(getParentDom(internal)).to.equalNode(portalParent); }); @@ -238,11 +255,11 @@ describe('getParentDom', () => { expect(scratch.innerHTML).to.equal('
                                                    '); - let fooInternal = getRoot(scratch)._children[0]._children[0]._children[0]; + let fooInternal = getRoot(scratch)._child._child._child; expect(fooInternal.type).to.equal(Foo); expect(getParentDom(fooInternal)).to.equalNode(portalParent); - let divInternal = fooInternal._children[0]; + let divInternal = fooInternal._child; expect(divInternal.type).to.equal('div'); expect(getParentDom(divInternal)).to.equalNode(portalParent); }); @@ -260,8 +277,7 @@ describe('getParentDom', () => { expect(scratch.innerHTML).to.equal('
                                                    '); - let internal = getRoot(scratch)._children[0]._children[0]._children[0] - ._children[0]; + let internal = getRoot(scratch)._child._child._child._child; expect(internal.type).to.equal('div'); expect(getParentDom(internal)).to.equalNode(portalParent); }); diff --git a/test/browser/keys.test.js b/test/browser/keys.test.js index 34339d1bc6..3c8dffe3ca 100644 --- a/test/browser/keys.test.js +++ b/test/browser/keys.test.js @@ -92,6 +92,32 @@ describe('keys', () => { expect(Foo.args[0][0]).to.deep.equal({}); }); + it('should update in-place keyed DOM nodes', () => { + render( +
                                                      +
                                                    • a
                                                      • +
                                                    • b
                                                      • +
                                                    • c
                                                      • +
                                                    , + scratch + ); + expect(scratch.innerHTML).to.equal( + '
                                                    • a
                                                    • b
                                                    • c
                                                    ' + ); + + render( +
                                                      +
                                                    • x
                                                      • +
                                                    • y
                                                      • +
                                                    • z
                                                      • +
                                                    , + scratch + ); + expect(scratch.innerHTML).to.equal( + '
                                                    • x
                                                    • y
                                                    • z
                                                    ' + ); + }); + // See preactjs/preact-compat#21 it('should remove orphaned keyed nodes', () => { render( @@ -254,9 +280,53 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal('abcd'); expect(getLog()).to.deep.equal([ - '
                                                  1. z.remove()', + '
                                                  2. x.remove()', '
                                                  3. y.remove()', - '
                                                  4. x.remove()' + '
                                                  5. z.remove()' + ]); + }); + + it('should move keyed children to the beginning', () => { + const values = ['b', 'c', 'd', 'a']; + + render(, scratch); + expect(scratch.textContent).to.equal('bcda'); + + move(values, values.length - 1, 0); + clearLog(); + + render(, scratch); + expect(scratch.textContent).to.equal('abcd'); + // A perfect algorithm would do this in one move. Our algorithm is a compromise of size vs common case perf + // expect(getLog()).to.deep.equal(['
                                                      bcda.insertBefore(
                                                    1. a,
                                                    2. b)']); + expect(getLog()).to.deep.equal([ + '
                                                        bcda.insertBefore(
                                                      1. b, Null)', + '
                                                          cdab.insertBefore(
                                                        1. c, Null)', + '
                                                            dabc.insertBefore(
                                                          1. d, Null)' + ]); + }); + + it('should move multiple keyed children to the beginning', () => { + const values = ['c', 'd', 'e', 'a', 'b']; + + render(, scratch); + expect(scratch.textContent).to.equal('cdeab'); + + move(values, values.length - 1, 0); + move(values, values.length - 1, 0); + clearLog(); + + render(, scratch); + expect(scratch.textContent).to.equal('abcde'); + // A perfect algorithm would do this in two moves. Our algorithm is a compromise of size vs common case perf + // expect(getLog()).to.deep.equal([ + // '
                                                              cdeab.insertBefore(
                                                            1. a,
                                                            2. c)', + // '
                                                                acdeb.insertBefore(
                                                              1. b,
                                                              2. c)' + // ]); + expect(getLog()).to.deep.equal([ + '
                                                                  cdeab.insertBefore(
                                                                1. c, Null)', + '
                                                                    deabc.insertBefore(
                                                                  1. d, Null)', + '
                                                                      eabcd.insertBefore(
                                                                    1. e, Null)' ]); }); @@ -325,11 +395,52 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal('abcd', 'move to beginning'); expect(getLog()).to.deep.equal( - ['
                                                                        bcda.insertBefore(
                                                                      1. a,
                                                                      2. b)'], + // A perfect algorithm would do this in one move. Our algorithm is a compromise of size vs common case perf + // ['
                                                                          bcda.insertBefore(
                                                                        1. a,
                                                                        2. b)'], + [ + '
                                                                            bcda.insertBefore(
                                                                          1. b, Null)', + '
                                                                              cdab.insertBefore(
                                                                            1. c, Null)', + '
                                                                                dabc.insertBefore(
                                                                              1. d, Null)' + ], 'move to beginning' ); }); + it('should move keyed children to the beginning on longer list', () => { + // Preact v10 worst case + const values = ['a', 'b', 'c', 'd', 'e', 'f']; + + render(, scratch); + expect(scratch.textContent).to.equal('abcdef'); + + move(values, 4, 1); + clearLog(); + + render(, scratch); + expect(scratch.textContent).to.equal('aebcdf'); + // A perfect algorithm would do this in one move. Our algorithm is a compromise of size vs common case perf + // expect(getLog()).to.deep.equal(['
                                                                                  abcdef.insertBefore(
                                                                                1. e,
                                                                                2. b)']); + expect(getLog()).to.deep.equal([ + '
                                                                                    abcdef.insertBefore(
                                                                                  1. b,
                                                                                  2. f)', + '
                                                                                      acdebf.insertBefore(
                                                                                    1. c,
                                                                                    2. f)', + '
                                                                                        adebcf.insertBefore(
                                                                                      1. d,
                                                                                      2. f)' + ]); + }); + + it('should move keyed children to the end on longer list', () => { + const values = ['a', 'b', 'c', 'd', 'e', 'f']; + + render(, scratch); + expect(scratch.textContent).to.equal('abcdef'); + + move(values, 1, values.length - 2); + clearLog(); + + render(, scratch); + expect(scratch.textContent).to.equal('acdebf'); + expect(getLog()).to.deep.equal(['
                                                                                          abcdef.insertBefore(
                                                                                        1. b,
                                                                                        2. f)']); + }); + it('should reverse keyed children effectively', () => { const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j']; @@ -342,16 +453,17 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal(values.join('')); + // expect(getLog()).to.have.lengthOf(9); expect(getLog()).to.deep.equal([ - '
                                                                                            abcdefghij.insertBefore(
                                                                                          1. j,
                                                                                          2. a)', - '
                                                                                              jabcdefghi.insertBefore(
                                                                                            1. i,
                                                                                            2. a)', - '
                                                                                                jiabcdefgh.insertBefore(
                                                                                              1. h,
                                                                                              2. a)', - '
                                                                                                  jihabcdefg.insertBefore(
                                                                                                1. g,
                                                                                                2. a)', - '
                                                                                                    jihgabcdef.insertBefore(
                                                                                                  1. f,
                                                                                                  2. a)', - '
                                                                                                      jihgfabcde.insertBefore(
                                                                                                    1. e,
                                                                                                    2. a)', - '
                                                                                                        jihgfeabcd.insertBefore(
                                                                                                      1. d,
                                                                                                      2. a)', - '
                                                                                                          jihgfedabc.insertBefore(
                                                                                                        1. c,
                                                                                                        2. a)', - '
                                                                                                            jihgfedcab.insertBefore(
                                                                                                          1. a, Null)' + '
                                                                                                              abcdefghij.insertBefore(
                                                                                                            1. i, Null)', + '
                                                                                                                abcdefghji.insertBefore(
                                                                                                              1. h, Null)', + '
                                                                                                                  abcdefgjih.insertBefore(
                                                                                                                1. g, Null)', + '
                                                                                                                    abcdefjihg.insertBefore(
                                                                                                                  1. f, Null)', + '
                                                                                                                      abcdejihgf.insertBefore(
                                                                                                                    1. e, Null)', + '
                                                                                                                        abcdjihgfe.insertBefore(
                                                                                                                      1. d, Null)', + '
                                                                                                                          abcjihgfed.insertBefore(
                                                                                                                        1. c, Null)', + '
                                                                                                                            abjihgfedc.insertBefore(
                                                                                                                          1. b, Null)', + '
                                                                                                                              ajihgfedcb.insertBefore(
                                                                                                                            1. a, Null)' ]); }); @@ -464,8 +576,8 @@ describe('keys', () => { expect(scratch.innerHTML).to.equal(expectedHtml); expect(ops).to.deep.equal([ - 'Unmount Stateful2', 'Unmount Stateful1', + 'Unmount Stateful2', 'Mount Stateful1', 'Mount Stateful2' ]); @@ -477,8 +589,8 @@ describe('keys', () => { expect(scratch.innerHTML).to.equal(expectedHtml); expect(ops).to.deep.equal([ - 'Unmount Stateful2', 'Unmount Stateful1', + 'Unmount Stateful2', 'Mount Stateful1', 'Mount Stateful2' ]); @@ -609,8 +721,8 @@ describe('keys', () => { expect(scratch.innerHTML).to.equal(expectedHtml); expect(ops).to.deep.equal([ - 'Unmount Stateful2', 'Unmount Stateful1', + 'Unmount Stateful2', 'Mount Stateful1', 'Mount Stateful2' ]); @@ -622,8 +734,8 @@ describe('keys', () => { expect(scratch.innerHTML).to.equal(expectedHtml); expect(ops).to.deep.equal([ - 'Unmount Stateful2', 'Unmount Stateful1', + 'Unmount Stateful2', 'Mount Stateful1', 'Mount Stateful2' ]); diff --git a/test/browser/placeholders.test.js b/test/browser/placeholders.test.js index e69cad8554..5c0e3c4fae 100644 --- a/test/browser/placeholders.test.js +++ b/test/browser/placeholders.test.js @@ -304,4 +304,72 @@ describe('null placeholders', () => { expect(scratch.innerHTML).to.equal(div([div('false'), div('the middle')])); expect(getLog()).to.deep.equal(['#text.remove()', '#text.remove()']); }); + + it('should properly mount & unmount null placeholders from the start', () => { + /** @type {(props: { show: [boolean, boolean, boolean] }) => any} */ + function App({ show }) { + return ( +
                                                                                                                                + {show[0] &&
                                                                                                                              1. first
                                                                                                                                2. } + {show[1] &&
                                                                                                                              2. middle
                                                                                                                                3. } + {show[2] &&
                                                                                                                              3. last
                                                                                                                                4. } +
                                                                                                                              + ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                                                                                                                              1. last
                                                                                                                              '); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. middle
                                                                                                                              2. last
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. first
                                                                                                                              2. middle
                                                                                                                              3. last
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. middle
                                                                                                                              2. last
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                                                                                                                              1. last
                                                                                                                              '); + }); + + it('should properly mount && unmount null placeholders from the back', () => { + /** @type {(props: { show: [boolean, boolean, boolean] }) => any} */ + function App({ show }) { + return ( +
                                                                                                                                + {show[0] &&
                                                                                                                              1. first
                                                                                                                                2. } + {show[1] &&
                                                                                                                              2. middle
                                                                                                                                3. } + {show[2] &&
                                                                                                                              3. last
                                                                                                                                4. } +
                                                                                                                              + ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                                                                                                                              1. first
                                                                                                                              '); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. first
                                                                                                                              2. middle
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. first
                                                                                                                              2. middle
                                                                                                                              3. last
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              1. first
                                                                                                                              2. middle
                                                                                                                              ' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                                                                                                                              1. first
                                                                                                                              '); + }); }); diff --git a/test/browser/portals.test.js b/test/browser/portals.test.js index bbf37d5d43..ffbb1baeb8 100644 --- a/test/browser/portals.test.js +++ b/test/browser/portals.test.js @@ -22,13 +22,28 @@ describe('Portal', () => { let resetRemoveChild; let resetRemove; + let containers; + /** @type {(type: string) => Element} */ + function createContainer(type) { + const container = document.createElement(type); + containers.push(container); + return container; + } + beforeEach(() => { scratch = setupScratch(); rerender = setupRerender(); + + containers = []; }); afterEach(() => { teardown(scratch); + + for (let container of containers) { + container.remove(); + } + containers = null; }); before(() => { @@ -46,7 +61,7 @@ describe('Portal', () => { }); it('should render into a different root node', () => { - let root = document.createElement('div'); + let root = createContainer('div'); document.body.appendChild(root); function Foo(props) { @@ -60,10 +75,10 @@ describe('Portal', () => { }); it('should preserve mount order of non-portal siblings', () => { - let portals = document.createElement('portals'); + let portals = createContainer('portals'); scratch.appendChild(portals); - let main = document.createElement('main'); + let main = createContainer('main'); scratch.appendChild(main); function Foo(props) { @@ -89,20 +104,20 @@ describe('Portal', () => { const log = getLog().filter(t => !/#text/.test(t)); expect(log).to.deep.equal([ - '
                                                                                                                              .insertBefore(

                                                                                                                              A, )', + '.remove()', + '
                                                                                                                              .insertBefore(

                                                                                                                              A, )', '.insertBefore(

                                                                                                                              B, Null)', - '
                                                                                                                              A.insertBefore(

                                                                                                                              C, )', + '
                                                                                                                              A.insertBefore(

                                                                                                                              C, )', 'B.insertBefore(

                                                                                                                              D, Null)', - '
                                                                                                                              AC.insertBefore(
                                                                                                                              E, )', - '.remove()' + '
                                                                                                                              AC.insertBefore(
                                                                                                                              E, )' ]); }); it('should preserve hydration order of non-portal siblings', () => { - let portals = document.createElement('portals'); + let portals = createContainer('portals'); scratch.appendChild(portals); - let main = document.createElement('main'); + let main = createContainer('main'); scratch.appendChild(main); main.innerHTML = '

                                                                                                                              A

                                                                                                                              C

                                                                                                                              E
                                                                                                                              '; @@ -246,8 +261,8 @@ describe('Portal', () => { }); it('should not render for Portal nodes', () => { - let root = document.createElement('div'); - let dialog = document.createElement('div'); + let root = createContainer('div'); + let dialog = createContainer('div'); dialog.id = 'container'; scratch.appendChild(root); @@ -266,8 +281,8 @@ describe('Portal', () => { }); it('should unmount Portal', () => { - let root = document.createElement('div'); - let dialog = document.createElement('div'); + let root = createContainer('div'); + let dialog = createContainer('div'); dialog.id = 'container'; scratch.appendChild(root); @@ -612,8 +627,8 @@ describe('Portal', () => { }); it('should not unmount when parent renders', () => { - let root = document.createElement('div'); - let dialog = document.createElement('div'); + let root = createContainer('div'); + let dialog = createContainer('div'); dialog.id = 'container'; scratch.appendChild(root); @@ -793,6 +808,8 @@ describe('Portal', () => { }); it('should order complex effects well', () => { + const container = createContainer('div'); + const calls = []; const Parent = ({ children, isPortal }) => { useEffect(() => { @@ -819,7 +836,7 @@ describe('Portal', () => { calls.push('Portal'); }, []); - return createPortal({content}, document.body); + return createPortal({content}, container); }; const App = () => { @@ -853,7 +870,7 @@ describe('Portal', () => { }); it('should include containerInfo', () => { - let root = document.createElement('div'); + let root = createContainer('div'); document.body.appendChild(root); const A = () => A; @@ -874,4 +891,77 @@ describe('Portal', () => { root.parentNode.removeChild(root); }); + + it('should preserve portal behavior when moving nodes around a portal', () => { + const portalRoot = createContainer('div'); + document.body.appendChild(portalRoot); + + render( + [ +
                                                                                                                              A
                                                                                                                              , +
                                                                                                                              B
                                                                                                                              , +
                                                                                                                              C
                                                                                                                              , +
                                                                                                                              D
                                                                                                                              , + createPortal(
                                                                                                                              Portal
                                                                                                                              , portalRoot) + ], + scratch + ); + + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              A
                                                                                                                              B
                                                                                                                              C
                                                                                                                              D
                                                                                                                              ' + ); + expect(portalRoot.innerHTML).to.equal('
                                                                                                                              Portal
                                                                                                                              '); + + render( + [ +
                                                                                                                              A
                                                                                                                              , +
                                                                                                                              B
                                                                                                                              , + createPortal(
                                                                                                                              Portal
                                                                                                                              , portalRoot), +
                                                                                                                              C
                                                                                                                              , +
                                                                                                                              D
                                                                                                                              + ], + scratch + ); + + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              A
                                                                                                                              B
                                                                                                                              C
                                                                                                                              D
                                                                                                                              ' + ); + expect(portalRoot.innerHTML).to.equal('
                                                                                                                              Portal
                                                                                                                              '); + }); + + it('should insert a portal before new siblings when changing container to match siblings', () => { + const portalRoot = createContainer('div'); + document.body.appendChild(portalRoot); + + render( + [ +
                                                                                                                              A
                                                                                                                              , +
                                                                                                                              B
                                                                                                                              , + createPortal(
                                                                                                                              Portal
                                                                                                                              , portalRoot), +
                                                                                                                              D
                                                                                                                              + ], + scratch + ); + + expect(scratch.innerHTML).to.equal('
                                                                                                                              A
                                                                                                                              B
                                                                                                                              D
                                                                                                                              '); + expect(portalRoot.innerHTML).to.equal('
                                                                                                                              Portal
                                                                                                                              '); + + render( + [ +
                                                                                                                              A
                                                                                                                              , +
                                                                                                                              B
                                                                                                                              , + // Change container to match siblings container + createPortal(
                                                                                                                              Portal
                                                                                                                              , scratch), + // While adding a new sibling +
                                                                                                                              C
                                                                                                                              , +
                                                                                                                              D
                                                                                                                              + ], + scratch + ); + + expect(scratch.innerHTML).to.equal( + '
                                                                                                                              A
                                                                                                                              B
                                                                                                                              Portal
                                                                                                                              C
                                                                                                                              D
                                                                                                                              ' + ); + expect(portalRoot.innerHTML).to.equal(''); + }); }); diff --git a/test/browser/render.test.js b/test/browser/render.test.js index 9159410654..6988334e8d 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1,5 +1,5 @@ import { setupRerender } from 'preact/test-utils'; -import { createElement, render, Component, options } from 'preact'; +import { createElement, render, Component, options, Fragment } from 'preact'; import { setupScratch, teardown, @@ -1224,4 +1224,32 @@ describe('render()', () => { expect(items[0]).to.have.property('parentNode').that.should.exist; expect(items[1]).to.have.property('parentNode').that.should.exist; }); + + it('should replace in-place children with different types', () => { + const B1 = () =>
                                                                                                                              b1
                                                                                                                              ; + const B2 = () =>
                                                                                                                              b2
                                                                                                                              ; + + /** @type {(c: React.JSX.Element) => void} */ + let setChild; + function App() { + // Mimic some state that may cause a suspend + const [child, setChildInternal] = useState(); + setChild = setChildInternal; + + return ( + +
                                                                                                                              a
                                                                                                                              + {child} +
                                                                                                                              c
                                                                                                                              +                                                                                                                               + ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                                                                                                                              a
                                                                                                                              b1
                                                                                                                              c
                                                                                                                              '); + + setChild(); + rerender(); + expect(scratch.innerHTML).to.equal('
                                                                                                                              a
                                                                                                                              b2
                                                                                                                              c
                                                                                                                              '); + }); });

should have a
should have a