Event handler being reattached/recreated every new component render

[shawncrawley]

I cannot figure this one out, so of course I'm finally turning here for help. I am running into an issue where the onClick event handler that I'm defining on the Map component is apparently getting recreated/reattached every time any state variable is updated. If I load this app and then click the map, "CLICKED!" gets printed to the terminal. If I click again, "CLICKED!" gets printed two more times. If I click again, "CLICKED!" gets printed four more times... The number of clicks detected is growing exponentially!

Below are the files to reproduce this minimal example. Any idea what's going on here? Is this a shortcoming of the way the Map component was designed?

// example.js
import React from "https://esm.sh/[email protected]"
import ReactDOM from "https://esm.sh/[email protected]/client"
import Map from "https://esm.sh/@planet/[email protected]/[email protected],[email protected],[email protected]";
export {Map};
import View from "https://esm.sh/@planet/[email protected]/[email protected],[email protected],[email protected]";
export {View};

loadCSS("https://esm.sh/[email protected]/ol.css");

function loadCSS(href) {  
    var head = document.getElementsByTagName('head')[0];

    if (document.querySelectorAll(`link[href="${href}"]`).length === 0) {
        // Creating link element 
        var style = document.createElement('link');
        style.id = href;
        style.href = href;
        style.type = 'text/css';
        style.rel = 'stylesheet';
        head.append(style);
    }
}

export function bind(node, config) {
    const root = ReactDOM.createRoot(node);
    return {
        create: (component, props, children) =>
            React.createElement(component, wrapEventHandlers(props), ...children),
        render: (element) => root.render(element),
        unmount: () => root.unmount()
    };
}

function wrapEventHandlers(props) {
    const newProps = Object.assign({}, props);
    for (const [key, value] of Object.entries(props)) {
        if (typeof value === "function") {
            newProps[key] = makeJsonSafeEventHandler(value);
        }
    }
    return newProps;
}

function stringifyToDepth(val, depth, replacer, space) {
    depth = isNaN(+depth) ? 1 : depth;
    function _build(key, val, depth, o, a) { // (JSON.stringify() has it's own rules, which we respect here by using it for property iteration)
        return !val || typeof val != 'object' ? val : (a=Array.isArray(val), JSON.stringify(val, function(k,v){ if (a || depth > 0) { if (replacer) v=replacer(k,v); if (!k) return (a=Array.isArray(v),val=v); !o && (o=a?[]:{}); o[k] = _build(k, v, a?depth:depth-1); } }), o||(a?[]:{}));
    }
    return JSON.stringify(_build('', val, depth), null, space);
}

function makeJsonSafeEventHandler(oldHandler) {
    // Since we can't really know what the event handlers get passed we have to check if
    // they are JSON serializable or not. We can allow normal synthetic events to pass
    // through since the original handler already knows how to serialize those for us.
    return function safeEventHandler() {

        var filteredArguments = [];
        Array.from(arguments).forEach(function (arg) {
            if (typeof arg === "object" && arg.nativeEvent) {
                // this is probably a standard React synthetic event
                filteredArguments.push(arg);
            } else {
                filteredArguments.push(JSON.parse(stringifyToDepth(arg, 3, (key, value) => {
                    if (key === '') return value;
                    try {
                        JSON.stringify(value);
                        return value;
                    } catch (err) {
                        return (typeof value === 'object') ? value : undefined;
                    }
                })))
            }
        });
        oldHandler(...Array.from(filteredArguments));
    };
}

# app.py
from reactpy import component, html, run, web

module = web.module_from_file(
    "example", "example.js",
)
Map, View = web.export(module, ["Map", "View"])

@component
def App():
    test, set_test = reactpy.hooks.use_state(True)
    return reactpy.html.div({"style": {"width": "100vw", "height": "100vh"}},
        Map({"onClick": lambda _: print("CLICKED!") is None and set_test(not test)},
            View({"center": [-100, 40], "zoom": 3.5})
        )
    )

run(App)

[Archmonger]

What version of ReactPy are you running? I believe this issue was resolved in the latest alpha releases.

[Archmonger]

Yes, the develop branch.

pip install git+https://github.com/reactive-python/reactpy@develop

[shawncrawley]

I just tried with the latest develop and it's definitely not fixed. In fact, I wrote two tests to verify and temporarily added them to test_module.py. Here they are:

async def test_compounding_event_html(display: DisplayFixture):
    @reactpy.component
    def App():
        click_count, set_click_count = reactpy.hooks.use_state(0)
        return reactpy.html.div({"id": "the-parent"},
            reactpy.html.button({
                "id": "the-button", 
                "onClick": lambda _: set_click_count(click_count + 1)
            }, "Click Me"),
            *[reactpy.html.div("Clicked") for _ in range(click_count)]
        )

    await display.show(lambda: App())

    button = await display.page.wait_for_selector("#the-button", state="attached")
    await button.click()
    await button.click()
    await button.click()
    parent = await display.page.wait_for_selector("#the-parent", state="attached")
    generated_divs = await parent.query_selector_all("div")

    assert len(generated_divs) == 3

async def test_compounding_event_web_module(display: DisplayFixture):
    module = reactpy.web.module_from_file(
        "compounding-event",
        JS_FIXTURES_DIR / "compounding-event.js",  # content equivalent to example.js shown above
    )

    Map, View = reactpy.web.export(module, ["Map", "View"])

    @reactpy.component
    def App():
        click_count, set_click_count = reactpy.hooks.use_state(0)
        return reactpy.html.div(
            reactpy.html.div({"style": {"width": "50vw", "height": "50vh"}},
                Map({"id": "the-map", "onClick": lambda _: set_click_count(click_count + 1)},
                    View({"center": [-100, 40], "zoom": 3.5})
                ),
                reactpy.html.div({"id": "the-parent"}, 
                    *[reactpy.html.div("Clicked") for _ in range(click_count)]
                )
            )
        )

    await display.show(lambda: App())

    map = await display.page.wait_for_selector("#the-map", state="attached")
    await map.click()
    await map.click()
    await map.click()
    parent = await display.page.wait_for_selector("#the-parent", state="attached")
    generated_divs = await parent.query_selector_all("div")

    assert len(generated_divs) == 3

The first test using a basic reactpy.html.button element as the event source succeeds, while the second test using the Map web module component fails. The error of the failure is 7 != 3 meaning that 3 clicks generates 7 events rather than the expected 3.

I'm going to keep digging into this, but any ideas are of course welcome.

[shawncrawley]

Interestingly, I just wrote another test nearly identical to the basic reactpy.html.button test, but swapped it for a different web module component: a Button from react-bootstrap. And that one worked! So there must be something unique about how the Map component from https://esm.sh/@planet/[email protected]/Map works... I don't understand enough about the core ReactPy javascript and how it determines if a component/element changed or not to re-render it. Should it be destroying the old map and creating a new one, each time, but that's not happening for some reason? I'll check if there are multiple maps being rendered on top of each other or something with each new click.

[Archmonger]

It shouldn't be recreating the element as long as its key identity stays the same and/or the element remains in the same position.

[shawncrawley]

I still don't fully understand the ReactPy lifecycle - nor the ReactJS one if I'm being honest. It appears to me - based on setting up breakpoints in the Chrome source code debugger - that every time the ReactPy component changes state somehow and needs to be re-rendered, that it ends up actually recreating (re-initializing? re-defining?) the underlying ReactJS component equivalents via the React.createElement function. This is the raw javascript version of the JSX <Eelment> syntax equivalent, correct?

So even though these JSX elements are being "created" anew every update, that's not synonymous with the actual underlying and associated DOM elements being recreated then, correct? Is that the distinction?

Either way, I uncovered what I believe to be a key piece of the puzzle regarding the Map onClick issue. The clue appears in the @planet/maps code base. I bring it to your attention here simply because I'm not sure if this code is actually a non-issue and still points to a fringe-case issue in ReactPy.

Every time a Map object is instantiated (I'm guessing via React.createElement since this is getting executed on every ReactPy state update), it executes this line of code which calls this updateInstanceFromProps function, but with the oldProps argument hard-coded to {}. Thus, it's not able to recognized that it doesn't need to re-attach the "onClick" event, and re-attaches it.

However... Upon thinking about this more... That would not necessarily explain the exponentially compounding click events... That would make it linear: The first click firing one event, the second click firing two, the third click firing three, and so on... Maybe the entire Map object is being recreated each time and held onto somehow - with each subequent render creating yet another Map from the already existing one(s). I don't know... I'll keep digging.

[shawncrawley]

I was able to reproduce this directly with React(JS), so I've opened up a ticket with the planetlabs/maps project here.