fix: cleanup signal listener #1113
Conversation
ronag
force-pushed
the
weak-signal-forward
branch
from
d30fb47 to
dbcc205
Compare
Codecov Report
@@ Coverage Diff @@
## main #1113 +/- ##
==========================================
+ Coverage 94.40% 94.56% +0.16%
==========================================
Files 39 40 +1
Lines 3806 3955 +149
==========================================
+ Hits 3593 3740 +147
- Misses 213 215 +2
Continue to review full report at Codecov.
|
|
@benjamingr @dominic not the prettiest solution... any suggestions? The spec doesn't specify how and when the abort listener should be cleaned up. |
|
Remove the abort listener when you know you are not going to abort (when the request finishes, was aborted, errored etc). FinalizationRegistry is a really heavy handed tool for this |
| ) | ||
| const abort = () => ac.abort() | ||
| signal.addEventListener('abort', abort, { once: true }) | ||
| requestFinalizer.register(this, { signal, abort }) |
There was a problem hiding this comment.
We should be unregistering this after successful completion. Always passing through the registry is expensive.
There was a problem hiding this comment.
Yea, but if we could do that we wouldn't need this hack at all.
It's a little more complicated 😄. The spec doesn't have any request finalisation so I would have to make up something myself. |
|
Oh that's a DOM fetch request? Yeah, I think cheating is probably fine the browser cheats by doing this internally |
ronag
force-pushed
the
main
branch
2 times, most recently
from
63eab1d to
47eb207
Compare
ronag
force-pushed
the
weak-signal-forward
branch
from
82e1a51 to
6d739f8
Compare
* fix: cleanup signal listener * Update lib/fetch/request.js
* fix: cleanup signal listener * Update lib/fetch/request.js
* fix: cleanup signal listener * Update lib/fetch/request.js
No description provided.