- Notifications
You must be signed in to change notification settings - Fork 178
Description
To reproduce in JS console with built.mjs pasted in:
let Tab = function() { return 'Hi'; } let index = 1; let href='/'; let onMouseDown=''; let isAction = ''; build`<${Tab} index=${index} key=${href} href=${href} onMouseDown=${onMouseDown} isActive=${isActive}> ${title} <span>Hi</span> <//>`;Expected output:
APPEND_CHILD, TAG_SET, PROP_SET, PROP_SET, CHILD_RECURSIVE
Real output:
[0] => Array ( [0] => 0 [1] => 2 [2] => 0 [3] => Array ( [0] => Array ( [0] => 0 ) [1] => 3 [2] => 1 [3] => [4] => 5 [5] => 2 [6] => 0 [7] => index [8] => 5 [9] => 3 [10] => 0 [11] => key [12] => 5 [13] => 4 [14] => 0 [15] => href [16] => 5 [17] => 5 [18] => 0 [19] => onMouseDown [20] => 5 [21] => 6 [22] => 0 [23] => isActive [24] => 0 [25] => 7 [26] => [27] => 0 [28] => 0 [29] => [30] => 2 [31] => 0 [32] => Array ( [0] => Array ( [0] => Array ( [0] => 0 ) [1] => 3 [2] => 1 [3] => [4] => 5 [5] => 2 [6] => 0 [7] => index [8] => 5 [9] => 3 [10] => 0 [11] => key [12] => 5 [13] => 4 [14] => 0 [15] => href [16] => 5 [17] => 5 [18] => 0 [19] => onMouseDown [20] => 5 [21] => 6 [22] => 0 [23] => isActive [24] => 0 [25] => 7 [26] => [27] => 0 [28] => 0 [29] => ) [1] => 3 [2] => 0 [3] => span [4] => 0 [5] => 0 [6] => Hi ) ) ) This is from my PHP port of preact (just an experiment), but it also happens with preact master in Javascript.
I think when recursing into the children, the data should be stored in a stack variable and not in current[0].
While for traversing that's not a problem, it increases the size of current dramatically and this - unless cleaned up - is what is cached, which is also not ideal as the sparse data should be as small as possible obviously.
To fix:
The fix is trivial also:
Line 262 in ed1c620
| mode = current; |
be replaced with:
mode = current; current_0 = current[0]; mode[0] = []; if (MINI) { (current = current_0).push(h.apply(null, mode.slice(1))); } else { (current = current_0).push(CHILD_RECURSE, 0, mode); } should be all that is needed. Also could fix the double usage of the name mode at the same time. Likely tmp or current_child or such might be better.