search

swiftwasm / WebAPIKit

Access the DOM and other Web APIs from Swift! (Very much a WIP)
MIT License
61 stars 8 forks source link

Assert on `queriedButton` not `button` in the test #6

Closed MaxDesiatov closed 3 years ago

MaxDesiatov commented 3 years ago

The current test was meaningless as we didn't assert on the text content of queriedButton.

I've also removed addEventListener because it doesn't actually work. The button instance is already deallocated by the time that test has finished, so the closure passed to addEventListener is released too. We need to find a way to run async tests like this, and I have a few thoughts, but this still needs to be explored.

j-f1 commented 3 years ago

Maybe we could stash the event callbacks in a global registry? That’s a simpler case than storing arbitrary functions since we know to remove them from the registry when calling removeEventListener (or watching for the node to be removed from the DOM?) Perhaps we can be OK with some level of memory leakage by default to provide good UX, as long as we document it.

MaxDesiatov commented 3 years ago

What if removeEventListener was never called? The only thing we can rely on is a call of deinit on a corresponding element, which is what we're doing right now. JavaScript won't let us know when exactly that element is gone.

I'm still convinced that the whole closure deallocation problem can be resolved with Wasm reference types as I described in https://github.com/swiftwasm/JavaScriptKit/issues/106. At least reference types are already being worked on in Safari, while I'm not sure if they'll even consider implementing FinalizationRegistry.