-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
base: v10.x
Are you sure you want to change the base?
Fix empty children #4822
Conversation
📊 Tachometer Benchmark ResultsSummaryA summary of the benchmark results will show here once they finish. ResultsThe full results of your benchmarks will show here once they finish. |
Size Change: +16 B (+0.02%) Total Size: 78.7 kB
ℹ️ View Unchanged
|
e447235
to
d55f62d
Compare
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 || []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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)
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 anundefined
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