-
Notifications
You must be signed in to change notification settings - Fork 66
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
Why not use ReadableStream/WritableStream/WritableStream*Writer #19
Comments
For reading, see #2 and w3c/FileAPI#40; i.e. I'm not against adding something to File/Blob to make it easier to use a stream to read from a file, but it's already pretty easy to create a ReadableStream out of a Blob (and no, that doesn't require using slice). For writing, this can't just use WritableStream at least, as that wouldn't support seeking/random access. But perhaps it would indeed make sense to optionally let you treat a file reference as a WritableStream. Not quite sure what that would look like. Also the explainer currently (somewhat on purpose) completely glosses over things like locking files, if multiple writers should be able to write to the same file at the same time, etc. |
Yes, a WritableStream would lock by default. At least the FileWriter could attempt to use the same method names for the same things. Would it be possible to inherit from a WritableStream instead and add the needed functionality (seek?) |
Thinking about this a bit more, being able to pass in a ReadableStream to write(), like the current explainer mentions, also means you can use an identity transform stream to get a WritableStream to then write to the file (i.e. like how https://streams.spec.whatwg.org/#example-transform-identity describes the same for providing a request body). So I'm really not sure about the need for more API surface here... |
I'd like to bump this a bit. The current shape of the spec is pretty nice: you get a What if instead you had a Then the code would look something like const stream = await fileHandle.createWritable();
// All I want to do is truncate:
await stream.truncate(1000);
// Or I want to do more complicated stuff:
const writer = stream.getWriter();
writer.seek(10);
writer.write(uint8Array);
writer.truncate(100);
// Or I want to compose with readablestream:
await readable.pipeTo(stream); What do you think? |
hmm, yeah... that does seem potentially nice. The main reason I hadn't done anything like that was that I wasn't sure if/how actually deriving from WritableStream and or WritableStreamDefaultWriter would work, both in spec and implementation. You don't think it would be weird to have thinks like seek and truncate on a writable stream like object? With such an API would it be crazy for write to not just accept uint8arrays, but also accept strings (that are utf-8 encoded) and (more importantly) blobs as well? cc @oyiptong |
In spec, we've been hand-waving a bit, treating them as IDL types. After whatwg/streams#963 it should be on solid ground, but until then, I think the semantics are relatively clear, if you do something like In the Blink implementation, similarly, this will be much easier with @ricea and @yhirano's work to move to IDL-based types.
Nah. Having specialized subtypes that give you higher-level or resource-specific operations has always been part of the plan. Not one we've stressed yet, so there might be some kinks, but conceptually it hangs together.
We've tried to keep streams to be single-type-chunked so far. Especially for string vs. Uint8Array, we've encouraged people to pipe through a TextEncoderStream or similar. This isn't enforced by the API; it's just a convention so far. So you could allow them to accept more, and do the conversion for your users, but it makes me a bit uncomfortable... Let's compare with the alternative, of a Uint8Array-only version: const encoder = new TextEncoderStream();
encoder.readable.pipeTo(writableFileStream);
const writableFileStreamAcceptingStrings = encoder.writable;
// Now use writableFileStreamAcceptingStrings as usual, e.g.
writableFileStreamAcceptingStrings.getWriter().write("a string"); blob.stream().pipeTo(writableFileStream); Seems not so bad, but I guess not as nice. Hmm. I'm unsure. |
Had a chat with @mkruisselbrink, he suggested instead to have the The reasoning is to have convenience methods be easily accessible. Conceptually, if one wants more control, they'd obtain the I.e. something akin to: const stream = await fileHandle.createWritable();
// Just write something.
await stream.write(0, "foo");
// Need more control now.
const writer = stream.getWriter();
writer.seek(4);
await writer.write(new Uint8Array([0x21, 0x21]));
... |
Ooh, I like that a lot! Riffing off that further, I wonder if you could even move the seeking before the getWriter() call, so that way you can keep using WritableStreamDefaultWriter. Then you have a nice clean separation of high-level manipulation/preparation APIs on the stream, and lower-level simple-sequential-stream operations once you grab the writer. |
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623534 Reviewed-by: Marijn Kruisselbrink <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Jeremy Roman <[email protected]> Auto-Submit: Olivier Yiptong <[email protected]> Cr-Commit-Position: refs/heads/master@{#662620}
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623534 Reviewed-by: Marijn Kruisselbrink <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Jeremy Roman <[email protected]> Auto-Submit: Olivier Yiptong <[email protected]> Cr-Commit-Position: refs/heads/master@{#662620}
…m API, a=testonly Automatic update from web-platform-tests [Native File System] Writable File Stream API This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623534 Reviewed-by: Marijn Kruisselbrink <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Jeremy Roman <[email protected]> Auto-Submit: Olivier Yiptong <[email protected]> Cr-Commit-Position: refs/heads/master@{#662620} -- wp5At-commits: 2263ea8f0429928725fb287e2ffd3d4f8bcc279e wpt-pr: 16945
…m API, a=testonly Automatic update from web-platform-tests [Native File System] Writable File Stream API This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623534 Reviewed-by: Marijn Kruisselbrink <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Jeremy Roman <[email protected]> Auto-Submit: Olivier Yiptong <[email protected]> Cr-Commit-Position: refs/heads/master@{#662620} -- wp5At-commits: 2263ea8f0429928725fb287e2ffd3d4f8bcc279e wpt-pr: 16945
This change adds a writable stream output to the `FileSystemFileHandle`. This follows some [discussion](WICG/file-system-access#19 (comment)) on github. While this writable stream does not yet feature stream-like behavior, it carries over some of the interface from `NativeFileSystemWriter`, namely `truncate` and `write`. Bug: 955189 Change-Id: I8b8c9ff1ef36adfb30501251563699c6c7973889 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623534 Reviewed-by: Marijn Kruisselbrink <[email protected]> Reviewed-by: Jeremy Roman <[email protected]> Commit-Queue: Jeremy Roman <[email protected]> Auto-Submit: Olivier Yiptong <[email protected]> Cr-Commit-Position: refs/heads/master@{#662620}
Ae you moving ahead with this new API suggestion? |
Yes, the plan is to iterate on this API suggestion. While we originally started implementation on this API shape, as we got deeper into implementing our security model, it turns out we might need to revise this API a bit. For instance, for the non-stream writer, we have a well understood way to protect users from harm (#37) and at the same time enabling atomic writes (#48), for the use-case where developers do not want incomplete writes. This has materialized in the spec in #51. This API suggestion as is currently does not have an explicit As for the streams implementation, the next step is to figure out a way to protect users and at the same time have a decent API shape. |
To clarify a bit on this, I don't think the concerns around safe browsing like behavior particularly impact the design of a streams based writing API. Writable Streams do support a "close" operation, so it should be no problem to match the current behavior with an API that exposes writing using writable streams (modulo fixing whatwg/streams#1007, as it would be a bit awkward to have to create a writer from the stream just to be able to close the stream). We just put figuring out the best way to have a streams based API a bit on the backburner, because the streams implementation in Blink wasn't ready to support other classes deriving from WritableStream. So yes, we definitely plan to pursue this, I don't see any major blockers, we just haven't prioritized working on the spec side of this since we were blocked on the implementation side of this. |
Perhaps you can nudge the Blink team to progress on |
Oh, progress is certainly being made. The streams implementation just has been going through a rewrite that made building other native features on top of it harder, but that is rolling out/should be rolling out soon, I believe. |
@kenchris, I'm compiling some public documentation about this API change and I was wondering if you had a specific use-case in mind to use streams. The pwrite-style API provides all capabilities necessary, and streams enables a higher level API with access to stream semantics. However, in view of making a good case for this change in my document, perhaps you could share TAG's perspective on using Streams semantics for the writer? |
The API currently does use WritableStream for writing. Closing this, but open to feedback for ways we should change the API. |
Instead of the FileReader and FileWriter.
It could potentially be specialized versions, or at least the new objects could share API
If there are obvious reasons why these cannot be used, I think the explainer should say so.
Of course FileReader already exists, but FileWriter doesn't to my knowledge, so it would be good to figure out what is the best API going forward.
Currently you have
createWriter
andWritableStream
hasgetWriter
returning aWritableStreamDefaultWriter
:https://streams.spec.whatwg.org/#default-writer-class
The FileReader also doesn't support streaming of content, unless you read manually by using
.slice()
The text was updated successfully, but these errors were encountered: