-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[url_launcher] Alternative to isMock
#2326
Conversation
isMock from being used in release builds
amirh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks!
|
@Hixie I've updated this PR based on our discussion, PTAL |
isMock from being used in release buildsisMock
|
After discussing with Amir I think we'll want to copy these new base classes into a new first-party package (verify_extends? platform_interface?) and also add the base classes to the Flutter SDK. We'll need to wait until after next Flutter stable release to switch over to using the Flutter SDK versions of the classes in federated plugins and then we can deprecate the first-party package.
I'm closing this PR since it's not able to land in the current state, @amirh can you take it from here? |
Adds a package with a common base class for platform interface. The PlatformInterface class provides common functionality for enforcing that platform implementations extend the platform interface with extends rather than implement it with implements. This is based on the existing code from the url_laucher_platform_interface, and the improvements proposed in #2326
Adds a package with a common base class for platform interface. The PlatformInterface class provides common functionality for enforcing that platform implementations extend the platform interface with extends rather than implement it with implements. This is based on the existing code from the url_laucher_platform_interface, and the improvements proposed in flutter#2326
This is an attempt to address what I understand to be the underlying motivation of @Hixie's comment on #2230.
Problem
The
isMockAPI is currently in the public docs (despite being@visibleForTesting) and we're not thrilled about it.The
@visibleForTestingannotation onisMockresults in a warning that can be easily ignored by the plugin consumer. This could create a maintenance burden if platform implementers create packages that use theisMockbackdoor to implement rather than extend the platform interface.Furthermore, it is currently possible to bypass
isMocktrivially withnoSuchMethod.Solution
This PR proposes a base class
PlatformInterfacethat plugins can extend. It provides a static validation method that is used to ensure that subclasses useextendsrather thanimplements. The verification is performed using anidenticalcomparison on a privateObjecttoken.I believe that this approach makes it impossible to bypass the validation check using
noSuchMethod.The base classes should be extracted into a common package, see flutter/flutter#43368
To avoid introducing new API surface area that we later have to remove we should probably move the
PlatformInterfaceclass into either the Flutter SDK or a package./cc @amirh @goderbauer