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

Define writable readyPromise handling #108

Merged
merged 2 commits into from
Aug 26, 2021

Conversation

youennf
Copy link
Collaborator

@youennf youennf commented May 27, 2021

Since this is observable, we need to define when ready promise resolves or not.
By default, follow what implementations currently do and consider chunks have a size equal to zero.


Preview | Diff

@alvestrand
Copy link
Contributor

Can you explain what the practical consequences are of having a sizeAlgorithm that returns zero?
I think (having tried to navigate the streams spec) that it will result in a writable stream that is unable to perform backpressure. If this is the property that we desire, we should probably say so explicitly.

@youennf
Copy link
Collaborator Author

youennf commented Aug 19, 2021

I think (having tried to navigate the streams spec) that it will result in a writable stream that is unable to perform backpressure.

Right, we could indeed say so, the writable enqueue operation returns a resolved promise which already achieves this somehow.
The observable consequence of setting a chunk size of 0 is that the writer ready promise will never change (and will always be resolved).

@alvestrand
Copy link
Contributor

Resolution: Add a note after step 7 of the setup about why we're doing 0 and the consequences thereof.

@alvestrand alvestrand merged commit 914eed4 into w3c:main Aug 26, 2021
github-actions bot added a commit that referenced this pull request Aug 26, 2021
SHA: 914eed4
Reason: push, by @alvestrand

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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

2 participants