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.

Sign up for GitHub

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

Editorial: account for removal of AbortSignal's follow #1277

Merged
merged 2 commits into from
May 8, 2023
Merged

Editorial: account for removal of AbortSignal's follow #1277

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
539532a
Update fetch spec for AbortSignal.any() changes
shaseley May 1, 2023
d4a04ff
Use 'must not' for signal abort warning
shaseley May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6999,9 +6999,8 @@ above [=WritableStream/set up=] algorithm:
|stream|.[=WritableStream/[[controller]]=].[=WritableStreamDefaultController/[[signal]]=].
Specifications can [=AbortSignal/add=] or [=AbortSignal/remove=] algorithms to this
{{AbortSignal}}, or consult whether it is [=AbortSignal/aborted=] and its [=AbortSignal/abort
reason=]. Specifications should not [=AbortSignal/signal abort=] or make it [=AbortSignal/follow=]
another {{AbortSignal}}, as that would interfere with the normal use of this signal to respond to
the stream being [=abort a writable stream|aborted=].
reason=]. Specifications should not [=AbortSignal/signal abort=], as that would interfere with the
normal use of this signal to respond to the stream being [=abort a writable stream|aborted=].
Copy link
Member

Choose a reason for hiding this comment

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

Follow-up nit: must not? Or what would a good reason be?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, "must not" would be better. Only the Streams standard itself should signal abort.

However, since whatwg/dom#1152 will make "to signal abort on an AbortSignal" private, it will no longer be possible for other specifications to affect this signal. So we can hopefully get rid of this entire footnote. 😁

By the way, should we already refactor Streams to use "to signal abort on an AbortController" instead? Or should we wait until whatwg/dom#1152 has landed?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I had missed that it no longer exports that definition.

@shaseley if that was intentional this would have to be a larger PR and other specifications would need updating as well: https://dontcallmedom.github.io/webdex/s.html#signal%20abort%40%40AbortSignal%40dfn.

@MattiasBuelens no need to do anything just yet. Perhaps we should scope down 1152 and leave that as a follow-up.

Copy link
Contributor Author

@shaseley shaseley May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

+1 to scoping down 1152 and making 'signal abort' private in a follow-up. I'll fix 1152, update the language to 'must not', and work on the follow-up after 1152 lands. Sorry about the confusion!


<p class="note">The usual usage is, after [=WritableStream/setting up=] the {{WritableStream}},
[=AbortSignal/add=] an algorithm to its [=WritableStream/signal=], which aborts any ongoing write
Expand Down