Repository navigation
Expose setMaxListeners property on AbortSignal #54758
Description
Activity
- addedfeature requestIssues requesting new Node.js features.Issues requesting new Node.js features.
on Sep 4, 2024 The right avenue to purse this change is WHATWG (the group maintaining those standards). From past experience, any time we deviate from the standard we cause more harm than good.
Reacted by Jordan Harband, Benjamin Gruenbaum and DeepanshuBut isn't
maxEventTargetListenersin itself - a limit on the number of listeners and issuing a warning - already a node-only deviation from the standard?So this would try to explore a way to get closer to the standard, not further away.
Reacted by Ben DurrantReacted by Neng SitipornBy that logic, another alternative suggestion here could be:
Alternative suggestion: set no default listener count limit on
AbortControllerOn all newly created
AbortSignalinstances, set thekMaxEventTargetListenersto 0 by default to align with standard behaviour - users that want a limit on event listeners can still usesetMaxListenersto set a specific limit.In the research before opening this ticket, I've seen a lot of instances where people were getting this warning with undici
fetch, so this seems to be a somewhat common problem.In my case, this is ironically caused because I want to pass an
AbortSignalas thesignaloption to a lot ofaddEventListenercalls so I can make sure to clean them up.Either way, it seems that
AbortSignalis just made to be reused by manyfetchcalls oraddEventListenercalls, so the current limit of 10 is easily reached by normal intended usage (and afaik not to spec).- addedweb-standardsIssues and PRs related to web-platform APIs and standards compliance.Issues and PRs related to web-platform APIs and standards compliance.
on Sep 4, 2024 @jasnell you added the warning in
#36001@KhafraDev you added those getters/setters in
#47039What do you folks think?
node shouldn't have implemented warnings or interop with EventEmitter in the first place, but we absolutely shouldn't deviate further from the spec and other implementations
Reacted by Jordan Harband and Benjamin Gruenbaum@nodejs/web-standards what do you all think?
Pinging @lucacasonato too (hope you don't mind).
Agree with @KhafraDev. If anything this should be a setter function you import from
node:eventsthat you pass the signal to as the first argument.I agree. We should not extend the standard API with non-standard extensions.
+1 (no nonstandard extensions). I do like the alternative suggestion tho (#54758 (comment))
I agree with the alternative suggestion as well
7 remaining items
@phryneas I think that library is simple to create, as I think most bundlers allow for browsers alternates, so that some code will execute in Node.js but never in the browsers, making it isomorphic.
So we've gone from a library with a single (and rather short) published code file to a library with 3 published code files (2 environment-specific ones, one shared), an
exportsfield with nested conditions and a sophisticated build setup.I might as well publish this on npm as
isomorphic-abortsignalin the end, to not bloat the original library and enable others to use it without going through the same pain.Honestly, this feels wrong - and I'm sorry that I voice my frustration here, but I feel like it needs to be said.
This is not how things should end up when node starts picking up web standards - the need for these workarounds should go down, not up :(
@phryneas even if we removed the warning, this will hardly trickle down to past releases. I'm not even sure we should make it semver-minor or not.
Anyhow, would you like to send a PR to remove the warning limit? I don't see much opposition in keeping it.
@phryneas even if we removed the warning, this will hardly trickle down to past releases. I'm not even sure we should make it semver-minor or not.
That's still a lot better than doing nothing :)
Anyhow, would you like to send a PR to remove the warning limit? I don't see much opposition in keeping it.
Will do!
- added a commit that references this issue
on Dec 7, 2024 - added a commit that references this issue
on Dec 10, 2024 - added a commit that references this issue
on Dec 20, 2024 - added a commit that references this issue
on Jan 5, 2025 github-actions commented
on May 11, 2025 on May 11, 2025 – with GitHub ActionsContributorMore actionsThere has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale
Issues and PRs exempt from automated stale handling. label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.- addedstaleIssues and PRs marked stale due to inactivity and scheduled for automatic closure.Issues and PRs marked stale due to inactivity and scheduled for automatic closure.
on May 11, 2025 Solved for my use case via #55816
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsAwaiting Triage
What is the problem this feature will solve?
Right now, there is no stable way to change the
maxEventTargetListenersof anAbortSignalwithout importing fromnode:events.This makes it impossible to write isomorphic library code that adds more than the default 10 event listeners, without having to resort to workarounds such as
This relies on the
events.maxEventTargetListenerssymbol description, which, to my knowledge is not part of any public API and could change at any time.What is the feature you are proposing to solve the problem?
Add a
AbortSignal.setMaxListenersinstance method so isomorphic code could do something likeWhat alternatives have you considered?
Alternatively, if this is not an option as it could pollute a spec-defined object with additional properties, use
Symbol.forinstead ofnew SymbolforkMaxEventTargetListenersand make that a part of the official api.I would be willing to implement this feature.