Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement AbortSignal.protototype.onabort #1848

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Mar 15, 2024

The spec defines onabort as a prototype property, implement it as such.

Technically speaking, this could be considered a breaking change :-/ ... I'm hoping we can get away with it without requiring a compat flag. It's a potentially breaking change because prior to this, the following operations would have different results:

  • 'onabort' in AbortSignal.prototype ... previously, false, now true
  • delete signal.onabort ... previously deletes the own property if set, now has no change
  • Object.keys(signal) ... previously would include onabort if it was set, now always excludes it

One thing this PR doesn't do (that we can do later in a separate PR) is define the type mapping override for onabort.

@jasnell jasnell requested a review from kentonv March 20, 2024 21:04
@jasnell jasnell force-pushed the jsnell/ew-8235-abortsignal-onabort branch 2 times, most recently from 8c81fee to bce8e35 Compare March 22, 2024 17:50
The spec defines `onabort` as a prototype property, implement
it as such
@jasnell jasnell force-pushed the jsnell/ew-8235-abortsignal-onabort branch from f94708e to 0500904 Compare April 4, 2024 15:37
@jasnell
Copy link
Member Author

jasnell commented Apr 4, 2024

I'm not going to do it in this PR, but I think because this pattern is common to all whatwg EventTarget types, it would make sense to wrap this in a JSG helper macro, like JSG_EVENT(abort) that wraps the boilerplate.

@jasnell jasnell merged commit 9b5650b into main Apr 4, 2024
10 checks passed
@jasnell jasnell deleted the jsnell/ew-8235-abortsignal-onabort branch April 4, 2024 16: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.

None yet

3 participants