Preact: Render inside an event hander (like onclick) triggers an event for the wrong element

Created on 30 Nov 2018 · 4Comments · Source: preactjs/preact

Thank you for the minimal library!

I think I have come across a bug where an additional event handler is triggered when it shouldn't be.
This happens in a special case of a render being called inside an event handler.
Here is my code example.

<!doctype html>
<script src="https://cdnjs.cloudflare.com/ajax/libs/preact/8.3.1/preact.dev.js"></script>
<div id=appEl>
</div>
<script>
var state = {
    showRow: false,
}
var appEl = document.getElementById("appEl");
var h = preact.h;
function View(state) {
    return h("div", null
        , state.showRow ? h("div", {}, "New Node") : null
        , h("div", {onclick: function() { console.log("** Unrelated Handler **"); }}, "Unrelated Element")
        , h("div", {} // this wrapper is important to demonstrate the bug
          , h("div", {onclick: function() { state.showRow = true; render(); console.log("Clicked!"); }}, "Click Me")
        )
    );
}

function render() {
  preact.render(View(state), appEl, appEl.firstChild);
}
render();
</script>

The first time that a user clicks the "Click Me" div, both the "Click Me" handler and the "Unrelated Element" handler will get called.

The console log will show this:

Clicked!
** Unrelated Handler **

(jsfiddle: https://jsfiddle.net/vawLh75m/)

I think this is because when the "New Node" is added at the top, the dom node that was for the "Unrelated Element" gets pushed down to become the wrapper node for the "Click Me" div.
That happens in the same event loop cycle that the "Click Me" div is clicked, so when the event bubbles up to the wrapper div, the wrapper div still carries the event handler for the "Unrelated Element" and it gets fired -- even though the wrapper div itself doesn't have an event handler defined.

(Edit: What is actually happening is when render is called when user clicks "Click Me", the wrapper div for "Click Me" becomes the "Unrelated Element" and gets the Unrelated Element's event listener. The "Click Me" click event bubbles up up to that wrapper div -- which is now the "Unrelated Element" so the Unrelated Element's event handler will fire.)

The simple workaround is for me to setTimeout (or similar) in the "Click Me" handler.
It would be nice to not have to worry about this case.

Is this something worth fixing? I would like to spend some time working on a fix.

Thank you!

bug duplicate

Source

drewlesueur

Most helpful comment

Duplicate of #857.

Preact cannot perfectly handle unkeyed same type children mounting/unmounting

You can solve it by adding keys: https://jsfiddle.net/vawLh75m/1/

yaodingyd on 30 Nov 2018

👍2

All 4 comments

Duplicate of #857.

Preact cannot perfectly handle unkeyed same type children mounting/unmounting

You can solve it by adding keys: https://jsfiddle.net/vawLh75m/1/

yaodingyd on 30 Nov 2018

👍2

This can be fixed by avoiding synchronous re-render from within an event handler. This is recommended for performance as well.

/** Assumes a global "preact"
 *  createRenderer(
 *    View,         // (state, update) => dom
 *    initialState, // gets passed to View() first
 *    parent        // the element to render into
 *  )
 */
function createRenderer(app, state, parent) {
  function Renderer() { this.state = state; }
  Renderer.prototype = new preact.Component();
  Renderer.prototype.render = function() {
    return app(state, update);
  };
  let component;
  preact.render(preact.h(Renderer, { ref: function(c) { component = c; } }), parent);
  const update = component.setState.bind(component);
}

Here's how you'd use it for your example (which would fix the issue):

function View(state, update) {
    return h("div", null
        , state.showRow ? h("div", {}, "New Node") : null
        , h("div", {onclick: function() { console.log("** Unrelated Handler **"); }}, "Unrelated Element")
        , h("div", {} // this wrapper is important to demonstrate the bug
          , h("div", {onclick: function() { update({ showRow = true }); console.log("Clicked!"); }}, "Click Me")
        )
    );
}

createRenderer(View, {
    showRow: false,
}, document.getElementById('appEl');

developit on 6 Dec 2018

👍1

Just a note @drewlesueur - you might like preact-cycle - it's basically a nicer version of what I wrote above.

developit on 6 Dec 2018

@developit Thank you very much. I will look into this.

Another solution that worked for me (besides doing a setTimeout or adding a key) was to update my original render function:

function render() {
    if (window.event) {
        window.event.stopPropagation()
    }
    preact.render(View(state), appEl, appEl.firstChild);
}

A little hacky, but that prevents a synchronous render from propagating the event to a new node that should no longer receive that event -- although that might cause its own problems.

I will look into asynchronous renders like you showed. Thanks again.

Thank you @yaodingyd for suggesting the key solution too!

drewlesueur on 6 Dec 2018

👍1

Was this page helpful?

0 / 5 - 0 ratings