Skip to content

fix: drop any in abort controller#997

Merged
ferhatelmas merged 1 commit intomasterfrom
ferhat/abort-controller
Apr 14, 2026
Merged

fix: drop any in abort controller#997
ferhatelmas merged 1 commit intomasterfrom
ferhat/abort-controller

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

@ferhatelmas ferhatelmas commented Apr 9, 2026

What kind of change does this PR introduce?

Refactor

What is the current behavior?

Using any.

What is the new behavior?

No any. Event is also passed to listener as expected.

Additional context

It's more generic to handle event listener object as well and add new coverage for it.
Handle removing by capturing identity in weak map.

Related to #922, extracted for behavioral changes.

@ferhatelmas ferhatelmas requested a review from a team as a code owner April 9, 2026 15:58
Copilot AI review requested due to automatic review settings April 9, 2026 15:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the internal AsyncAbortController to eliminate any usage and to properly support both function listeners and EventListenerObject listeners while tracking abort-handler completion.

Changes:

  • Replaces Promise<any>[] with Promise<unknown>[] for stronger typing.
  • Binds and wraps AbortSignal.addEventListener to support EventListenerOrEventListenerObject and forward the Event argument.
  • Awaits async abort handlers by resolving a tracked promise per registered abort listener.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/internal/concurrency/async-abort-controller.ts
Comment thread src/internal/concurrency/async-abort-controller.ts Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 9, 2026

Coverage Report for CI Build 24392407877

Coverage increased (+0.04%) to 81.028%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 18 uncovered changes across 1 file (167 of 185 lines covered, 90.27%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/internal/concurrency/async-abort-controller.ts 185 167 90.27%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 37842
Covered Lines: 30835
Line Coverage: 81.48%
Relevant Branches: 4284
Covered Branches: 3299
Branch Coverage: 77.01%
Branches in Coverage %: Yes
Coverage Strength: 317.4 hits per line

💛 - Coveralls

@ferhatelmas ferhatelmas force-pushed the ferhat/abort-controller branch 2 times, most recently from 4f3cea2 to 2ea7e4e Compare April 9, 2026 16:45
@ferhatelmas ferhatelmas requested a review from Copilot April 9, 2026 16:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ferhatelmas ferhatelmas force-pushed the ferhat/abort-controller branch 3 times, most recently from b3b4f4a to 8eb1bbc Compare April 13, 2026 14:49
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/abort-controller branch from 8eb1bbc to 1696665 Compare April 14, 2026 09:48
@ferhatelmas ferhatelmas merged commit 6202eb8 into master Apr 14, 2026
5 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/abort-controller branch April 14, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants