Skip to content

Review feedback v11 #4655

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.

Sign up for GitHub

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

Merged
JoviDeCroock merged 5 commits into from
Feb 12, 2025
Merged

Review feedback v11 #4655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c9d8fb
Revert assign and avoid repeating indexed access
JoviDeCroock Feb 12, 2025
063358d
Check parentNode instead
JoviDeCroock Feb 12, 2025
9c160d7
Use flag
JoviDeCroock Feb 12, 2025
91d06e5
Remove type
JoviDeCroock Feb 12, 2025
897265a
Remove todo file
JoviDeCroock Feb 12, 2025
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
5 changes: 0 additions & 5 deletions TODO.md
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions src/diff/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export function diff(
c.state = c._nextState;

if (c.getChildContext != null) {
globalContext = assign({}, globalContext, c.getChildContext());
globalContext = assign(assign({}, globalContext), c.getChildContext());
}

if (isClassComponent && !isNew && c.getSnapshotBeforeUpdate != null) {
Expand Down Expand Up @@ -300,9 +300,8 @@ export function diff(
newVNode._dom = oldDom;
} else {
for (let i = excessDomChildren.length; i--; ) {
if (excessDomChildren[i]) {
excessDomChildren[i].remove();
}
const child = excessDomChildren[i];
if (child) child.remove();
}
}
} else {
Expand Down Expand Up @@ -544,7 +543,8 @@ function diffElementNodes(
// Remove children that are not part of any vnode.
if (excessDomChildren != null) {
for (i = excessDomChildren.length; i--; ) {
if (excessDomChildren[i]) excessDomChildren[i].remove();
const child = excessDomChildren[i];
if (child) child.remove();
}
}
}
Expand Down Expand Up @@ -643,7 +643,7 @@ export function unmount(vnode, parentVNode, skipRemove) {
}
}

if (!skipRemove && vnode._dom != null && typeof vnode.type != 'function') {
if (!skipRemove && vnode._dom != null && vnode._dom.parentNode) {
vnode._dom.remove();
}

Expand Down
10 changes: 0 additions & 10 deletions src/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,16 +299,6 @@ interface ContainerNode {
}

export function render(vnode: ComponentChild, parent: ContainerNode): void;
/**
* @deprecated Will be removed in v11.
*
* Replacement Preact 10+ implementation can be found here: https://gist.github.com/developit/f4c67a2ede71dc2fab7f357f39cff28c
Copy link
Member

@rschristian rschristian Feb 12, 2025

Choose a reason for hiding this comment

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

Good spot, had completely forgotten about this

*/
export function render(
vnode: ComponentChild,
parent: ContainerNode,
replaceNode?: Element | Text
): void;
export function hydrate(vnode: ComponentChild, parent: ContainerNode): void;
export function cloneElement(
vnode: VNode<any>,
Expand Down
16 changes: 7 additions & 9 deletions src/render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EMPTY_OBJ } from './constants';
import { EMPTY_OBJ, MODE_HYDRATE } from './constants';
import { commitRoot, diff } from './diff/index';
import { createElement, Fragment } from './create-element';
import options from './options';
Expand All @@ -8,21 +8,17 @@ import { slice } from './util';
* Render a Preact virtual node into a DOM element
* @param {import('./internal').ComponentChild} vnode The virtual node to render
* @param {import('./internal').PreactElement} parentDom The DOM element to render into
* @param {import('./internal').PreactElement | object} [replaceNode] Optional: Attempt to re-use an
* existing DOM tree rooted at `replaceNode`
*/
export function render(vnode, parentDom, replaceNode) {
export function render(vnode, parentDom) {
// https://github.com/preactjs/preact/issues/3794
if (parentDom == document) {
parentDom = document.documentElement;
}

if (options._root) options._root(vnode, parentDom);

// We abuse the `replaceNode` parameter in `hydrate()` to signal if we are in
// hydration mode or not by passing the `hydrate` function instead of a DOM
// element..
let isHydrating = replaceNode === hydrate;
// @ts-expect-error
let isHydrating = !!(vnode && vnode._flags & MODE_HYDRATE);

// To be able to support calling `render()` multiple times on the same
// DOM node, we need to obtain a reference to the previous tree. We do
Expand Down Expand Up @@ -65,5 +61,7 @@ export function render(vnode, parentDom, replaceNode) {
* @param {import('./internal').PreactElement} parentDom The DOM element to update
*/
export function hydrate(vnode, parentDom) {
render(vnode, parentDom, hydrate);
// @ts-expect-error
vnode._flags |= MODE_HYDRATE;
render(vnode, parentDom);
}
14 changes: 13 additions & 1 deletion src/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
import { EMPTY_ARR } from './constants';

export const isArray = Array.isArray;
export const assign = Object.assign;
export const slice = EMPTY_ARR.slice;

/**
* Assign properties from `props` to `obj`
* @template O, P The obj and props types
* @param {O} obj The object to copy properties to
* @param {P} props The object to copy properties from
* @returns {O & P}
*/
export function assign(obj, props) {
// @ts-expect-error We change the type of `obj` to be `O & P`
for (let i in props) obj[i] = props[i];
return /** @type {O & P} */ (obj);
}