Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix debug tests #3980

Open
wants to merge 8 commits into
base: v11-linked-list-prev-index-nextDom-2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compat/src/forwardRef.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
17 changes: 10 additions & 7 deletions debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function initDebug() {
);
}

let { type, _parent: parent } = internal;
let { type } = internal;

if (type === undefined) {
throw new Error(
Expand All @@ -161,8 +161,6 @@ export function initDebug() {
);
}

validateTableMarkup(internal);

hooksAllowed = true;

let isCompatNode = '$$typeof' in vnode;
Expand All @@ -181,6 +179,8 @@ export function initDebug() {
}

if (typeof internal.type == 'string') {
validateTableMarkup(internal);

for (const key in vnode.props) {
if (
key[0] === 'o' &&
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions debug/src/validateMarkup.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ 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') &&
Expand Down
14 changes: 13 additions & 1 deletion debug/test/browser/debug.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { createElement, render, createRef, Component, Fragment } from 'preact';
import {
createElement,
render,
createRef,
Component,
Fragment,
hydrate
} from 'preact';
import {
setupScratch,
teardown,
Expand Down Expand Up @@ -51,6 +58,11 @@ describe('debug', () => {
expect(fn).to.throw(/render/);
});

it('should print an error on hydrating on undefined parent', () => {
let fn = () => hydrate(<div />, undefined);
expect(fn).to.throw(/render/);
});

it('should print an error on rendering on invalid parent', () => {
let fn = () => render(<div />, 6);
expect(fn).to.throw(/valid HTML node/);
Expand Down
2 changes: 1 addition & 1 deletion src/create-root.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function createRoot(parentDom) {

rootInternal._context = {};

mount(rootInternal, parentDom, firstChild);
mount(rootInternal, vnode, parentDom, firstChild);
}

// Flush all queued effects
Expand Down
11 changes: 8 additions & 3 deletions src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions src/diff/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -237,7 +238,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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do this typeof once and reuse it's result here and line 249?

Same for patch

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked and storing the result in a variable is 10b larger. I think for now it's fine to leave it as is.

)
continue;
else if (Array.isArray(vnode)) vnode = createElement(Fragment, null, vnode);
else if (typeof vnode !== 'object') vnode = String(vnode);

Expand All @@ -248,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;

Expand Down
3 changes: 1 addition & 2 deletions src/diff/unmount.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}
3 changes: 3 additions & 0 deletions test/browser/toChildArray.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ describe('toChildArray', () => {
render(<Foo>{child}</Foo>, scratch);
expect(children).to.be.an('array');
expect(scratch.innerHTML).to.equal('<div></div>');

render(<Foo>{child}</Foo>, scratch);
expect(scratch.innerHTML).to.equal('<div></div>');
});

it('returns an array containing a VNode with a text child', () => {
Expand Down