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

Create a dedicated section for interfacing with other specs #1073

Merged
merged 9 commits into from
Sep 14, 2020

Conversation

domenic
Copy link
Member

@domenic domenic commented Aug 26, 2020

This incorporates much of https://fetch.spec.whatwg.org/#streams, as well as https://wicg.github.io/compression/#generic-transform-stream. It also looks at how https://wicg.github.io/native-file-system/ uses writable streams, and creates wrapper algorithms to make that easier.

Closes #372.

This is probably ready for review, but we shouldn't merge it until I have created update pull requests for at least:

and have validated that this suffices for their purposes.

Also, I'll be opening another issue about GenericTransformStream.


Preview | Diff

@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label Aug 26, 2020
@domenic
Copy link
Member Author

domenic commented Aug 26, 2020

This should also have a section on duplex streams like https://reillyeon.github.io/serial/#serialport-interface. I don't think they need any new algorithms, but stating things like "use the names readable and writable" and explaining how you can use the create algorithms is probably a good idea.

@wolenetz
Copy link

Note that w3c/media-source#14 may eventually also be resolved such that Media Source Extensions becomes another interface point for the Streams API.

@ricea
Copy link
Collaborator

ricea commented Aug 26, 2020

I've had an initial read-through and it looks good.

@ricea
Copy link
Collaborator

ricea commented Aug 26, 2020

We will also need to integrate with https://wicg.github.io/web-transport/ but that should be straightforward.

domenic added a commit to whatwg/fetch that referenced this pull request Aug 27, 2020
Mostly adding a lot of "-ing" linking text variants
domenic added a commit to domenic/ServiceWorker that referenced this pull request Aug 27, 2020
domenic added a commit to whatwg/encoding that referenced this pull request Aug 27, 2020
domenic added a commit to domenic/compression that referenced this pull request Aug 27, 2020
Follows whatwg/streams#1073.

Also includes minor editorial touchups to the algorithms as I was editing them.
domenic added a commit to domenic/native-file-system that referenced this pull request Aug 27, 2020
@domenic
Copy link
Member Author

domenic commented Aug 27, 2020

OK, PRs sent to all of the above; I updated the OP with them.

I got all ready to send one for WebTransport, but they don't appear to be currently at a level of rigor where this makes sense; they just talk about "Set X to a ReadableStream of Uint8Arrays representing (some abstract concept), currently empty". I guess the assumption is that all the enqueuing/closing/erroring is encapsulated in that "represents", hmm.

@ricea
Copy link
Collaborator

ricea commented Aug 27, 2020

I got all ready to send one for WebTransport, but they don't appear to be currently at a level of rigor where this makes sense; they just talk about "Set X to a ReadableStream of Uint8Arrays representing (some abstract concept), currently empty". I guess the assumption is that all the enqueuing/closing/erroring is encapsulated in that "represents", hmm.

No problem, we have to increase the level of rigor anyway, so we can adopt the new interfaces at the same time. Sorry for wasting your time.

annevk pushed a commit to whatwg/encoding that referenced this pull request Sep 8, 2020
Copy link
Collaborator

@ricea ricea left a comment

Choose a reason for hiding this comment

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

Apart from my concern about read-loop, this lgtm.

Sorry for the delayed review.

1. If |chunk| is not a {{Uint8Array}} object, [=reject=] |promise| with a {{TypeError}} and
abort these steps.
1. Append the bytes represented by |chunk| to |bytes|.
1. [=Read-loop=] given |reader|, |bytes|, and |promise|.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can call ReadableStreamDefaultReaderRead re-entrantly, which is a bit scary. As far as I can tell it's safe in the standard language, but implementations can fail due to stack overflow. Easiest fix would probably be to post a microtask.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Given that there's already a note explaining that this might be implemented rather differently, I think it'd be best to make a note of the danger, and encourage implementations to mitigate it in a way that doesn't require "read all bytes" to delay for (number of chunks) microtasks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I guess such a requirement wouldn't be observable either, so scratch that part of my comment. But still.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ricea ricea left a comment

Choose a reason for hiding this comment

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

lgtm

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Sep 14, 2020
@domenic domenic merged commit dadfc12 into master Sep 14, 2020
@domenic domenic deleted the other-specs branch September 14, 2020 23:37
annevk added a commit to whatwg/encoding that referenced this pull request Sep 15, 2020
Follows whatwg/streams#1073.

Co-authored-by: Andreu Botella <abb@randomunok.com>
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
annevk pushed a commit to whatwg/fetch that referenced this pull request Sep 15, 2020
jakearchibald added a commit to w3c/ServiceWorker that referenced this pull request Sep 15, 2020
* Use new Streams algorithms

Follows whatwg/streams#1073.

* Namespacing tweaks

* No subsubsteps

Co-authored-by: Jake Archibald <jaffathecake@gmail.com>

* Missing ::

Co-authored-by: Jake Archibald <jaffathecake@gmail.com>
ricea pushed a commit to whatwg/compression that referenced this pull request Sep 15, 2020
Follows whatwg/streams#1073.

Also includes minor editorial touchups to the algorithms as I was editing them.

Also improves the Makefile.
mkruisselbrink pushed a commit to WICG/file-system-access that referenced this pull request Sep 15, 2020
domenic added a commit to whatwg/xhr that referenced this pull request Sep 29, 2020
annevk pushed a commit to whatwg/xhr that referenced this pull request Sep 30, 2020
Bishwarupjee pushed a commit to Bishwarupjee/xhr that referenced this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Separate out exported abstract ops from internal ones
4 participants