Skip to content

Fix empty children #4822

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

Open
wants to merge 1 commit into
base: v10.x
Choose a base branch
from
Open

Fix empty children #4822

wants to merge 1 commit into from

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Jun 28, 2025

In preactjs/prefresh#568 the point was rightfully made that we lose our assurance that _children is always an array. When a component throws on first-render we risk setting an undefined value and then re-using it for recovered renders.

I haven't found a way to actually make this occur in our test suite yet though

@JoviDeCroock JoviDeCroock changed the base branch from main to v10.x June 28, 2025 16:53
Copy link

github-actions bot commented Jun 28, 2025

📊 Tachometer Benchmark Results

Summary

A summary of the benchmark results will show here once they finish.

Results

The full results of your benchmarks will show here once they finish.

tachometer-reporter-action v2 for CI

Copy link

github-actions bot commented Jun 28, 2025

Size Change: +16 B (+0.02%)

Total Size: 78.7 kB

Filename Size Change
dist/preact.js 4.74 kB +3 B (+0.06%)
dist/preact.min.js 4.76 kB +2 B (+0.04%)
dist/preact.min.module.js 4.76 kB +3 B (+0.06%)
dist/preact.min.umd.js 4.78 kB +2 B (+0.04%)
dist/preact.mjs 4.75 kB +2 B (+0.04%)
dist/preact.module.js 4.75 kB +2 B (+0.04%)
dist/preact.umd.js 4.79 kB +2 B (+0.04%)
ℹ️ View Unchanged
Filename Size
compat/dist/compat.js 4.15 kB
compat/dist/compat.mjs 4.08 kB
compat/dist/compat.module.js 4.08 kB
compat/dist/compat.umd.js 4.21 kB
debug/dist/debug.js 3.82 kB
debug/dist/debug.mjs 3.82 kB
debug/dist/debug.module.js 3.82 kB
debug/dist/debug.umd.js 3.9 kB
devtools/dist/devtools.js 260 B
devtools/dist/devtools.mjs 274 B
devtools/dist/devtools.module.js 274 B
devtools/dist/devtools.umd.js 346 B
hooks/dist/hooks.js 1.54 kB
hooks/dist/hooks.mjs 1.57 kB
hooks/dist/hooks.module.js 1.57 kB
hooks/dist/hooks.umd.js 1.61 kB
jsx-runtime/dist/jsxRuntime.js 1.01 kB
jsx-runtime/dist/jsxRuntime.mjs 985 B
jsx-runtime/dist/jsxRuntime.module.js 985 B
jsx-runtime/dist/jsxRuntime.umd.js 1.08 kB
test-utils/dist/testUtils.js 473 B
test-utils/dist/testUtils.mjs 477 B
test-utils/dist/testUtils.module.js 477 B
test-utils/dist/testUtils.umd.js 555 B

compressed-size-action

@JoviDeCroock
Copy link
Member Author

JoviDeCroock commented Jun 28, 2025

I am well and thoroughly confused by our tests erroring out right now 😂

EDIT: ah a rebase was all it took

@@ -312,7 +312,7 @@ export function diff(
}
} else {
newVNode._dom = oldVNode._dom;
newVNode._children = oldVNode._children;
newVNode._children = oldVNode._children || [];
Copy link
Member

@rschristian rschristian Jun 30, 2025

Choose a reason for hiding this comment

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

Suggested change
newVNode._children = oldVNode._children || [];
newVNode._children = oldVNode._children || EMPTY_ARR;

Will need to add it as an import tho, wish GH allowed users to suggest outside the diff.

Weird that the CI is failing. Edit: Ah, looks like we'll need to manually upload something to get the 10.x line working again.

Copy link
Member

Choose a reason for hiding this comment

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

Would add a small comment here too on why we set it to a default value (e.g. point to prefresh issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants