-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-land "Delete engine v1 android embedding" #52022
base: main
Are you sure you want to change the base?
Conversation
cc @stuartmorgan, as we talked about this on flutter/packages#6494 |
Hmm actually I need to retain the contents of the interface as well, part of which references the deprecated converting to draft to figure this out |
I think this unfortunately is actually going to have to wait until we warn plugin authors, which will happen in the next stable release. io.flutter.plugin.common.PluginRegistry.Registrar references the deprecated FlutterView, which in turn references much of the rest of the v1 embedding. And there are plugins depending on being able to get a I've requested to include a section in the next "whats new in" announcement, warning plugin authors to remove this method. As soon as the next stable is released, and that announcement is made, I'll continue forward with this deletion. |
Questions I have:
|
My understanding is that the code the plugins are using isn't even part of an interface, so we don't have an effective method of deprecating it. We simply told them to include a method (which references a deprecated type from the v1 embedding), and then v1 apps would call that method expecting it existed? I'm not 100% sure on that though (how it gets called - I am 100% sure it isn't in the FlutterPlugin interface)
There isn't a direct alternative. One method was for supporting v1 apps, and the other was for supporting v2 apps. At the time of the v1 embedding deprecation our recommendation was to support both. We haven't given further guidance since then, to my knowledge.
16 of our own here flutter/packages#6494. Also the |
Also because https://docs.flutter.dev/release/breaking-changes/plugin-api-migration was written for an audience of plugin developers familiar with the v1 embedding, it might not be clear what I mean when I say "we told them to" reference a deprecated type - its specifically this portion
This registerWith method that they would have had existing in their plugins already, has a signature |
That's correct, because in previous discussions the agreement was that we would remove tool support for v1 apps (on stable) before we told plugin authors (including ourselves) to remove v1 support from their plugins. Because IMO it doesn't make sense for us to tell the ecosystem to not support a build configuration that we are still supporting in the tool. |
Re-lands #51229
The only change is to un-delete the
PluginRegistry.Registrar
interface from the v1PluginRegistry
. The reason is that we recommended plugin authors to include a static method referencing this type in our own docs on v1->v2 migration, so deleting it would be a breaking change for lots of existing plugins.*Other failures from the initial attempt are fixed in flutter/flutter#146523
*There is more discussion at flutter/packages#6494
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.