Files API Implementation

[rylev]

Aims to implement #50

This is still work-in-progress and the discussion on the API proposal is not complete, but I thought the conversation could be enriched through some code.

[fitzgen]

Thanks for sharing this @rylev! Left a bunch of notes below -- also don't forget to go through the checklist :)

[rylev]

So we're in a good minimal place. The following things are left to implement and/or discuss:

• The other FileReader methods: .readAsDataURL(), readAsBinaryString(), and readAsArrayBuffer()
  • Should dataUrl and binaryString be strings? Should gloo have a higher level ArrayBuffer object or should we copy the file into the wasm heap and return a Vec to the user? What about other integer sizes?
• Add cancelation support to ReadAsString future.
• Fine grained support of different events on the FileReader (e.g., loadstart and progress)
  • Should these only be exposed as a high level stream API. What about callbacks?
• Add file metadata (e.g., name) getters
• Add a slice interface on BlobLike things
• Constructors for Blob and File (both for data on the wasm heap and on the JS heap)

[samcday]

Should gloo have a higher level ArrayBuffer object or should we copy the file into the wasm heap and return a Vec to the user?

It should be sufficient to just provide the Uint8Array, right? The API web_sys provides for it is already rich enough. If the caller wants to copy that into Rust land, it's a mere Uint8Array::copy_to away.

[fitzgen]

readAsBinaryString seems deprecated, its functionality is subsumed by readAsArrayBuffer, and it only exists for backwards compatibility -- I say we ignore it.

readAsArrayBuffer -- I don't think we want to force a copy into wasm memory. If for whatever reason the user is just going to pass it back to some other JS method, then that copy would be a big waste. The lowest common denominator API is returning the js_sys::ArrayBuffer directly. On top of that, we could provide convenience methods to do the typed-array copy_to dance for different integer sizes and signednesses. Would require a little care so that we allowed folks to bring their own buffers rather than forcing new Vec allocations.

* Add cancelation support to `ReadAsString` future.

I haven't looked at the updated code yet, but this should be automatic on drop.

Fine grained support of different events on the FileReader (e.g., loadstart and progress)

* Should these only be exposed as a high level stream API. What about callbacks?

I think we can:

• provide a constructor or builder that takes optional callbacks for these events (using set_onprogress rather than gloo_events)

• allow folks to add arbitrary listeners to the underlying event target

• in a backwards-compatible way, add methods for getting the progress events as a Stream (this can be delayed to a new PR; we probably want to add a generic event listener -> stream conversion infrastructure to build this on top of)

[Pauan]

provide a constructor or builder that takes optional callbacks for these events (using set_onprogress rather than gloo_events)

Wouldn't they have to manually handle all the Closure related stuff? Why not just use gloo-events?

[fitzgen]

Wouldn't they have to manually handle all the Closure related stuff? Why not just use gloo-events?

I was imagining that there would be onprogress: Option<Closure<FnMut()>> inside FileReader (rather than a Vec<EventListener>)

But yeah I'm not super happy about it, and all the alternatives seem to have different downsides.

Alternatives for progress would be:

• Only exposing the event target for users to manually add callbacks. Now they have to manage the EventListener separately from the FileReader. Maybe this is OK.
• Not exposing callbacks at all, and just the streams of events. Goes against our layering principles, but maybe is worth it in certain cases. Does mean that folks must use streams to get progress events.
• Storing a Vec<EventListener> inside FileReader. These would probably be add-only since (as I believe you mentioned in other threads) there is no good way to remove a specific one without introducing a whole new RAII type or Rc or something.

[Pauan]

I was imagining that there would be onprogress: Option<Closure<FnMut()>> inside FileReader (rather than a Vec<EventListener>)

Why can't it be onprogress: EventListener or onprogress: Option<EventListener>? There's no need for Vec if you know ahead of time what events you want.

The reason I mentioned Vec<EventListener> in the other thread was because WebSockets actually wanted the ability to dynamically add/remove arbitrary events (not just a hardcoded list).

[fitzgen]

Oh yeah totally. That's what we should do. <brain-fart>

[rylev]

I've made good progress on the todos above. I need to wait for the Events PR to be merged to finish this out though.

@fitzgen I want to provide a slice interface for BlobLike things. The JavaScript interface uses signed integers to indicate start and end indices (e.g., -10 for the end value is 10 away from the end). This convention isn't too common in Rust. Do you think we should stick with the JavaScript convention or do something more straightforward (signed integers that just specify beginning and end indices?

[fitzgen]

We should move towards Rust idioms, not JavaScript idioms. Therefore, I do not think we should do the signed-index-with-negative-indexing-from-the-end thing.

[rylev]

@fitzgen @Pauan I'm going to close this for now until we close on the API proposal. Once that's decided I can open this up again.