google / flutter-desktop-embedding

Experimental plugins for Flutter for Desktop
Apache License 2.0
7.1k stars 607 forks source link

Recommended 'file_selector' plugin does not appear to support Windows #831

Closed esDotDev closed 3 years ago

esDotDev commented 3 years ago

This README instructs us to switch to file_selector https://github.com/google/flutter-desktop-embedding/blob/master/plugins/file_selector/README.md.

After doing so we're seeing MissingPluginException on Windows.

Is this expected? If so, maybe the README could be updated to clarify this. It costs us a couple precious hours as we switched over, and now apparently need to switch back.

stuartmorgan commented 3 years ago

The unendorsed Windows implementation does in fact support Windows. Did you follow the instructions in the README that you linked to? Please provide your pubspec.yaml.

esDotDev commented 3 years ago

Thanks Stuart, I missed that last line, and when looking for an example in the linked repo didn't find any mention of demonstration of that syntax. I didn't realize there was still an implementation example here.

For anyone else who stumbles on it, you want to use this:

file_selector_platform_interface: ^1.0.2
  file_selector_linux:
    path: ../file_selector_linux
  file_selector_macos:
    path: ../file_selector_macos
  file_selector_windows:
    path: ../file_selector_windows

from here: https://github.com/google/flutter-desktop-embedding/tree/master/plugins/file_selector/example

stuartmorgan commented 3 years ago

Thanks Stuart, I missed that last line, and when looking for an example in the linked repo didn't find any mention of demonstration of that syntax.

I'm not sure what syntax you are referring to. The implementation packages are regular published packages as it says in the README. There is no special syntax for using them.

For anyone else who stumbles on it, you want to use this:

That is for example code in this repository; like all plugin examples, it uses path-based dependencies to the package it is conceptually part of. Nobody else should do that. They should follow the instructions, and depend on the published packages.

esDotDev commented 3 years ago

I'm referring to the federated plugin syntax in the YAML that was not familiar to me, and probably is not to many developers.

Anyone coming here who previously used file_chooser, and is not familiar with the new federated plugin syntax, will likely be confused by the README as it stands.

The linked repo states explicitly: "To use this plugin, add file_selector as a dependency in your pubspec.yaml file."

Which is mis-leading and incomplete.

If you look at the pubspec for the src repo, it does not show the include syntax: https://github.com/flutter/plugins/blob/master/packages/file_selector/file_selector/pubspec.yaml

The example also does not show a complete usage example: https://github.com/flutter/plugins/blob/master/packages/file_selector/file_selector/example/pubspec.yaml

So, while I'm aware it's common to use path based includes for example, it doesn't change the fact that no actual usage example exists. The only place one actually exists is right here in this repo, as far as I can tell?

This would have save a lot of confusion if the README here (https://pub.dev/packages/file_selector) just plainly stated "This package uses the new federated plugin structure, and the full include statement is: "

stuartmorgan commented 3 years ago

I'm referring to the federated plugin syntax in the YAML that was not familiar to me, and probably is not to many developers.

I still don't know what that means. It's just multiple package dependencies. E.g.:

file_selector: ^0.7.0
file_selector_windows: ^0.0.1

I don't understand what part of that syntax is unfamiliar to Flutter developers.

Anyone coming here who previously used file_chooser, and is not familiar with the new federated plugin syntax, will likely be confused by the README as it stands.

The linked repo states explicitly: "To use this plugin, add file_selector as a dependency in your pubspec.yaml file."

Which is mis-leading and incomplete.

You're referring to the README of the primary (app-facing) package. There's a reason that the redirect from file_chooser links to this repository's README which both links to an explanation of endorsed and unendorsed packages (which describes exactly the above) and specifically calls out that part again inline.

From the standpoint of endorsed plugins, the file_selector README's description is complete. The whole concept of unendorsed plugins is that the main package isn't endorsing them.

This would have save a lot of confusion if the README here (https://pub.dev/packages/file_selector) just plainly stated _"This package uses the new federated plugin structure

It explicitly states that it's unendorsed with a link to a description of what that means—an explanation that is part of the federated plugin documentation. I don't know how to be more explicit than that. Especially since the part that you were confused by is not that it's federated, but that it's unendorsed. Most federated plugins are endorsed, in which case none of this applies.

and the full include statement is

The readme exactly describes what you need to do in terms that are familiar to anyone who knows how to depend on a package, which I expect to be the case for anyone using Flutter. You said above that you just didn't read that part (i.e., about 1/3 of the very short README). I don't see how using a different format for the part of the README that you didn't read would have changed the outcome here.

esDotDev commented 3 years ago

I still don't know what that means. It's just multiple package dependencies. E.g.: It means I don't know what the deps are, and I need them... how am I supposed to know what to type here? Totally confused how you expect devs to know this...

"It explicitly states that it's unendorsed with a link to a description of what that means" - Right so a developer who just wants to use a plugin, needs to dive a generic docs page to figure out how to use the plugin? Not what we want to be doing when we're on tight timelines.

"It explicitly states that it's unendorsed with a link to a description of what that means—an explanation that is part of the federated plugin documentation. I don't know how to be more explicit than that. Especially since the part that you were confused by is not that it's federated, but that it's unendorsed. Most federated plugins are endorsed, in which case none of this applies."

I have no clue what any of this means, federated, not federated, endorsed, not endorsed. I'm just looking for the lines I can add to my pub-spec, and a line of code I can call to get images, so I can pick some files and build an app. I expect this to be completely explained in the readme for the package, as every flutter plugin to date has been.

You're assuming a level of pre-knowledge here, that might exist inside the team, it doesn't exist among common developers. When I have 30 things on a todo list, the last thing I want to do is dive into some long documentation about federated plugins, that doesn't actually answer my direct question: "What is the full include statement for this specific plugin"

I really don't get the pushback here. The ask is "provide a complete example" which doesn't seem to be too much to ask. Your response seems to be "Well you should know this anyways, and one sentence linking to federation docs shoudl be enough".

When I actually provide the include statements devs will need, you respond that no one will ever need that. So I'm really confused... but for anyone who actually wants to just use the lib, there's the actual syntax so it will run. ^

esDotDev commented 3 years ago

Maybe I can clarify my question on this.

Starting from https://pub.dev/packages/file_selector, where do I find the list of available interfaces, like file_selector_windows: ^0.0.1 etc?

Things I tried:

Eventually I realized the trick here is to search directly in pub.dev: "file_selector", and then actually get the results we need. https://pub.dev/packages?q=file_selector

From dev POV, 2 main improvement can be made here:

  1. Clicking "packages that depend" on should work, but better yet, all the interfaces would show up in the right-hand sidebar
  2. The usage instructions could be much more clear, and just explicitly explain what non-federated means:
    
    Usage 
    This package is currently non-federated, which means you will need to explicitly include each platform you want to support.

To start, add file_selector as a dependency in your pubspec.yaml file. file_selector: ^0.8.2

Then add the platforms that you require: file_selector_windows: ^0.0.2 file_selector_macos: ^0.0.4 file_selector_linux: ^0.0.2 file_selector_web: ^0.8.1



I'm sure this would be greatly appreciated by devs as they begin to use this plugin. Obviously if it gets federated tmrw its a non-issue :) 
stuartmorgan commented 3 years ago

Starting from https://pub.dev/packages/file_selector, where do I find the list of available interfaces, like file_selector_windows: ^0.0.1 etc?

I'm assuming you mean "implementations" not "interfaces". I'm not aware of any way to specifically search for unendorsed implementations of a plugin. Seems like a reasonable (although probably low-priority since it's a niche use case; unendorsed plugins are almost certainly very rare) feature request for pub.dev.

  1. Clicking "packages that depend" on should work

It does work, but none of these implementations depend on the file_selector package. (Some implementations could depend on file_selector_platform_interface, but in the case of these that's not true either.)

but better yet, all the [implementations] would show up in the right-hand sidebar

This could be done via pub analysis of implements keys; again, a reasonable request for pub.dev.

  1. The usage instructions could be much more clear, and just explicitly explain what non-federated means:

I assume you mean "non-endorsed" rather than "non-federated".

Again, I think you're not understanding the critical distinction between endorsed and unendorsed implementations. The whole concept of an unendorsed implementation is that the author/maintainer of the main plugin is not recommending it. If they wanted to recommend that everyone use it, they would just endorse it in their pubspec and everything would happen behind the scenes.

Instruction for using unendorsed implementations belong in the unendorsed implementations. Such as the README in this repository (and of the desktop implementations). Someone would currently find the unendorsed desktop implementations of file_selector in exactly the same way they would have found file_chooser before that, which is that they'd be pointed to this repository by someone.

This package is currently non-federated, which means you will need to explicitly include each platform you want to support.

This statement doesn't make sense. It is federated, but assuming you meant to talk about endorsement "non-endorsed" is something that applies to implementations. The app-facing package does endorsement, it doesn't have it. And any federated plugin can have unendorsed implementations. They could easily have unendorsed implementations that the primary plugin developer doesn't even know about. The existence of unendorsed implementations is not a property of the app-facing package.

To start, add file_selector as a dependency in your pubspec.yaml file. file_selector: ^0.8.2

Then add the platforms that you require: file_selector_windows: ^0.0.2 file_selector_macos: ^0.0.4 file_selector_linux: ^0.0.2 file_selector_web: ^0.8.1

This is almost exactly what the READMEs for file_selector_{linux,macos,windows} already say.

esDotDev commented 3 years ago

Thanks for the explainer, sorry for my butchery of the terminology. I do understand now federated/endorsed, and why un-endorsed package would, under normal circumstances not be referenced in the read me.

This is almost exactly what the READMEs for fileselector{linux,macos,windows} already say.

Sure, once you figure out where they are hiding, and load 4 different web pages and individually copy each one by hand. If it were me, I'd definitely throw devs a bone in the readme, even if it's literally a one liner on the topic, and link to pub.dev search results, because clarity is better than non-clarity which certainly exists now with those usage instructions and zero endorsed plugins.

Feels like it almost being purposefully vague, and just not sure what is the point of that. If you can save a multiplier of people confusion downstream, with a sentence or two, why not?

esDotDev commented 3 years ago

Also, fwiw searching "open and save files" in pub-dev, returns only the interface and no implementations. So again, a developer would have absolutely no clue to copy "file_selector" and search for a 2nd time in pub.dev search box. Only by searching the exact name of the package can these even be found...

stuartmorgan commented 3 years ago

Sure, once you figure out where they are hiding

They are no harder to find than file_chooser was, so I'm still confused as to why you continue to assert that they are hidden when you got to them from the file_chooser README.

Feels like it almost being purposefully vague

The README here--the README that is in the same folder as the packages you describe as hidden--explains the situation, what to do, and also links to more details. We'll have to agree to disagree that that is vague.

and zero endorsed plugins

The current release endorses web, so now behaves in every respect like any other plugin that only supports web.

If you are upset that the app-facing plugin was published prior to having endorsed implementations, that is a) unrelated to this project, and b) not a decision I was part of, so complaining about it here is unproductive.

Also, fwiw searching "open and save files" in pub-dev, returns only the interface and no implementations. So again, a developer would have absolutely no clue to copy "file_selector" and search for a 2nd time in pub.dev search box.

I do not expect developers to find the--quoting from this project's README--"experimental, early-stage desktop plugins" that live in FDE by searching on pub.dev. No other plugins from this repository even exist on pub.dev.

I don't know why you expect these plugins to follow a completely different standard of discoverability than the other plugins here--including the plugin it replaced--but that is not, and has never been, the intent.

esDotDev commented 3 years ago

To be clear I'm not upset, I'm just making the case for more clarity for future devs. I have the includes and they work fine, we're not blocked.

As far as I'm aware this is the only way today any developer can get a file picker running on Web and all Desktop OS's, so regardless of labelling developers will be looking to use this.

I don't think this is hard to understand if you simply put yourself in the shoes of a developer looking to use this package. Go from step 1, you are a dev, on pub-dev and you need a file picker. The path to getting this installed is way more obfuscated than it needs to be, it's honestly silly.

They will

If you shrug your shoulders at that, and think that's an acceptable experience for your users, there's not much more to say I guess.

file_chooser had a functional example in the same repo it was created it, making it extremely easy to look at the example and pull the code in, fwiw. It's the de-coupling to a fault that is an issue here.

stuartmorgan commented 3 years ago

Go from step 1, you are a dev, on pub-dev and you need a file picker.

I have already said, explicitly, that I do not expect developers starting in pub.dev to find these unendorsed implementations. In exactly the same way that they would not have found file_chooser, as again, I've already said explicitly.

The path to getting this installed is way more obfuscated than it needs to be

Getting desktop implementations when starting from pub.dev will be trivial once they are endorsed. Feel free to vote for that issue in the Flutter issue tracker.

They will [...]

I do not expect anyone to follow the flow you have described.

and think that's an acceptable experience for your users

The only flow i expect users of these unendorsed implementations to follow is:

  1. Be pointed to the file_selector README in this repository by some resource (e.g., an SO answer).
  2. Follow the instructions there (assuming they are comfortable using experimental plugins).

Exactly like someone who was using file_chooser, or someone using menubar or window_size.

That level of discoverability is not only acceptable to me for the plugins in this repository, it is an intentional reflection of the level of automated testing (none) and support (minimal) they currently have.

file_chooser had a functional example in the same repo it was created it

As does file_selector. It's even closer to the relevant code, and more clearly named, than was the case with file_chooser.

If you have more to say on this topic, please find a more constructive way to do so than dismissing the considered decision of the creator and maintainer of this repository "silly" just because you disagree with it.