11:10 | <Andreu Botella> | I wonder if dispatchSnapshot as a property of events would be clear to developers who have no clue what AsyncContext even is or what it's meant for |
11:11 | <Andreu Botella> | well, I guess we don't need it to be clear, we just need them not to use it by mistake |
12:46 | <littledan> | An important thing is that it’s googleable |
12:47 | <littledan> | We could be very explicit and say causalAsyncContextSnapshot. Too long? |
12:59 | <littledan> | Anyway, all of these options should be googleable/searchable in mdn so this name search doesn’t have a big downside risk |
13:04 | <Chengzhong Wu> | I think it is ok to have a single property name to be generic but specifically searchable with its holder's name |
13:06 | <littledan> | It might be acceptable, but will be not as convenient for some people |
13:07 | <littledan> | Especially here, this will come up on a lot of autocompletes and people will be trying to figure out what it is |
17:49 | <Andreu Botella> | Hey, we're discussing whether any of you have use cases for having the dispatch snapshot on user-created events |
17:51 | <Andreu Botella> | So 1) should it be there at all, and 2) if so, should the default behavior would be to take the snapshot, or to have it as null |
17:52 | <Andreu Botella> | One use case is polyfills, but the property could be added with Object.defineProperty |
17:52 | <Steve Hicks> | For user-created events, could you define class SnapshottingEvent extends Event { dispatchSnapshot = new AsyncContextSnapshot(); } and then just dispatch that? |
17:53 | <Andreu Botella> | yeah |
17:54 | <Andreu Botella> | so I guess the discussion is rather, are there use cases for having the dispatchSnapshot property populated by default? |
17:54 | <Steve Hicks> | So then the question is whether the dispatching code should need to be concerned about it |
17:57 | <Steve Hicks> | I don't have a good sense here... putting it on the Event constructor wouldn't be exactly accurate w.r.t. being the dispatch snapshot, if event objects are shared or preallocated. To really be accurate, you'd want dispatchEvent to populate it, but then what does that function use to decide whether to populate it or not...? |
18:01 | <Andreu Botella> | usually the creation and dispatch times is the same, and even for web platform events the spec text I'm proposing would set it when the event is created (not sure if that difference would be observable for any event) |
18:01 | <Andreu Botella> | so I guess technically dispatchSnapshot wouldn't be an accurate name |
18:05 | <Steve Hicks> | event creation time seems very reasonable |
18:05 | <Steve Hicks> | what's the reasoning to not put a snapshot on every event? |
18:05 | <littledan> | well, we don't want to use the registration-time snapshot--the we are back to Zalgo fallback logic |
18:05 | <littledan> | (if we care about that) |
18:06 | <littledan> | many events only have one relevant snapshot, and that's the registration-time one |
18:06 | <Steve Hicks> | sure, but for those the dispatchSnapshot (name tbd) would just be empty |
18:07 | <littledan> | that was another point that Andreu and I were chatting about: should the property be missing, or filled in with null? Andreu convinced me the latter is better |
18:07 | <littledan> | it's more consistent to say, it's an attribute of Event |
18:07 | <Steve Hicks> | or filled in with the top-level snapshot? |
18:07 | <littledan> | what's the reasoning to not put a snapshot on every event? |
18:07 | <littledan> | what we don't want to do is change the logic defining the snapshot later |
18:07 | <littledan> | or filled in with the top-level snapshot? |
18:08 | <littledan> | I thought we wanted to let people know whether there was a more interesting causal snapshot |
18:08 | <Steve Hicks> | some events would require quite some complexity around various specs to find the right one |
18:09 | <Stephen Belanger> | As APMs tend to view what we are patching somewhat generically, it'd be in our interest for the snapshot to always be there regardless of origin. |
18:09 | <Andreu Botella> | I don't imagine doing any searching here - for consistency this property is just the snapshot when the event is created. If there's another snapshot that's more relevant, it would need a different property |
18:09 | <Andreu Botella> | and that's not trivial |
18:10 | <littledan> | there's always a snapshot used when running the event handler, there's just not always an additional causal snapshot presented |
18:10 | <Andreu Botella> | the idea is to handle that for a subset of use cases in the initial rollout |
18:10 | <Andreu Botella> | and then incrementally increase that set |
18:10 | <littledan> | if we always provided one, we'd want to change some of them later as we incrementally increase accuracy |
18:10 | <littledan> | but the other option is for it to be null, to say "no additional causal information to see here" |
18:10 | <Steve Hicks> | I thought we wanted to let people know whether there was a more interesting causal snapshot |
18:10 | <littledan> | this field is not a feature that ordinary programmers should use |
18:11 | <Stephen Belanger> | there's always a snapshot used when running the event handler, there's just not always an additional causal snapshot presented |
18:11 | <littledan> | I see where you're going with this, but I'm concerned about the consistency of whether we're in an empty snapshot or not in a snapshot at any given time |
18:11 | <Steve Hicks> | this seems like it would be implying that the event loop runs outside of any snapshot |
18:11 | <littledan> | the event runs in the registration time snapshot |
18:11 | <littledan> | no |
18:11 | <littledan> | we're talking about the additional causal snapshot, which you can .run if you want |
18:11 | <Steve Hicks> | s/empty snaphot/host-created initial snapshot/ whatever |
18:11 | <littledan> | yeah that doesn't come up here |
18:11 | <littledan> | that only comes up for modules |
18:12 | <Andreu Botella> | That's the registration time though, right? APMs need the dispatch context. |
18:12 | <Stephen Belanger> | we're talking about the additional causal snapshot, which you can .run if you want |
18:12 | <littledan> | That's the registration time though, right? APMs need the dispatch context. |
18:12 | <Steve Hicks> | that only comes up for modules |
18:12 | <littledan> | yes that too, but not events |
18:13 | <Stephen Belanger> | I don't see making sure that the dispatch context is tracked for every single event with an asynchronous source as feasible in a short amount of time |
18:13 | <littledan> | I mean, if the plan is to reserve a slot for it to be there but just not having all sources supported immediately, that's probably fine. Most APMs don't really do full structural tracing in the browser yet anyway. 🤷 |
18:13 | <Stephen Belanger> | I mostly just want to be sure we're going in a direction that won't block us from being able to support that in the future. |
18:14 | <littledan> | I worry that if we always provide something for the sake of it, it could disrupt existing programs to change that snapshot. By contrast, the contract for "null" would be "the platform might give you something later, but not now" |
18:14 | <littledan> | because if we always give you something for the sake of it, we'd end up using the registration time snapshot as the fallback |
18:15 | <Stephen Belanger> | Yeah, that seems acceptable to me. |
18:15 | <littledan> | (but you don't need to be given that--you're already running in the registration time snapshot) |
18:15 | <Steve Hicks> | My thought here is that any given property should have a clear and easily-understandable rule for where its snapshot comes from |
18:15 | <Stephen Belanger> | For more immediate term we mostly only care that async/await is handled appropriately, thus my push on through-flow. |
18:15 | <Steve Hicks> | so if it's anything other than "the snapshot when the event was (created|dispatched)" then it should be a different property |
18:16 | <Stephen Belanger> | All the other asynchrony is Node.js-provided, so we can just make whatever changes we need to make to Node.js to capture and restore in the correct places. |
18:17 | <Steve Hicks> | what I want to avoid is "for event X, .dispatchSnapshot comes from Y, for event Z, .dispatchSnapshot comes from W, for event U, .dispatchSnapshot is null, etc" |
18:17 | <Steve Hicks> | this would be a nightmare both to reason about and to polyfill. |
18:17 | <littledan> | The idea is, there is no property for the registration time (it just gets run in that snapshot) and there is a property for this causal one (often null, but the rules for filling it are complex and evolving) |
18:17 | <littledan> | We could also expose the registration time snapshot as a property |
18:18 | <Steve Hicks> | On the other hand, if (say) we wanted to expose the .sendSnapshot for XHR events, that could be a different property with different rules |
18:18 | <littledan> | Causal snapshots are just inherently complex but they may be useful |
18:18 | <Stephen Belanger> | There are some subtleties like timers which we implement and so those should probably follow the spec for how flow works, but technically we implement those ourselves so we can violate the spec a bit if we need to for our use case. 😅 |
18:18 | <Andreu Botella> | so if it's anything other than "the snapshot when the event was (created|dispatched)" then it should be a different property |
18:18 | <Andreu Botella> | for XHR, you could imagine .send() as starting a background fetch, and creating the event after that await |
18:18 | <littledan> | For browser-internal asynchronous events, this "creation/dispatch time" would be propagated browser-internally, as if there were awaits inside the browser |
18:19 | <Steve Hicks> | for XHR, you could imagine |
18:19 | <littledan> | We could make two properties, I was just talking about the causal one |
18:19 | <Steve Hicks> | I think "causal" is too loaded a name |
18:20 | <littledan> | Ugh I don’t care what we call it, the complex one |
18:20 | <littledan> | I just thought there was no need to expose the registration time snapshot in a property |
18:20 | <littledan> | Since we are already running the event there |
18:21 | <Andreu Botella> | yes, which is why I don't see it as appropriate to use the same snapshot property for this use case |
18:21 | <littledan> | But we could if it is useful |
18:21 | <Steve Hicks> | but that's what I'm saying - we've simplified the rules for which snapshot to run in to not be complex, I think we can do the same for these properties by keeping them similarly well-defined, and deferring the logic of selecting which one is needed to the holder of the event object |
18:22 | <Steve Hicks> | I just thought there was no need to expose the registration time snapshot in a property |
18:23 | <Stephen Belanger> | I think part of the problem is that AsyncContext seems to be viewed as an isolated data type rather than a runtime feature. For example, I went through and watched all the SES review meetings the other day and they circled a whole lot around the idea of the internal context map being a mutable global, but if you consider it as a runtime-level thing there are lots of mutable globals. They eventually figured out it's like passing an implicit parameter, but didn't get quite so far as realizing that it's essentially just a reserved register for passing context same as you would do for passing lexical scope. |
18:24 | <Stephen Belanger> | And for through flow, it's the same as just having an extra reserved register for multiple-return with one return value reserved for outward flowing context. |
18:26 | <Stephen Belanger> | If you think of it at the level of being a runtime feature the internals of the type actually become kind of irrelevant from the SES perspective. |
18:26 | <Andreu Botella> | in every case I've looked at, when there is a synchronous dispatch context, there is no relevant asynchronous dispatch context |
18:26 | <Andreu Botella> | so it would be two properties where at least one of them would always be null |
18:28 | <Stephen Belanger> | Considering it at the runtime level also makes the context segments between awaits make a lot more sense as you can then visualize it alongside the execution DAG and flowing context along the same edges as the execution data would flow. |
18:28 | <Stephen Belanger> | Context is very much a runtime feature, not a data type. |
18:28 | <Steve Hicks> | so it would be two properties where at least one of them would always be null |
18:29 | <Andreu Botella> | if you call into some API from JS, and that synchronously dispatches an event, that is a synchronous dispatch context (e.g. el.click() ) |
18:29 | <Andreu Botella> | the async dispatch context is when you call into some API that causes some async operation behind the scenes that eventually fires an event |
18:30 | <Steve Hicks> | gotcha, so async would be like xhr send. And in some cases there's neither (like a UI click) |
18:30 | <Andreu Botella> | right |
18:32 | <Andreu Botella> | there are some cases where there are multiple possible async dispatch contexts, but those are the extreme minority (in the subset of events I've looked at, this can only happen for media events, and would not be the usual case) |
18:33 | <Steve Hicks> | Is anyone else tracking what I'm saying about trying not to be smart here, though? It still seems like a pretty big win in terms of future direction/flexibility if we keep the scope narrow by simply saying .dispatchSnapshot is this-and-only-this, and if we need something else in the future, it will be a different property. This lets us be very consistent with applying it immediately to all events, while still have flexibility to add different snapshots later. The alternative means we need to be very conservative. |
18:34 | <Steve Hicks> | how many different async dispatch contexts have you identified? |
18:34 | <Andreu Botella> | for media events? |
18:34 | <Steve Hicks> | (in short, I'm allergic to special cases) |
18:34 | <Steve Hicks> | no, across all events |
18:34 | <Andreu Botella> | what do you mean by "different async dispatch contexts"? how would you categorize them? |
18:35 | <Steve Hicks> | e.g. xhr send would be one, media events is one or a few |
18:36 | <Steve Hicks> | what other events have dispatches triggered asynchronously from a concrete point in code? |
18:36 | <Steve Hicks> | are there a bunch more, or is it just a small handful? |
18:37 | <Andreu Botella> | there are a bunch more |
18:37 | <Andreu Botella> | a lot of events are dispatched asynchronously |
18:37 | <Andreu Botella> | e.g. setting img.src eventually fires a load event |
18:40 | <Andreu Botella> | some events are sometimes dispatched synchronously and sometimes asynchronously – e.g. window.print() will print synchronously if the page is fully loaded, but it will wait if it's not loaded yet, so the beforeprint and afterprint events can be dispatched sync or async |
18:41 | <Andreu Botella> | I don't have a list in front of me of different event behaviors, but many (maybe most?) that are triggered by some JS code are asynchronous |
18:41 | <littledan> | who's talking about registration snapshot as a property? |
18:42 | <littledan> | so if you want a reliable property, we can add the registration one, but that wouldn't give us this other information |
18:43 | <Andreu Botella> | I think the only cases where there would be a subjective choice would be cases like the media events, those would be the extreme minority I think |
18:44 | <Steve Hicks> | the only thing that can be reliable is the registration time one, because the other one is very context-dependent and complex, and sometimes involves API designers making a subjective choice between a couple options |
18:46 | <Steve Hicks> | I think we need to put ourselves in the shoes of someone who's accessing these properties. Are they likely to be looking for "just give me any meaningful causal context, I don't care where it comes from" or will they tend to know exactly the property they need in order to get a specific shapshot from a single well-defined point in time? |
18:46 | <Steve Hicks> | (I don't have an answer to this) |
18:47 | <Andreu Botella> | In cases where you have multiple async dispatch contexts, like media events, I don't think you can split them out into narrowly scoped types of contexts |
18:47 | <Andreu Botella> |
|
18:47 | <Andreu Botella> | this is essentially a merge, just inside the browser |
18:48 | <Andreu Botella> | how would you narrowly scope those branches? |
18:49 | <Steve Hicks> | would either or both videoEl.load() and/or videoEl.play() by itself have triggered the event? |
18:49 | <Andreu Botella> | yeah |
18:49 | <Andreu Botella> | both |
18:49 | <Andreu Botella> | IIRC, but I'm fairly sure |
18:50 | <Steve Hicks> | naively, I'd suggest loadSnapshot and playSnapshot would be analogous to XHR's sendSnapshot , and one or both may be populated depending on the circumstances? |
18:50 | <Steve Hicks> | (for transparency, I don't have a particular horse in this race, I'm just trying to make sure we're considering all the options) |
18:50 | <Andreu Botella> | okay, I guess I didn't explain this clearly enough |
18:51 | <Andreu Botella> | if a video is stopped, IIRC .load() will start fetching it, .play() will start fetching and then play when ready. But only one load event is fired, when the load starts(?) |
18:51 | <Andreu Botella> | so you could call .play() multiple times |
18:52 | <Andreu Botella> | and that would result in a single load event |
18:52 | <Steve Hicks> | ah |
18:52 | <Steve Hicks> | so it's a debouncing issue |
18:52 | <Andreu Botella> | exactly |
18:52 | <Steve Hicks> | seems reasonable to go with the first one? |
19:20 | <littledan> | right, but there's no point because it's already the one the callback is run in. What I'm saying is that we can maybe reduce the complexity and context-dependence by defining many narrowly-scoped properties, such that each individual one is simple, well-defined, and straightforward, even if it usually doesn't apply. |
19:21 | <littledan> | I like this idea! |
19:21 | <littledan> | then it should be easier to find docs which describe where that comes from |
19:21 | <littledan> | I imagine it should be just as usable by APMs right? |
23:33 | <Stephen Belanger> | Probably fine. Might be some challenges with selecting the appropriate context in some cases, but we do generally need to be able to better support context merges, so that's somewhat an us problem that needs to be dealt with. |
23:57 | <littledan> | So this is (hopefully) a primitive that gives you both (sometimes) so it’s a good basis? |