miguelpruivo / flutter_file_picker

File picker plugin for Flutter, compatible with mobile (iOS & Android), Web, Desktop (Mac, Linux, Windows) platforms with Flutter Go support.
MIT License
1.33k stars 664 forks source link

Package is very hard to use, nullability semantics in particular #1469

Open matanlurey opened 7 months ago

matanlurey commented 7 months ago

I want to mention I think this package is awesome, and saves me tons of work on hobby projects. Kudos.

That being said, the API state of the project leaves a lot to be desired. Some suggestions:

I have more suggestions, but these are the two that hit me in every project I start with this package.

miguelpruivo commented 6 months ago

Hi, thank you for the suggestion but you are actually saving just 1 loc with that change vs. the effort that has to be put in order to refactor the API at this point, specially having support for all the platforms. The pickFiles supports multiple properties that are quite verbose and somewhat easy to understand, specially because they are very well documented and with examples on the Wiki.

The way the API was made was to make it as much as seamless experience between the different platforms and not only on mobile, so the devs don't have to call different API's for each like:

if(kIsWeb){
// Pick in a way
} else if(Platform.isIOS){
// Use another
}

Specially when multiple devs support multiple platforms at once.

As for your last points asking for the documentation, I'm not sure if you checked the Wiki where you have the explanation with examples for most of the parameters, as well as the PlatformFile itself where all the properties have docs (regardless of some of them being self-explanatory).

Don't take this the wrong way, but trust me, in the early days of the project I took a lot of time thinking and re-thinking the best way this could be done to seamless support multiple platforms with the less struggle for the dev. It might not be perfect but I think it's solid enough and it works following the Flutter and Dart principles.

In addition, the presence of bytes and readStream is really confusing - the latter makes it seem like I can read from the file >asynchronously, but the former synchronously (and perhaps it's even pre fetched, because it's stored as final Uint8List? >bytes. Which one is it? Why do I need both?

A stream of data is completely different of an array of bytes. The purpose of both is kinda self-explanatory and you have the info in the properties of the bytes for example, but anyway here it is:

Again, don't take me wrong, but at this point there's no ETA for a foundational API change so I'll close this. Like you, I'd like to see a few more methods that could save some loc, but this needs to be well thought about. If later on this comes as topic, I'll be happy to re-open it.

navaronbracke commented 6 months ago

I have to agree with @matanlurey here. We have a bunch of things that can be improved in our API that could make things better.

The first of which is getting rid of the monstrosity that is FilePickerWeb in the public API, which forces people to do Platform.is checks. Which lead to a lot of people complaining on the web.