flutter_web_plugins cleanup and documentation #67164
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Hixie
force-pushed
the
flutter_web_plugins
branch
5 times, most recently
from
78c8018
to
715e263
Compare
| @@ -25,6 +30,8 @@ class PluginRegistry { | |||
| /// Registers this plugin handler with the engine, so that unrecognized | |||
| /// platform messages are forwarded to the registry, where they can be | |||
| /// correctly dispatched to one of the registered plugins. | |||
| /// | |||
| /// Only one [PluginRegistry] can be so registered at a time. | |||
There was a problem hiding this comment.
updated docs with more information while i was at it
Hixie
force-pushed
the
flutter_web_plugins
branch
from
715e263
to
fce0edc
Compare
|
Thanks for the review! Added label so this will land on green. |
| /// the [controller]. | ||
| /// | ||
| /// Events added to the [controller] when the framework is not | ||
| /// subscribed are silently dropped on the floor. |
There was a problem hiding this comment.
Not sure if "drop on the floor" is a commonly understood idiom. How about "ignored"?
| @Deprecated( | ||
| 'Replace calls to the "controller" setter with calls to the "setController" method. ' | ||
| 'This feature was deprecated after v1.23.0-7.0.pre.' | ||
| ) |
There was a problem hiding this comment.
Would be good to clean this up before stable release. How widespread is the usage of this setter?
There was a problem hiding this comment.
@Piinks is working on a deprecation policy.
There was a problem hiding this comment.
I'll make sure to send you the proposal, it will include what usage data I've been able to collect and some guidelines for cleaning up old code. <=1 week out from publishing :)
* Provide library documentation, so that there is introductory material on https://master-api.flutter.dev/flutter/package-flutter_web_plugins_flutter_web_plugins/package-flutter_web_plugins_flutter_web_plugins-library.html * Describe the API exposed by PluginEventChannel over the message channel. * Deprecate the `controller` setter on PluginEventChannel in favour of a method. Having only a setter violates the Dart style guide. * Avoid creating the _EventChannelHandler when it is immediately discarded. * Assert that the arguments to `listen` and `cancel` are null, and remove corresponding TODOs. * Apply the Flutter formatting style more widely (especially to avoid left margin confusion where less indented code is nested deeper in the AST). * Add additional detail to various parts of the docs. * Use the \`\`\`dart style for API doc sample code, to get analysis. * Improve the precision of some error messages. * Remove the use of ChannelBuffers, which as far as we can tell doesn't really make sense here (they are for engine->framework buffering, so using it here introduces a bunch of weird race conditions that could send messages in unexpected directions). I think it hasn't caused any problems yet because it's pretty rare to unsubscribe from channels which is when most of teh issues would manifest.
Hixie
force-pushed
the
flutter_web_plugins
branch
from
fce0edc
to
4ba64f5
Compare
Provide library documentation, so that there is introductory material on https://master-api.flutter.dev/flutter/package-flutter_web_plugins_flutter_web_plugins/package-flutter_web_plugins_flutter_web_plugins-library.html
Describe the API exposed by PluginEventChannel over the message channel.
Deprecate the
controllersetter on PluginEventChannel in favour of a method. Having only a setter violates the Dart style guide.Avoid creating the _EventChannelHandler when it is immediately discarded.
Assert that the arguments to
listenandcancelare null, and remove corresponding TODOs.Apply the Flutter formatting style more widely (especially to avoid left margin confusion where less indented code is nested deeper in the AST).
Add additional detail to various parts of the docs.
Use the ```dart style for API doc sample code, to get analysis.
Improve the precision of some error messages.
Remove the use of ChannelBuffers, which as far as we can tell doesn't really make sense here (they are for engine->framework buffering, so using it here introduces a bunch of weird race conditions that could send messages in unexpected directions). I think it hasn't caused any problems yet because it's pretty rare to unsubscribe from channels which is when most of teh issues would manifest.