-
Notifications
You must be signed in to change notification settings - Fork 165
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
Early draft of WritableStream "Design Philosophy" section #718
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2568,6 +2568,48 @@ nothrow>ReadableByteStreamControllerShouldCallPull ( <var>controller</var> )</h4 | |
writeRandomBytesForever(myWritableStream).catch(e => console.error("Something broke", e)); | ||
</code></pre> | ||
</div> | ||
|
||
<h3 id="ws-design-philosopy">Design Philosophy</h3> | ||
|
||
<div>While sharing the principles for streams in general, a number of additional principles have informed the design of | ||
the WritableStream class.</div> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we can make clearer the "scope" of this section. It's got great info but it's of a very specific kind. It's not about the philosophy behind writable streams in general (of the type of stuff covered by the "Model" section). It's more about the details of its API surface and the interactions between the sink, controller, and writer APIs. Not sure on the exact phrasing, but that might be a start. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the name to "Design Of The State Machine" which may or may not be helpful. |
||
|
||
<ul> | ||
<li><p>Only one sink method can ever be executing at a time. | ||
<li><p>Sink methods are treated as atomic. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this can be one paragraph instead of sub-bullets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I use paragraphs for everything instead of sub-bullets? I quite like the sub-bullets to be honest. |
||
<ul> | ||
<li><p>"Atomic" here means that the time between a sink method being called and the returned Promise resolving or | ||
rejecting is treated as indivisible. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't really clarify. What would divide them? I think the next bullet points actually state what atomic means. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just took this out. |
||
<li><p>A new sink method will never be called until the Promise from the previous one has resolved. | ||
<li><p>State changes do not take effect until any in-flight sink method has completed. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "State changes" isn't too clear here, especially given the next two bullet points worth of exceptions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded a bit. |
||
</ul> | ||
<li><p>Exception: If something has happened that will error the stream, for example writer.abort() has been called, then | ||
new calls to writer.write() will start failing immediately. There's no user benefit in waiting for the current operation | ||
to complete before informing the user that writer.write() has failed. | ||
<li><p>The writer.ready promise and the value of writer.desiredSize reflect whether a write() performed right | ||
now would be effective. | ||
<ul> | ||
<li>writer.ready and writer.desiredSize will change even in the middle of executing a sink method, as soon as we | ||
know that calling writer.write() won't work any more. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They also change in cases besides "won't work" though There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded a bit. |
||
<div class=note>Because promises are dispatched asynchronously, the state can still change between | ||
writer.ready becoming fulfilled and write() being called.</div> | ||
<li>The value of writer.desiredSize decreases synchronously with every call to writer.write(). This implies that | ||
strategy.size() is executed synchronously. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given how we're not calling strategy.size as a method, I'd state this as "the queuing strategy's size() function is executed..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
</ul> | ||
<li><p>The writer.closed promise and the promises returned by writer.close() and writer.abort() do not resolve or | ||
reject until no sink methods are executing and no further sink methods will be executing. | ||
<ul> | ||
<li>If the user of the WritableStream wants to retry against the same underlying data item, it is important to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "data item" = chunk? "Retry against" is a bit confusing to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to clarify by using the example of a file, rather than try to come up with a general term for "named or otherwise identifiable entities which can be retried independently of the lifetime of any particular stream". |
||
have confidence that all other operations have ceased. | ||
<li>This principle also applies to the ReadableStream pipeTo() method. | ||
</ul> | ||
<li><p>Promises resolve or reject in consistent order. In particular, writer.ready always resolves before | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think "fulfill" is more informative here (although both are technically accurate) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
writer.closed, even in cases where they both resolve "at the same time". | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd state this as "where both are fulfilling in reaction to the same occurrence" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great! Thanks. |
||
</ul> | ||
|
||
<div>Some of these design decisions improve predictability, ease-of-use and safety for developers at the expense of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oxford comma please (after "ease-of-use") I know it's not markup-critiquing time yet but this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could move up to merge with the intro actually. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved it up and made it a paragraph. |
||
making implementations more complex.</div> | ||
|
||
<h3 id="ws-class" interface lt="WritableStream">Class <code>WritableStream</code></h3> | ||
|
||
<h4 id="ws-class-definition">Class Definition</h4> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Needs to be marked non-normative)