search

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.35k stars 675 forks source link

fix default_plugin warnings with recent dart sdk #1567

Closed Sameri11 closed 3 months ago

Sameri11 commented 3 months ago

This PR addresses #1555 and removes warning. Implementation is based on this comment from https://github.com/flutter/flutter/issues/152037

Fixes https://github.com/miguelpruivo/flutter_file_picker/issues/1484 Fixes https://github.com/miguelpruivo/flutter_file_picker/issues/1483 Fixes https://github.com/miguelpruivo/flutter_file_picker/issues/1343

Details:

Old implementation with default_plugin never did anything over than providing correct list of supported platforms for package page on pub.dev. Since some version of dart sdk, this behaviour has become warning. New implementation provides (arguably) correct usage of flutter:plugins: section to avoid warning. Since all logic for choosing current platform and appropriate plugin's implementation is encapsulated inside current code, this PR just provides empty registerWith static methods needed for compilation. Also, it is mandatory for plugin implementations to be public, so we export them.

Testing:

Current tests are not affected by this PR. Example app is also functioning (tested on android and ios). Since tests on macOS (my dev machine) and example on both emulators works – I believe that functionality is not broken.

1555 provides reproduction with https://github.com/zulip/zulip-flutter repo. This fix can be tested in that repo by adding dependency_overrides to pubspec.yaml. This override will point to my fork with fix:

dependency_overrides:
  file_picker:
    git:
      url: git@github.com:Sameri11/flutter_file_picker.git
      ref: fix-issue-1555
Sameri11 commented 3 months ago

Hey @miguelpruivo, @philenius can you please review this fix and/or approve workflow?

OlaiyaO commented 3 months ago

I see you're all actively working on it. I'm still experiencing the issue here.

"Package file_picker:windows references file_picker:windows as the default plugin, but it does not provide an inline implementation. Ask the maintainers of file_picker to either avoid referencing a default implementation via platforms: windows: default_package: file_picker or add an inline implementation to file_picker via platforms: windows: pluginClass or dartPluginClass."

navaronbracke commented 3 months ago

I see you're all actively working on it. I'm still experiencing the issue here.

"Package file_picker:windows references file_picker:windows as the default plugin, but it does not provide an inline implementation. Ask the maintainers of file_picker to either avoid referencing a default implementation via platforms: windows: default_package: file_picker or add an inline implementation to file_picker via platforms: windows: pluginClass or dartPluginClass."

That is indeed what this PR would aim to solve yes. Once we get the required changes in this PR, I'll review it again and then we can release a fix version.

Sameri11 commented 3 months ago

@navaronbracke can you please take another look at this? I think I implemented what you talked about here.

Btw, I had hard times reproducing #1343 both based on this PR and master – no success. Either I do something wrong, or, maybe, these issues were fixed earlier somehow.

navaronbracke commented 3 months ago

I also edited the original PR comment to list fixed issues, so these get closed when the PR is merged.

As for not being able to reproduce https://github.com/miguelpruivo/flutter_file_picker/issues/1343 I'm not sure either.

navaronbracke commented 3 months ago

It looks like one of the test is failing, because https://github.com/miguelpruivo/flutter_file_picker/pull/1561/files did not update a test expectation.

navaronbracke commented 3 months ago

Checks are green, so ship it!