doc: add snippet for AsyncResource and EE integration #33751
Conversation
puzpuzpuz
added
the
async_hooks
puzpuzpuz
force-pushed
the
doc/add-async-resource-bind-sample
branch
from
b894b3e to
9fa4728
Compare
There was a problem hiding this comment.
It may be worthwhile building this in the the EventEmitter API (and possibly the EventTarget API). We've talked before about adding a context type of option to events, which could be expressed as an additional argument to addListener/on/once ... e.g.
const asyncResource = new AsyncResource('request');
req.on('close', () => {}, asyncResource)Under the covers it would effectively just do exactly what this example does, but in a way that is a bit more ergonomic for the user. For instance, in order to subsequently remove the event listener, the user has to track the bound function themselves (e.g. const fn = asyncResource.runInAsyncScope.bind(asyncResource, () => {}); ee.on('foo', fn'); ee.off('foo', fn); which is a bit cumbersome when we could handle it internally in a way similar to how we approach once handlers.
For now, however, this example is very valuable.
puzpuzpuz
force-pushed
the
doc/add-async-resource-bind-sample
branch
from
9fa4728 to
0648fff
Compare
puzpuzpuz
force-pushed
the
doc/add-async-resource-bind-sample
branch
from
0648fff to
ed845b8
Compare
|
For what it's worth, I still think having an actual method for this is friendlier, and it's simple enough that the maintenance burden is near zero. bindToAsyncScope(fn, thisArg) {
return this.runInAsyncScope.bind(this, fn, thisArg);
}I'm +0 on this. If this is how we want to proceed, that's fine. 🤷 |
There was a problem hiding this comment.
Yep, my perspective was just due to the other issue being closed before so it seemed like we decided against that route in favour of only documenting it. I would still like to work towards an actual fix in core. Since it seems the doc change is being considered more as a potentially temporary solution, I'll upgrade to 👍 .
PR-URL: #33751 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]>
|
Landed in db3d6b3 |
PR-URL: #33751 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]>
PR-URL: #33751 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]>
PR-URL: #33751 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]>
PR-URL: #33751 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]>
Continuation of #33736
Adds a snippet for
AsyncResource+EventEmitterintegration, based onasyncResource.runInAsyncScope.bind.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes