Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Root + Async = Headaches #23

Closed
Qix- opened this issue Apr 11, 2019 · 5 comments
Closed

Root + Async = Headaches #23

Qix- opened this issue Apr 11, 2019 · 5 comments

Comments

@Qix-
Copy link

Qix- commented Apr 11, 2019

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-
Copy link
Author

Qix- commented Apr 11, 2019

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

@adamhaile
Copy link
Owner

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-
Copy link
Author

Qix- commented Apr 11, 2019

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
Copy link
Owner

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-
Copy link
Author

Qix- commented Apr 11, 2019

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.

@Qix- Qix- closed this as completed Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants