Add a mid-level file-rfc

[rylev]

rendered

As we've settled on an RFC process, I'd like to continue #50 here.

[fitzgen]

LGTM!

I'm OK with the BlobLike trait because while we went the other way with web-sys, I'm not convinced that all the same arguments made there apply directly here. For example, with web-sys we are exactly exposing the JS API as close as we can. With Gloo, we are actively trying to make things more Rust-y.

[rylev]

I'll wait for @Pauan's feedback and approval before submitting an implementation PR.

[yoshuawuyts]

This feels solid for a mid-level API!

One question tho: I'm wondering tho if having Futures be part of the mid-level API makes the most sense, or if that would actually be better suited as the end-user interface we expose in the high-level API.

[rylev]

@yoshuawuyts Having futures in the same API feels right to me, because It still maps very closely to the JS API.

I don't have strong feelings about this though so if the group wants to be extra conservative and only do the callback based API at first I'm fine with that. We could later open a new proposal that would extend the mid-level API with futures OR create a new high level API with futures.

[yoshuawuyts]

We could later open a new proposal that would extend the mid-level API with futures OR create a new high level API with futures.

Yeah, I'm definitely feeling like having a high-level API on top of this would be a great fit. For example slice could be abstracted through Index / IndexMut to make it similar to Rust slices.

Also IntoIterator, Add and other might have a place. I feel Futures provide a similar high-level interface on top of the mid-level API.

But yeah, don't feel too strongly about this. Just curious how we're envisioning we slice this up (:

[Pauan]

Is there a reason this can't internally be a Vec<File>, and then impl Deref<Target = [File]>?

Even the current editor's draft says that uses of FileList are getting replaced with Array (which would get translated into Vec in Rust).

[rylev]

This would require iterating the internal js_sys::FileList and collecting the files into a Vec. Are we sure we want the user to pay this price each time a FileList is constructed? I personally think an into_vec API is more appropriate since it conveys that there will be an additional allocation.

In any effect, it's possible to add these in a backward compatible way. Can we push this off to another PR/RFC?

[Pauan]

I don't think it's possible to add it in a backwards compatible way, since a Vec would have a different API.

And since (apparently) FileList is being phased out in favor of a normal JS Array, I think Vec is quite appropriate.

In fact, we may even want to skip the FileList type entirely and just return Vec<File>.

That also makes it pretty clear that there is an allocation (since Vec obviously allocates).

[rylev]

@Pauan Can you explain how the API would need to be different? I'm sorry, I don't really get the reason for this given the cost users would have to pay.

[Pauan]

Basically, rather than having a method which returns a FileList, it would instead return Vec<File>.

That means we don't actually need FileList at all.

The cost is quite small, but the benefits are significant: things are drastically simplified, and users get to use the slice methods for free.

[Pauan]

(By the way, sorry for the delay on this, I recently caught a nasty cold)

[rylev]

@Pauan @fitzgen I've made some final changes including removing the builders for File and Blob since the constructors are now now that unwieldy.

I'd like to merge this. I know there's one concern not fully addressed from @Pauan, but I think the proposal is in a good state and ready to be implemented. Thoughts?

[fitzgen]

I'm happy with it. Let's address the last outstanding concern and get this merged!

[rylev]

@Pauan @fitzgen shall we merge this?

[fitzgen]

@Pauan @fitzgen shall we merge this?

I'm 👍, let's just wait for @Pauan's response before merging.

[Pauan]

I would really like us to explore Vec<File> instead of FileList, but I don't want to block on it, since everything else seems good.

[rylev]

@Pauan I'm not convinced. If the user wants a Vec<File> they can call iter().collect::<Vec<File>>(). If we think that's too much, we can always include a to_vec() method that does that for the user. I think the proposal as is does a good job of walking the line between feeling like Rust and still mirroring the JS API fairly closely. Perhaps we'll want to have a higher level API that goes even further?

[Pauan]

Yes of course the user can do that... but the point is to simplify things. Not needing to have the FileList type (or any of the various methods), being able to use slice methods for free. For the user it's great!

It's less things to learn, less things to worry about. It's also extremely Rusty (since Vec is one of the core Rust data types).

And as the W3C page says, they're deprecating FileList and moving toward a JS Array (which maps directly to Vec in Rust). So using Vec avoids any future compatibility issues.

So I think you need to do a better job of justifying why we need FileList: what does it buy us? What do we gain from it?

[rylev]

@Pauan Do you propose just doing impl From<web_sys::FileList> for Vec<File> for now? I'd be interested in @fitzgen opinion here. Perhaps he can break the deadlock?

[Pauan]

Yes, that's basically my proposal. Though we haven't yet decided on how this will interact with <input> (since that's a separate proposal).

[fitzgen]

gloo_files::FileList

Pros

• Could be made zero cost: if we add a 'a lifetime parameter so it is gloo_files::FileList<'a> and make it wrap a anyref stack-allocated &'a web_sys::FileList, then there are no heap allocations involved. Vec<gloo_files::File> requires heap allocations.

Cons

• Additional type to learn in the API.

• Spec is moving away from web_sys::FileList and moving towards JS array. To get the no-implicit-heap-allocation version in this world, gloo_files::FileList<'a> would have to wrap a &'a js_sys::Array.

Vec<gloo_files::File>

Pros

• Fewer types in the API to learn.

• Conceptually matches where the spec is heading.

Cons

• Requires heap allocation.

Conclusions

In the case of a single file in the file list, it is nice to avoid the Vec's heap allocation. But it also doesn't seem like a hard requirement in this case, since it is just a single heap allocation that shouldn't ever be part of a loop or anything like that. It seems to me like this particular allocation doesn't really matter.

Therefore I think we should go with Vec<gloo_files::File> and not gloo_files::FileList.

But mostly I think we would be just fine with either design, it isn't a big deal, and we are bike shedding at this point. Its been 5 weeks since this API was first proposed, and while it has certainly improved from the initial proposal, at this point I think we gain more by just merging than by deliberating further.

[rylev]

@Pauan @fitzgen updated. Let's merge this.