adamhaile / S

S.js - Simple, Clean, Fast Reactive Programming in Javascript
MIT License
1.31k stars 67 forks source link

Root + Async = Headaches #23

Closed Qix- closed 5 years ago

Qix- commented 5 years ago

Is there a good way to gracefully handle disposals where I have to build up values or data signals from within async/await code? E.g. loading an image asynchronously and then attaching some styles to the element afterward that create and use signals.

It would be nice to have something akin to Objective-C's non-ARC autopool that acted similar to S.root() but as a bit more manual - for example, something like:

const sroot = S.root();
try {
    await myCode();
} finally {
    sroot.release();
}
Qix- commented 5 years ago

Or, I suppose alternatively, just accept an async function into S.root().

adamhaile commented 5 years ago

Hey @Qix- Can you post a code example of the use case you're having issues with? I said in the issue about Promises that interop between async code and S is something I'd call a "current area of research" :). I might be able to understand better the issue you're facing with a code example. Plus, I'm eager to see how people are attempting to use S and async.

I mention in that issue my usual way of handling async with S, which is to proxy through a data signal, either with raw data or a generator function.

S.root() can accept an async function, since an async function is a function that returns a promise. So your snippet could be rewritten:

S.root(async release => {
    try {
        await myCode();
    } finally {
        release();
    }
});

But that would dispose any computations created as soon as the promise returned by myCode() completed. Not sure if that was what you intended or not. It also won't capture any computations generated after the first await (you don't do that in the example, but if you did), as S.root() has returned by that point. -- Adam

Qix- commented 5 years ago

Your first point about disposing computations, that's a problem because the call to myCode() in this case is an async surplus element generator function.

Your second point about capturing computations outside the call also is a problem because new computations might happen outside of the call.

I mostly understand when I'm using global computations in a way where they won't be disposed. It might be useful to instead have the console warning when you try to specify a disposal function where one will never be called, though I'm not sure how feasible that is.

I'm currently seeing hundreds of warnings because of async code at the moment.

I kind of hacked it together (it's not the cleanest) but a colleague was making a matching game using react hooks and I wanted to see how easy it'd be to do one with surplus. You can see the result at https://GitHub.com/qix-/matching-game-surplus-electron. That's the experiment that prompted me to open this ticket.

adamhaile commented 5 years ago

I have a soft spot for making simple games. I usually use surplus-toys to avoid the need for a build system and keep them single-file.

It's often important in S apps to avoid implicit state -- application state which isn't contained in a data signal and isn't immediately addressable. In your app, the implicit state is whether the tile image has loaded yet. If we were to ask "has the tile loaded yet?", we can't really answer it. Instead, you're using async/await to say "we won't run this code until it has." So the state is encoded into your execution order, which means it also has to propagate all the way up the call tree.

My advice would be to make that state explicit, by making an image data signal that starts as null and then is populated once it has arrived.

I just pulled and made the changes locally. Where you did:

        const img = await getImage();
        const imgClone = img.cloneNode();

... I changed to ...

        // make image load state explicit via data signals
        const img = S.data(null);
        const imgClone = S.data(null);

        getImage().then(_img => {
            // populate image data signals once we have them
            img(_img);
            imgClone(_img.cloneNode());
        });

Since img was now a signal, I changed {img} to {img()} in makeTile(img).

Then strip all the async and await calls from the file.

Qix- commented 5 years ago

to avoid the need for a build system and keep them single-file.

I'm a build systems nerd - this is one I built up for another app that I just stripped out for this small little exercise :) Seems overkill, because it is haha. It's part of a much larger project ;) surplus-toys is super neat though!

My advice would be to make that state explicit, by making an image data signal that starts as null and then is populated once it has arrived.

Derp, that's so obvious now. I think this is also a side effect of me mixing logic with my display code, which I did here for time's sake but wouldn't normally do.

Thanks, this is much better.