fengari-web icon indicating copy to clipboard operation
fengari-web copied to clipboard

`EventTarget:removeEventListener()` does not work

Open oezingle opened this issue 10 months ago • 5 comments

Hi all,

Fengari seems like an awesome project. I'm working on integrating a UI library I wrote with it for web support, but I've run into a small issue - it looks like events can't be removed from Elements on-the-fly. I assumed this was a JavaScript limitation, but it appears to work fine in JavaScript in Firefox ("Mozilla/5.0 (X11; Linux x86_64; rv:136.0) Gecko/20100101 Firefox/136.0" ) and Chromium ('Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/134.0.0.0 Safari/537.36')

Here's a demo HTML file:

<html>

<head>
    <title>Fengari bug demo</title>

    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/fengari-web.min.js"></script>
</head>

<body>
    <button id="btn-lua">Lua button</button>
    <button id="btn-js">JS button</button>

    <script type="application/lua" async>
        local document = require("js").global.document

        local btn = document:querySelector("#btn-lua")
        local handler
        handler = function ()
            print("Button click lua")

            btn:removeEventListener("click", handler)                
        end
        btn:addEventListener("click", handler)
    </script>

    <script>
        const btn = document.querySelector("#btn-js")

        let handler
        handler = function () {
            console.log("Button click js")

            btn.removeEventListener("click", handler)
        }
        btn.addEventListener("click", handler)
    </script>
</body>

</html>

In the lua script, btn:removeEventListener fails to work, which can be checked by clicking the "Lua button" multiple times. I tried to get around this with setTimeout to no avail.

Removing the event listener right after adding it does not work ether:

btn:addEventListener("click", handler)
btn:removeEventListener("click", handler)                

oezingle avatar Mar 07 '25 19:03 oezingle

Looking around the codebase, it seems this is moreso an issue with fengari-interop - tojs calls wrap, which creates a new proxy object every time a Lua object is marshalled. This works fine in most cases, but results in a distinct object being passed to addEventListener and removeEventListener. ~~Could wrap get around this by referring to an Object (JS side) that contains WeakRefs to proxies?~~

edit: I'm working on a patch, I feel WeakRef is too new of a feature to rely on. Garbage collection will remain an issue.

oezingle avatar Mar 07 '25 19:03 oezingle

I have a partial (leaky) fix for this in https://github.com/fengari-lua/fengari-interop/pull/84

oezingle avatar Mar 07 '25 20:03 oezingle

edit: I'm working on a patch, I feel WeakRef is too new of a feature to rely on. Garbage collection will remain an issue.

fengari-interop already relies on WeakRef: https://github.com/fengari-lua/fengari-interop/blob/d6362d1fc7621433ae41d7425d98281495f1877d/src/js.js#L178

daurnimator avatar Mar 08 '25 08:03 daurnimator

If I'm not mistaken you could generalize that to work with strings as well. Which would speed things up a lot since you would only need to convert an interned Lua string to a JS string once, after the first conversion it would use the WeakMap

rweichler avatar Apr 25 '25 04:04 rweichler

Which would speed things up a lot

Would it though? I'd love to see some real code where it's a significant slow point

since you would only need to convert an interned Lua string to a JS string once, after the first conversion it would use the WeakMap

I think I once wrote this code, but decided against it for some reason.... Perhaps because JS strings can have fields set on them? But I'm unable to replicate that now:

$ node
Welcome to Node.js v23.11.1.
Type ".help" for more information.
> a="asdads"
'asdads'
> a.foo = 1
1
> a.foo
undefined

Are strings stateful in any version of JS?

Note that we already have a cache going the other direction: https://github.com/fengari-lua/fengari/blob/bd945f0ef42caad86aff59365fcb8a3ec8509d06/src/defs.js#L177

daurnimator avatar Jun 03 '25 04:06 daurnimator