From 77cff8bb8b384c6bf3f6a5ce7dea50f69d6eb078 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 18:56:09 +0200 Subject: [PATCH 1/8] Fix function child not skipped --- src/diff/children.js | 7 ++++++- src/diff/mount.js | 8 +++++++- test/browser/toChildArray.test.js | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/diff/children.js b/src/diff/children.js index 4e48e60266..7a94b6e5ef 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -107,7 +107,12 @@ function findMatches(internal, children, parentInternal) { let vnode = children[index]; // holes get accounted for in the index property: - if (vnode == null || vnode === true || vnode === false) { + if ( + vnode == null || + vnode === true || + vnode === false || + typeof vnode === 'function' + ) { 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 diff --git a/src/diff/mount.js b/src/diff/mount.js index e17d20deeb..6da8a18a4b 100644 --- a/src/diff/mount.js +++ b/src/diff/mount.js @@ -237,7 +237,13 @@ export function mountChildren(parentInternal, children, parentDom, startDom) { vnode = children[i]; // account for holes by incrementing the index: - if (vnode == null || vnode === true || vnode === false) continue; + if ( + vnode == null || + vnode === true || + vnode === false || + typeof vnode === 'function' + ) + continue; else if (Array.isArray(vnode)) vnode = createElement(Fragment, null, vnode); else if (typeof vnode !== 'object') vnode = String(vnode); diff --git a/test/browser/toChildArray.test.js b/test/browser/toChildArray.test.js index d1cb950992..d806c04020 100644 --- a/test/browser/toChildArray.test.js +++ b/test/browser/toChildArray.test.js @@ -67,6 +67,9 @@ describe('toChildArray', () => { render({child}, scratch); expect(children).to.be.an('array'); expect(scratch.innerHTML).to.equal('
'); + + render({child}, scratch); + expect(scratch.innerHTML).to.equal('
'); }); it('returns an array containing a VNode with a text child', () => { From e9c43bd365c7db1deb5a458a4428efb4c4e44c9a Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 19:23:53 +0200 Subject: [PATCH 2/8] Fix vnode not passed to options._diff on mount --- src/create-root.js | 2 +- src/diff/children.js | 4 ++-- src/diff/mount.js | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/create-root.js b/src/create-root.js index c40327efbe..a3b38e605f 100644 --- a/src/create-root.js +++ b/src/create-root.js @@ -49,7 +49,7 @@ export function createRoot(parentDom) { rootInternal._context = {}; - mount(rootInternal, parentDom, firstChild); + mount(rootInternal, vnode, parentDom, firstChild); } // Flush all queued effects diff --git a/src/diff/children.js b/src/diff/children.js index 7a94b6e5ef..4b5df85600 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -49,7 +49,7 @@ export function patchChildren(internal, children, parentDom) { if (internal._index === -1) { let nextDomSibling = getDomSibling(internal); - mount(internal, parentDom, nextDomSibling); + mount(internal, vnode, 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 @@ -61,7 +61,7 @@ export function patchChildren(internal, children, parentDom) { (internal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) === (MODE_HYDRATE | MODE_SUSPENDED) ) { - mount(internal, parentDom, internal.data); + mount(internal, vnode, parentDom, internal.data); } else { patch( internal, diff --git a/src/diff/mount.js b/src/diff/mount.js index 6da8a18a4b..c0cef13e92 100644 --- a/src/diff/mount.js +++ b/src/diff/mount.js @@ -22,12 +22,13 @@ 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} vnode The vnode that was used to create the internal * @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, parentDom, startDom) { - if (options._diff) options._diff(internal, null); +export function mount(internal, vnode, parentDom, startDom) { + if (options._diff) options._diff(internal, vnode); /** @type {import('../internal').PreactNode} */ let nextDomSibling, prevStartDom; @@ -254,7 +255,7 @@ export function mountChildren(parentInternal, children, parentDom, startDom) { else parentInternal._child = internal; // Morph the old element into the new one, but don't append it to the dom yet - mountedNextChild = mount(internal, parentDom, startDom); + mountedNextChild = mount(internal, vnode, parentDom, startDom); newDom = internal.data; From 8565fb26cbf5cf3da2062ce9f81b8c70c4c68d6b Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 19:24:10 +0200 Subject: [PATCH 3/8] Fix table markup validation --- debug/src/debug.js | 4 ++-- debug/src/validateMarkup.js | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 0aa0573ccd..e669d1dde4 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -161,8 +161,6 @@ export function initDebug() { ); } - validateTableMarkup(internal); - hooksAllowed = true; let isCompatNode = '$$typeof' in vnode; @@ -181,6 +179,8 @@ export function initDebug() { } if (typeof internal.type == 'string') { + validateTableMarkup(internal); + for (const key in vnode.props) { if ( key[0] === 'o' && diff --git a/debug/src/validateMarkup.js b/debug/src/validateMarkup.js index 7040ab5ec2..e475b17caa 100644 --- a/debug/src/validateMarkup.js +++ b/debug/src/validateMarkup.js @@ -23,7 +23,7 @@ export function validateTableMarkup(internal) { if ( (type === 'thead' || type === 'tfoot' || type === 'tbody') && - parentDomInternal.type !== 'table' + (parentDomInternal === null || parentDomInternal.type !== 'table') ) { console.error( 'Improper nesting of table. Your should have a parent.' + @@ -32,23 +32,30 @@ export function validateTableMarkup(internal) { ); } else if ( type === 'tr' && - parentDomInternal.type !== 'thead' && - parentDomInternal.type !== 'tfoot' && - parentDomInternal.type !== 'tbody' && - parentDomInternal.type !== 'table' + (parentDomInternal === null || + (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') { + } else if ( + type === 'td' && + (parentDomInternal === null || parentDomInternal.type !== 'tr') + ) { console.error( 'Improper nesting of table. Your parent.' + serializeVNode(internal) + `\n\n${getOwnerStack(internal)}` ); - } else if (type === 'th' && parentDomInternal.type !== 'tr') { + } else if ( + type === 'th' && + (parentDomInternal === null || parentDomInternal.type !== 'tr') + ) { console.error( 'Improper nesting of table. Your .' + serializeVNode(internal) + From 5a3e91d3646a97eabe55abaf59354b23cecc9a60 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 19:35:45 +0200 Subject: [PATCH 4/8] Fix duplicate keys detection --- debug/src/debug.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index e669d1dde4..968bf9c319 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -321,11 +321,14 @@ export function initDebug() { if (oldDiffed) oldDiffed(internal); - if (internal._children != null) { + if (internal._child != null) { + let child = internal._child; const keys = []; - for (let i = 0; i < internal._children.length; i++) { - const child = internal._children[i]; - if (!child || child.key == null) continue; + if (child.key != null) { + keys.push(child.key); + } + while ((child = child._next) != null) { + if (child.key == null) continue; const key = child.key; if (keys.indexOf(key) !== -1) { From e1c5c78d338b9cf5dd034e9b25482f92c6df709d Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 19:38:32 +0200 Subject: [PATCH 5/8] Fix undefined container detection --- debug/test/browser/debug.test.js | 14 +++++++++++++- src/render.js | 4 ++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/debug/test/browser/debug.test.js b/debug/test/browser/debug.test.js index fbfce2cd93..7c30788a46 100644 --- a/debug/test/browser/debug.test.js +++ b/debug/test/browser/debug.test.js @@ -1,4 +1,11 @@ -import { createElement, render, createRef, Component, Fragment } from 'preact'; +import { + createElement, + render, + createRef, + Component, + Fragment, + hydrate +} from 'preact'; import { setupScratch, teardown, @@ -51,6 +58,11 @@ describe('debug', () => { expect(fn).to.throw(/render/); }); + it('should print an error on hydrating on undefined parent', () => { + let fn = () => hydrate(
, undefined); + expect(fn).to.throw(/render/); + }); + it('should print an error on rendering on invalid parent', () => { let fn = () => render(
, 6); expect(fn).to.throw(/valid HTML node/); diff --git a/src/render.js b/src/render.js index 21323efcdd..80c2e423c9 100644 --- a/src/render.js +++ b/src/render.js @@ -11,7 +11,7 @@ export function render(vnode, parentDom) { if (!root) { root = createRoot(parentDom); } - parentDom._root = root; + if (parentDom) parentDom._root = root; root.render(vnode); } @@ -26,6 +26,6 @@ export function hydrate(vnode, parentDom) { if (!root) { root = createRoot(parentDom); } - parentDom._root = root; + if (parentDom) parentDom._root = root; root.hydrate(vnode); } From f350e1faca9580f9425d79a9f672a6ad09445e0d Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 19:45:44 +0200 Subject: [PATCH 6/8] Fix forwardRef ref passing --- compat/src/forwardRef.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compat/src/forwardRef.js b/compat/src/forwardRef.js index 75be7f26d6..1be0a9fb44 100644 --- a/compat/src/forwardRef.js +++ b/compat/src/forwardRef.js @@ -10,10 +10,10 @@ options._diff = (internal, vnode) => { if (vnode) { vnode.props.ref = vnode.ref; vnode.ref = null; - } else { - internal.props.ref = internal.ref; - internal.ref = null; } + + internal.props.ref = internal.ref; + internal.ref = null; } if (oldDiffHook) oldDiffHook(internal, vnode); }; From a5371344a5116515e79f34422e914284c51b8070 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sat, 22 Apr 2023 20:36:52 +0200 Subject: [PATCH 7/8] Fix linting errors --- debug/src/debug.js | 2 +- src/diff/mount.js | 2 +- src/diff/unmount.js | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/debug/src/debug.js b/debug/src/debug.js index 968bf9c319..b514f1992d 100644 --- a/debug/src/debug.js +++ b/debug/src/debug.js @@ -134,7 +134,7 @@ export function initDebug() { ); } - let { type, _parent: parent } = internal; + let { type } = internal; if (type === undefined) { throw new Error( diff --git a/src/diff/mount.js b/src/diff/mount.js index c0cef13e92..44c4910583 100644 --- a/src/diff/mount.js +++ b/src/diff/mount.js @@ -13,7 +13,7 @@ import { MODE_SVG, DIRTY_BIT } from '../constants'; -import { normalizeToVNode, createElement, Fragment } from '../create-element'; +import { createElement, Fragment } from '../create-element'; import { setProperty } from './props'; import { createInternal, getParentContext } from '../tree'; import options from '../options'; diff --git a/src/diff/unmount.js b/src/diff/unmount.js index 84793a09f3..80b10b9958 100644 --- a/src/diff/unmount.js +++ b/src/diff/unmount.js @@ -13,8 +13,7 @@ import { ENABLE_CLASSES } from '../component'; * current element is already detached from the DOM. */ export function unmount(internal, topUnmountedInternal, skipRemove) { - let r, - i = 0; + let r; if (options.unmount) options.unmount(internal); internal.flags |= MODE_UNMOUNTING; From 4d2049eecdef9ebaf2871b204aa64ccfa22e6992 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Sun, 30 Apr 2023 16:16:05 +0200 Subject: [PATCH 8/8] Use early return --- debug/src/validateMarkup.js | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/debug/src/validateMarkup.js b/debug/src/validateMarkup.js index e475b17caa..a7b3746075 100644 --- a/debug/src/validateMarkup.js +++ b/debug/src/validateMarkup.js @@ -20,10 +20,11 @@ function getClosestDomNodeParent(parent) { export function validateTableMarkup(internal) { const { type, _parent: parent } = internal; const parentDomInternal = getClosestDomNodeParent(parent); + if (parentDomInternal === null) return; if ( (type === 'thead' || type === 'tfoot' || type === 'tbody') && - (parentDomInternal === null || parentDomInternal.type !== 'table') + parentDomInternal.type !== 'table' ) { console.error( 'Improper nesting of table. Your
should have a
should have a
should have a
parent.' + @@ -32,30 +33,23 @@ export function validateTableMarkup(internal) { ); } else if ( type === 'tr' && - (parentDomInternal === null || - (parentDomInternal.type !== 'thead' && - parentDomInternal.type !== 'tfoot' && - parentDomInternal.type !== 'tbody' && - parentDomInternal.type !== 'table')) + 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 === null || parentDomInternal.type !== 'tr') - ) { + } 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 === null || parentDomInternal.type !== 'tr') - ) { + } else if (type === 'th' && parentDomInternal.type !== 'tr') { console.error( 'Improper nesting of table. Your .' + serializeVNode(internal) +
should have a
should have a