03:13 | <Justin Ridgewell> | I had a similar thought reading it. It seems like it make it possible to use any hooks in conditions codepaths. But, it’d only work if the component directly calls the hook, any indirection (like a custom hook) and we’d need to know the call locations if everything in the stack to differentiate. |
04:45 | <Ashley Claymore> | being able to useContext conditionally is a nice follow on, and makes sense that the context can serve as the tracking marker. Not being able to use{State,Effect} conditionally still seems fine, as conditional state is a bit like having a class where the code delete s a field while its not using it instead of setting it to undefined |
04:46 | <Ashley Claymore> | the choice to mutate the promise object rather than require it to be memoized surprised me, considering react is usually all about immutability. |
13:50 | <littledan> | rbuckton: Got it. That's unfortunate. In that case, I think it'd be better to remove all async dispose stuff, as there's no incentive for at least the web platform to add a bunch of aliases/wrappers without syntax support. The rationale for the async dispose stuff in the web platform would be something like:
|
13:50 | <littledan> | Domenic: I don't fully understand how important it is to correct the current conflation that the web platform does (where the same method name is used for both sync and async disposal). If we don't add add the async disposal protocol, we'd have to decide whether web platform objects which have a logically asynchronous disposal should implement Symbol.dispose anyway (at the risk that some usages of these disposals should wait for the "commit" to complete, but the syntax does not provide this). How do you think the web platform should answer this question? |
13:51 | <littledan> | (also a question for annevk :) |
14:10 | <annevk> | littledan: I'd need more context to be useful. I might have read something about this at some point, but I forgot :-) |
14:34 | <littledan> | You might start at https://github.com/tc39/proposal-explicit-resource-management#relation-to-dom-apis (and earlier in that doc) and https://github.com/tc39/proposal-explicit-resource-management/issues/91 |
14:34 | <littledan> | IMO the protocol itself and web platform implementation of it is a huge thing about this proposal's value, and would be worth it to add even if we didn't have the syntax |
14:38 | <littledan> | in particular, I think composing disposables is a big thing, and it will be useful for frameworks to be able to tie into a common construct for this (probably this assertion should be validated with actual framework authors...) |
14:39 | <littledan> | I do think we need a strong web platform review and roughly agreed-on integration plan before this goes to Stage 3 |
14:42 | <annevk> | Interesting, we did talk about this at some point. For addEventListener(), is there some way dispose could integrate with AbortSignal instead? |
14:42 | <annevk> | Perhaps it invokes AbortController's abort(). Which then does a bunch of cleanup. |
14:42 | <littledan> | yeah, I guess this is an addition I'd make to the integration plan above, that disposing an AbortController would abort it. |
14:43 | <littledan> | (my intuitions here are largely based on a conversation I had with wycats) |
14:46 | <annevk> | I guess in general I wonder how much of this can be done through signals instead. I've been kinda seeing that as our "cleanup API". |
14:46 | <littledan> | what do you mean by signals? |
14:47 | <annevk> | https://dom.spec.whatwg.org/#aborting-ongoing-activities |
14:47 | <littledan> | oh AbortSignals |
14:48 | <annevk> | I call them signals as they're somewhat more general-purpose these days (and that's how the dictionary member is called, well in the singular) |
14:48 | <annevk> | Though the documentation doesn't reflect that terribly well |
14:48 | <littledan> | makes sense, the term signal is just a bit overloaded in the ecosystem... |
14:49 | <littledan> | yeah I guess my intuition is that a signal is a more flexible thing in the same space and a bit less convenient, which is why I thought that Symbol.dispose would generalize the set of things that you can abort |
14:49 | <annevk> | As in, do we want Worker's terminate() to be pluggable in some way or should Worker just take a signal |
14:50 | <littledan> | it's sort of convenient to just call the method directly on the thing, rather than keeping around some other object |
14:50 | <littledan> | so... you might want both versions |
14:50 | <littledan> | (not extremely satisifying, the duplication) |
14:51 | <annevk> | Yeah maybe, exploring that seems worthwhile as part of stage 2-3 |
14:51 | <littledan> | yeah we could really use more WHATWG/DOM input here. I'm glad that Ron got this started with a proposal but we need more conversation here. |
14:57 | <annevk> | To get back to the earlier question, I think I agree with Domenic that it would be good if adding an @@ thing provided a benefit of sorts other than a consistent name. I could see that still being the case without syntax if it always returned a promise for the async case, which seems unlikely to be what the web platform currently offers. But syntax support does seem nicer. |
14:58 | <littledan> | One reason the syntax is delayed/omitted is because there's strong interest in an RAII-style syntax for this feature, but it'd be weird if we had a totally implicit await pause at the end of the block |
14:59 | <littledan> | I advocated for just focusing on the sync case for now, and being OK with these async disposals being launched off into space (I think this makes sense for cases that are like resource cleanup, though not for cases that are about committing storage) |
15:00 | <littledan> | an async disposal construct would probably look more block-like (as in Python with statements), but this is less usable for common sync disposal cases, where a flat RAII-style thing is more usable. |
15:49 | <littledan> | I guess AbortSignal has no way to wait for something to be committed... |
17:32 | <rbuckton> | Perhaps it invokes AbortController's abort(). Which then does a bunch of cleanup. AbortController have a concept of "I should never be aborted"? In C#, a CancellationTokenSource can be disposed without calling cancel() , in which case it can never be canceled and any registered subscriptions or linked tokens can be GC'd. I'd love to see something like that for AbortController /AbortSignal , since I'm not 100% sure I agree that disposing of an AbortController should cause an abort. |
17:33 | <rbuckton> | To get back to the earlier question, I think I agree with Domenic that it would be good if adding an @@ thing provided a benefit of sorts other than a consistent name. I could see that still being the case without syntax if it always returned a promise for the async case, which seems unlikely to be what the web platform currently offers. But syntax support does seem nicer. |
18:00 | <littledan> | rbuckton: What do you think of the growing use in the web platform of AbortSignal as a way to dispose of things? |
18:52 | <rbuckton> | Unfortunately, userland use of AbortSignal via the abort event isn't a 100% reliable mechanism for disposal. The abort event uses DOM event dispatch, which is asynchronous, and thus won't work well with the semantics of using with respect to error aggregation (nor any potential Symbol.exitContext extension in the future that might allow for control over error suppression). It also doesn't release the event listeners when the controller is aborted, which can hold onto references that should be GC'd. |
18:54 | <rbuckton> | I don't think AbortSignal is the right primitive from a resource management perspective, though its fine as a async coordination primitive. |
19:03 | <rbuckton> | That said, DOM built-ins aren't handled in the same way that userland abort event handlers are. They are privileged and abort synchronously, but the dual nature of sync abort for built-ins and async abort for userland has the potential to cause confusion. |
19:03 | <littledan> | I guess coming up with a shared understanding of this area would be good |
19:04 | <littledan> | we shouldn't really have two mechanisms that are parallel due to disagreements about how the same problem should be approached |
19:14 | <bakkot> |
... is it?
prints "aborted" before it prints "after calling abort" |
19:17 | <bakkot> | by "asynchronous" do you mean "errors thrown by event listeners aren't propagated to the person who called .abort() "? because I'd imagine that's a solvable problem (controller.abort(null, { handleErrors: true }) or something) |
20:01 | <rbuckton> | Partially, while I admit my original impression was that abort event handlers were invoked in a later turn, there is still the issue that errors are reported out of band from the invocation, and that built-ins are handled before "abort" callbacks regardless of the order things are attached to the signal. |
20:03 | <rbuckton> | Either way, my concern stands. If using controller = new AbortController() were to abort on dispose, I'd like a way to prevent that from happening so that abort event listeners aren't triggered in the event my code runs to completion successfully. |
20:06 | <rbuckton> | I tend to lean more towards the behavior here, based on my prior experience: https://learn.microsoft.com/en-us/dotnet/api/system.threading.cancellationtokensource.dispose?view=net-7.0. CancellationTokenSource.Dispose() does not cancel |
20:08 | <ptomato> | with the way that cancellations are used in GNOME, which is the other platform I'm familiar with that has such a facility, it wouldn't work well if Gio.Cancellable aborted on dispose |
20:08 | <rbuckton> | Mostly because the link between using and "abort" event listeners firing isn't immediately obvious, so its fairly easy to write code that only expects to be triggered while the operation is still active. |
20:11 | <bakkot> | on the other hand,
would be really nice |
20:11 | <bakkot> | (I don't yet have an opinion on this either way, just thinking through use cases) |
20:21 | <rbuckton> | either way, you need to be able to model both behaviors.
|
20:24 | <rbuckton> | I prefer option 1 because its more GC friendly wrt/closures. If disposing the controller doesn't abort it, but instead makes it unusable and frees references to held "abort algorithms" or userland abort event handlers, then it doesn't matter if a closure holds a reference to the controller since everything it was holding can be GC'd. Option 2 doesn't have that benefit. |
20:25 | <bakkot> | hmmm, yeah; on the other hand it makes the simple case a lot uglier |
20:25 | <rbuckton> | Also, option 1 can be simplified by introducing a method on AbortController.prototype that lets you opt-in to "abort-on-dispose" semantics. |
20:26 | <bakkot> | yeah |
20:27 | <rbuckton> | hmmm, yeah; on the other hand it makes the simple case a lot uglier Not if you have some kind of "enter abort context" method like I mentioned above, i.e.:
|
20:27 | <bakkot> | right, I just mean it makes the simple case uglier unless you also add other stuff |
20:28 | <rbuckton> | Defining such lifetime contexts is well within the domain of resource management (see Python's contextlib ) |
20:28 | <bakkot> | I could also imagine instead having a AbortController.AutoCancel constructor, so using controller = new AbortController.AutoCancel() would give you a controller with the auto-cancel behavior |
20:28 | <rbuckton> | You say simple case, but both the "i want to abort on dispose" and the "i don't want to abort on dispose" scenarios are equally as valid. |
20:29 | <bakkot> | mm... I agree they are both valid, but personally I anticipate wanting the "abort on dispose" semantics several orders of magnitude more often than I want the "don't want abort on dispose" semantics, and I expect this is true of almost all JS code |
20:29 | <rbuckton> | My examples illustrate how to do the same thing regardless of each behavior. |
20:30 | <bakkot> | (at least if I have understood correctly that the only use case for disposal-without-abort is for GC) |
20:30 | <bakkot> | and yes I agree it will be possible to accomplish either semantics regardless of which the language makes simple, but I would like to make the common case the simple one, as a general rule |
20:31 | <rbuckton> | mm... I agree they are both valid, but personally I anticipate wanting the "abort on dispose" semantics several orders of magnitude more often than I want the "don't want abort on dispose" semantics, and I expect this is true of almost all JS code |
20:31 | <bakkot> | (which case is more common is of course an empirical question I could be entirely wrong about; this is all off-the-cuff intuition) |
20:31 | <rbuckton> | I'd rather make the case that is less likely to unexpectedly trigger unwanted behavior the easiest to accomplish. |
20:32 | <bakkot> | hm, well, I think I would be more surprised by not-abort-on-dispose than abort-on-dispose, right? |
20:32 | <rbuckton> | Similar to how DisposableStack.from() seems like it would be common, but it's also wrong. |
20:33 | <rbuckton> | Does an AbortController abort if it is GC'd? |
20:33 | <bakkot> | No |
20:34 | <bakkot> | I think I also disagree about which semantics are more likely to be surprising here, and how bad the surprising behavior is in each case. the only thing AbortController right now does is cancellation, so it is natural to assume that |
20:34 | <rbuckton> | While its expected that there will be cases that don't follow that behavior, in general disposables should try to be "better than just GC" |
20:36 | <bakkot> | on the other hand, if using x = new AbortController does do cancellation, and you're expecting it not to (... in which case, why did you write that code in the first place? were you looking for the GC semantics? JS developers probably are not looking for the GC semantics), what's the bug you get? probably an error in the happy path case? so you will almost certainly notice the error |
20:36 | <rbuckton> | One of the big things I wanted for the cancellation proposal was the ability to clean up complex cancellation graphs when not canceled. That is something that has never made it to AbortController /AbortSignal . |
20:38 | <bakkot> | I agree that's a reasonable thing to want, I just expect JS devs will not want that nearly as often as they want "cancel outstanding fetch requests as soon as other simultaneous requests failed" |
20:39 | <bakkot> | I suppose in principle we could solve the "people having the wrong expectations" problem by not making AbortController disposable at all, and instead adding two new properties (.cleanupOnDispose and .abortOnDipsose , say) each of which is disposable in different ways |
20:40 | <rbuckton> | Its perfectly reasonable to want an easy way to make that work, but I still believe it's the wrong default behavior. I'd honestly rather AbortController not be disposable at all than to abort on dispose. |
20:44 | <rbuckton> | Something like this is fairly easy to write in user code, or to build into the DOM API:
|
20:57 | <bakkot> | It would be helpful I think to have more examples of places where you specifically want cleanup-but-not-cancel |
20:58 | <bakkot> | I am mostly thinking about fetch , and cancelling a completed request doesn't do anything, so doing the cancellation isn't problematic |
20:59 | <bakkot> | is the concern things which can't handle being cancelled after they complete, or cases where you no longer want the ability to cancel things which are still in progress? |
21:01 | <bakkot> | I guess cases like addEventListener(x, y, { signal }) probably fall into the second bucket |
21:03 | <ptomato> | you mentioned the use case of sending concurrent fetch requests and cancelling them all when one fails; there's also the case of starting concurrent async operations that aren't redundant with each other and Promise.all'ing them, like reading multiple different config files |
21:03 | <ptomato> | (neither redundant with nor dependent on, I mean) |
21:05 | <rbuckton> | I'm more concerned about userland, since signal.addEventListener("abort", () => { someDestructiveCleanupAction(); }) is likely and every userland "abort" handler would need some way to be informed it shouldn't execute when the operation completed successfully. |
21:06 | <rbuckton> | Ideally, that would be having a mechanism that makes the controller unusable (i.e., cannot abort it), and removes all abort handlers such as a .preventAbort() method. However, if that exists that is what cleanup would entail so I'd argue that is what should happen when disposed. |
21:09 | <rbuckton> | That's the case I want to be the common case, but its not feasible currently without additions to the DOM API. Its important to keep in mind that an AbortController will usually outlive the request that uses its signal . That means there's almost always a period of time after requests complete successfully where the controller shouldn't be aborted. |
21:11 | <rbuckton> | built-ins generally don't care since they're spec'd not to, but its much more complex and cumbersome to do so in user code. |
21:12 | <bakkot> | well, usually just a if (this.#done) return will suffice, but yes it is something you have to handle |
21:12 | <bakkot> | something you probably should be handling either way |
21:17 | <rbuckton> | well, usually just a AbortController /AbortSignal in common, c) may be using third-party code that doesn't have a mechanism to signal disinterest. |
21:19 | <rbuckton> | AbortController has no mechanism to signal disinterest other than "never call abort and let the controller be GC'd", so if we intend to have using on an AbortController mean you can't do that (because abort will always be called), we need some mechanism to opt out. And that opt-out mechanism is exactly what I would expect dispose to do if it exists. |
21:27 | <bakkot> | Sorry, I mean, making your cancelable thing robust against calling "abort()" after the thing has finished is just a matter of doing if (this.#done) return (in the signal listener) |
21:28 | <bakkot> | If everything is robust against abort being called after the thing has finished, then calling abort when disposing of the controller should be harmless, generally |
21:29 | <bakkot> | (except in the rare case that you are done with the controller before you are done with all of the things it controls) |
21:51 | <rbuckton> | "after the thing has finished" is still glossing over the issue. Knowing "the thing has finished" is actually the crux of my concern. Having a means for an AbortController signal disinterest is far cheaper than defending against an incorrect abort for all the reasons I mentioned above. |
21:52 | <rbuckton> | Even if you're done with it, the controller still lives until its signal has GC'd, which in turn keeps everything else alive it's holding. |
21:52 | <bakkot> | I am not understanding the reasons you think it's difficult for a cancelable thing to defend against an incorrect abort. |
21:52 | <bakkot> | or rather "an abort which happens after the thing is finished", which I would not think is incorrect personally. |
21:53 | <rbuckton> | Its not difficult, its time consuming. You have to write scaffolding to handle that case for every single userland "abort" handler. Other implementations don't have this issue, because the ability to signal disinterest is baked in. |
21:55 | <bakkot> | You're almost certainly going to want do that anyway because someone calling .abort after your thing is finished is a totally normal thing to happen |
21:55 | <rbuckton> | Its far too easy to write code that doesn't handle "abort" well. That's not so much a concern if you can just drop the controller on the floor, but if AbortController aborts on dispose and every new StackOverflow example has using controller = ... , we've just made folks lives a lot harder. |
21:57 | <bakkot> | Assuming cancel-on-dispose semantics, the only reason you'd see using controller = ... is if it's useful to call .abort , so we haven't made anyone's lives harder |
21:58 | <bakkot> | presumably if we had a using _ = controller.cancelOnDipose helper, instead of cancel-on-dispose semantics, then the stackoverflow example in question would have that line instead |
21:58 | <rbuckton> | Here's an example: Someone writes a website that asynchronously requests a page of data from the server. They use signal.addEventListener("abort", () => { someElement.innerHTML = "Request aborted" }) , and all is well and good because they only call .abort() when they need to. |
21:59 | <rbuckton> | Now someone makes a change to this working code and switches const to using . Now, after the page of data is presented successfully, it is then replaced with a "Request aborted" message. |
22:01 | <rbuckton> | Many users don't write extremely defensive code in event handlers (i.e., "click", "mousemove", etc.), and they haven't had to do so for AbortController up until now because you can simply just drop it on the floor. using and abort-on-dispose now creates this action-at-a-distance that can be hard to pin down. |
22:02 | <rbuckton> | That, plus it doesn't make sense to abort a completed operation. |
22:03 | <bakkot> | yes, changing your code can cause it to have different behavior. presumably they switched const to using because they wanted it to cancel at the end of the block. this does not really seem problematic to me? |
22:04 | <bakkot> | The whole design of AbortSignal is that it can be passed to multiple things, so that someone might reasonably want to cancel a different operation in a group even after a particular member of the group is done, so I definitely do not agree with "it doesn't make sense to abort a completed operation" |
22:09 | <Kris Kowal> | I think we all agree that whomever calls abort is not in a position to know whether any or all of a tree of dependent operations have completed. |
22:10 | <Kris Kowal> | And that calling abort should be a noöp for any part of the dependent operations that have already completed. |
22:12 | <Kris Kowal> | (Yes, you saw technically correct but still insufferable use of diaeresis in noöp here first. Tell a friend.) |
22:12 | <bakkot> | If we do in fact all agree on that then I don't know what we're disagreeing about |
22:17 | <bakkot> | anyway like I said I don't yet have firm opinions on what we should do here. my only strongly-held opinions are a.) it would be nice if something like my "cancel outstanding requests if any request fails" example were easy and b.) usability of that case should be prioritized above usability of the "I want to make things easier to GC" case |
22:19 | <bakkot> | personally I am not yet convinced that it would be problematic to make AbortController disposal do cancelation but I am not dead set on that particular solution |
22:22 | <Kris Kowal> | If we agree about all that, it’s possible that we also agree that EventTarget is a poor choice of base type for AbortSignal, since it’s up to the user to check whether the signal won or lost the completion race. My feeling is definitely that it’s a poor design. Cancellation tokens should resemble and compose like promises, except that they can only fail, and their completion bit should be synchronously observable. |
22:24 | <Kris Kowal> | And, at that, they should compose with promises such that await Promise.race([cancelled, undefined]) is a sensible way to bail early if already cancelled. |
22:37 | <bakkot> | well, given that AbortController already exists and is ubiquitous it's not all that useful to worry about ways its design could have been better |
22:44 | <rbuckton> | with linking registrations and the ability to signal disinterest, we could have these capabilities in a single place rather than requiring every user roll their own defense mechanism to handle completion/disinterest. |
22:46 | <rbuckton> | The whole design of AbortSignal is that it can be passed to multiple things, so that someone might reasonably want to cancel a different operation in a group even after a particular member of the group is done, so I definitely do not agree with "it doesn't make sense to abort a completed operation" |
22:48 | <rbuckton> | Years ago when I proposed a CancelToken API, I expressly argued for linking registrations and the ability to signal disintrest/cleanup. Some of these things have been slowly making their way to AbortController, so there is hope. |
22:54 | <rbuckton> | With an AbortSignal.prototype.canBeAborted , for example, you can write optimized code paths that avoid subscribing to the signal to begin with. With an AbortController.prototype.close() that sets canBeAborted to false and removes all registrations/abort handlers, you can free up swaths of memory related to abort callbacks that will never be executed. That should exist and is, IMO, probably the single most important thing a cancellation system should do aside from actually signaling cancellation, and is exactly what I'd expect Symbol.dispose to do. All other features of a robust cancellation system (linking registrations, building cancellation graphs, etc.) can be built on subscribing to cancellation and signaling disinterest. You cannot efficiently write them (with good memory/GC) without the ability to signal disinterest and cleanup. |
23:03 | <littledan> | Probably popular opinion which might not be so well-informed: We need to find a way to solve these issues without adding tons of switches |
23:10 | <littledan> | (Or at least, we should avoid adding switches. I don’t really understand the problem Ron is describing) |
23:14 | <Domenic> | Lots of discussion here while I was sleeping, but I think my main belief that web platform integration without syntax is not valuable still stands. I hope you all work on the syntax for async disposes. |
23:15 | <rbuckton> | Some of it is described in the original cancellation proposal: https://github.com/tc39/proposal-cancellation/tree/master/stage0, but I recall there were some open issues to add a linking registration mechanism (even if its just something like AbortSignal.race() or the like). |