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

Should we add writableStream.write()? readableStream.read()? #1072

Open
domenic opened this issue Aug 26, 2020 · 4 comments
Open

Should we add writableStream.write()? readableStream.read()? #1072

domenic opened this issue Aug 26, 2020 · 4 comments

Comments

@domenic
Copy link
Member

domenic commented Aug 26, 2020

I was reviewing https://wicg.github.io/native-file-system/#api-filesystemwritablefilestream, which is one of the the major non-TransformStream users of WritableStream in spec land so far. And I noticed they added a write() convenience method that just wraps this.getWriter().write() + a release step, similar to the existing writableStream.close() and writableStream.abort().

It feels like maybe we're doing something wrong, if specs are subclassing us to add convenience methods in this way. Should we consider adding one-shot writing methods? What about one-shot reading methods?

It would be a nice simplification for folks who are just writing simple code and don't need to care about the exclusivity of their writers/readers...

On the other hand, maybe the real issue here is that it's too hard to add convenience methods to the return value of getWriter()? Maybe if that were easier, the NativeFS spec would have stuck with the "acquire a writer first" design, and just added the seek() and truncate() convenience methods to the writer?

/cc @mkruisselbrink

@ricea
Copy link
Collaborator

ricea commented Aug 26, 2020

My impression is that the write() method is more focussed on providing features that are not available in in the WritableStreamDefaultWriter write() method, rather than convenience.

@domenic
Copy link
Member Author

domenic commented Aug 26, 2020

That was my initial impression, but it appears to not be the case: https://wicg.github.io/native-file-system/#dom-filesystemwritablefilestream-write .

@mkruisselbrink
Copy link

Heh, yeah. I think earlier on we had some thoughts that the via the WritableStreamDefaultWriter should only accept bytes, and we'd provide the Stream.write as a convenience to support blobs and strings as well. But then to get seek/truncate working the underlying sink would have to accept something more complicated anyway. So yeah, now it is just a wrapper around creating a writer.

@domenic
Copy link
Member Author

domenic commented Aug 26, 2020

An alternative would be to go the other way around, and have stream.write() only accept blobs/strings/bytes (and not accept "command" chunks, for which you should use stream.truncate() and stream.close().) But I'm not sure pushing in that direction would be sound; it feels more like I'm trying to justify the separation, versus actually making the API better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants