[BUG] built is duplicating "current" in the array for every child element

[LionsAd]

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:

https://github.com/developit/htm/blob/ed1c62066e4ef33c01b9269efd4e8b1a73f89887/src/build.mjs#L262

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.

[developit]

@LionsAd finally getting back to this - any chance you're seeing a circular reference in JS but the PHP port happens to clone the Array making it grow? 'm still digging into around, but on master it appears to be a shared array reference that gets reset.

[LionsAd]

Oh - that might very well be. I saw it growing in the JS array as well, but did not check memory usage when memo‘ed.

I will double check that memory usage actually grows.

Thanks for checking that.

[developit]

No problem! Sorry I didn't have a definitive answer - I still need to refamiliarize myself with the current codebase 😅

[jviide]

This is intentional. We use current to store the parsing state for the currently processed element, but current also doubles as a parsing stack of sorts: current[0] always points to the parent element's parsing state. When we do current = current[0] we essentially "pop" the stack and go back to processing the parent element's state.

Therefore each current[0] contains just a pointer to an already existing array - the parent's parsing state - and shouldn't take extra memory. In fact, doing mode = current; current_0 = current[0]; mode[0] = []; may take more memory, as it allocates a new array.

Tangentially related: After the whole tree has been parsed the "stack" functionality isn't used anymore, so in evaluate we use current[0] for storing bitflags for identifying static subtrees.