Skip to content

Commit 952846b

Browse files
kfuledead-claudia
authored andcommitted
normalizeChildren: preallocate array length and perform key-consistency checks after normalization
This change preallocates the array to the input length and collapses multiple loops into a single pass. Assigning immediately after preallocation improves performance on V8 (generally neutral elsewhere). Key checks are now performed on normalized vnodes, making the consistency validation more accurate and clarifying the correspondence between error messages and code. Perf-sensitive comments have been clarified to reflect the original intent of commit 6c562d2. Behavior is unchanged, except that the timing/order of related errors may differ slightly. All existing tests pass. Additionally, bundle size is slightly reduced.
1 parent 6dc1b75 commit 952846b

File tree

1 file changed

+17
-18
lines changed

1 file changed

+17
-18
lines changed

render/vnode.js

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,23 @@ Vnode.normalize = function(node) {
1010
return Vnode("#", undefined, undefined, String(node), undefined, undefined)
1111
}
1212
Vnode.normalizeChildren = function(input) {
13-
var children = []
14-
if (input.length) {
15-
var isKeyed = input[0] != null && input[0].key != null
16-
// Note: this is a *very* perf-sensitive check.
17-
// Fun fact: merging the loop like this is somehow faster than splitting
18-
// it, noticeably so.
19-
for (var i = 1; i < input.length; i++) {
20-
if ((input[i] != null && input[i].key != null) !== isKeyed) {
21-
throw new TypeError(
22-
isKeyed && (input[i] == null || typeof input[i] === "boolean")
23-
? "In fragments, vnodes must either all have keys or none have keys. You may wish to consider using an explicit keyed empty fragment, m.fragment({key: ...}), instead of a hole."
24-
: "In fragments, vnodes must either all have keys or none have keys."
25-
)
26-
}
27-
}
28-
for (var i = 0; i < input.length; i++) {
29-
children[i] = Vnode.normalize(input[i])
30-
}
13+
// Preallocate the array length (initially holey) and fill every index immediately in order.
14+
// Benchmarking shows better performance on V8.
15+
var children = new Array(input.length)
16+
// Count the number of keyed normalized vnodes for consistency check.
17+
// Note: this is a perf-sensitive check.
18+
// Fun fact: merging the loop like this is somehow faster than splitting
19+
// the check within updateNodes(), noticeably so.
20+
var numKeyed = 0
21+
for (var i = 0; i < input.length; i++) {
22+
children[i] = Vnode.normalize(input[i])
23+
if (children[i] !== null && children[i].key != null) numKeyed++
24+
}
25+
if (numKeyed !== 0 && numKeyed !== input.length) {
26+
throw new TypeError(children.includes(null)
27+
? "In fragments, vnodes must either all have keys or none have keys. You may wish to consider using an explicit keyed empty fragment, m.fragment({key: ...}), instead of a hole."
28+
: "In fragments, vnodes must either all have keys or none have keys."
29+
)
3130
}
3231
return children
3332
}

0 commit comments

Comments
 (0)