Compile errors in dependent packages when making minor changes to FacebookAuthPlatform

[collinjackson]

Describe the bug

flutter_facebook_auth_platform_interface's use of plugin_platform_interface is not consistent with documentation and could lead to compile errors in other dependent packages if it changes.

A minor update to FacebookAuthPlatform (such as adding a new method) is a breaking change for apps that depend on flutter_facebook_auth directly. It is also a compile error if they depend flutter_facebook_auth indirectly, e.g. through the flutterfire_ui package, because they'll pick up the minor update through the transitive dependency on facebook_auth_platform_interface. It breaks builds even for users who are not using Facebook auth at all, but are using another auth provider.

Suggested fix:

• FacebookAuthPlatform should not be exported to clients of FacebookAuth. (As an added bonus, this will cause it to disappear from the flutter_facebook_auth documentation.)
• FacebookAuth should not extend or implement FacebookAuthPlatform.
• FacebookAuthPlatform should use static final instead of static const for _token.
• The setter for instance should call verify. (See example.)

/cc @stuartmorgan @Ehesp

Environment flutter_facebook_auth_platform_interface 2.7.1 flutter_facebook_auth 3.5.7

To Reproduce

• flutter create an app
• Add a dependency of flutterfire_ui: ^0.3.1 to the app's pubspec.yaml and import 'package:flutterfire_ui/auth.dart';.
• Use dependency_overrides to point to a local version of flutter_facebook_auth_platform_interface in pubspec.yaml.
• Add a method void test() {} in the local version of FacebookAuthPlatform
• flutter run on the command line in the root of the app
• The following compile error appears:

  
  /../flutter/.pub-cache/hosted/pub.dartlang.org/flutter_facebook_auth-3.5.7/lib/flutter_facebook_auth.dart:8:7: Error: The non-abstract class
  'FacebookAuth' is missing implementations for these members:
  - FacebookAuthPlatform.test
  Try to either
  - provide an implementation,
  - inherit an implementation from a superclass or mixin,
  - mark the class as abstract, or
  - provide a 'noSuchMethod' implementation.

class FacebookAuth implements FacebookAuthPlatform { ^^^^^^^^^^^^ facebook_auth_platform_interface/lib/src/facebook_auth_plaftorm.dart:36:8: Context: 'FacebookAuthPlatform.test' is defined here. void test() {} ^^^^

**Expected behavior**
App should build

[stuartmorgan]

• The setter for instance should call verify. (See example.)

Note that this needs to be a breaking version change to the platform interface package, otherwise it can break existing versions of any package using it.

[stuartmorgan]

Is not a valid solution you never should do any update in a dependency of a package or plugin.

Are you saying you want it to be impossible to ever add anything to the plugin's platform interface? That's the current situation you have.

[darwin-morocho]

Is not a valid solution you never should do any update in a dependency of a package or plugin.

Are you saying you want it to be impossible to ever add anything to the plugin's platform interface? That's the current situation you have.

Can do any change but to avoid compile errors I need to do a refactor of the current facebookAuth class removing the implementation of FacebookAuthPlatform.

[darwin-morocho]

@collinjackson I will test your Suggested fix. except for The setter for instance should call verify. (See example.) because this could be a breaking change for flutterfire_ui

Removing implements FacebookAuthPlatform from FacebookAuth must fix the problem.

also if

FacebookAuthPlatform should not be exported to clients of FacebookAuth

How do you propose to maintain compatibility for ios, android and web?

[stuartmorgan]

Can do any change but to avoid compile errors [...]

To be clear by "impossible" I didn't mean that it's literally impossible to make changes, but that you cannot make any changes without causing the other packages not to compile.

except for The setter for instance should call verify. (See example.) because this could be a breaking change for flutterfire_ui

That's why I said above that you would need to do this as a breaking version change.

If you don't make a breaking version change to the platform interface you can't actually fix the problem, because existing, already published packages are already depending on this major version but doing the wrong thing. Any addition of methods to the platform interface would be incompatible with those already-published versions.

[stuartmorgan]

To provide a concrete example of what I mean by the above: Version 3.5.7 of flutter_facebook_auth contains: https://github.com/darwin-morocho/flutter-facebook-auth/blob/2c183e562aab2ab8f3df59d6de1d815e39c72a54/facebook_auth/lib/flutter_facebook_auth.dart#L8 and depends on ^2.7.1 of the platform interface: https://github.com/darwin-morocho/flutter-facebook-auth/blob/2c183e562aab2ab8f3df59d6de1d815e39c72a54/facebook_auth/pubspec.yaml#L13

Even if you do this:

I need to do a refactor of the current facebookAuth class removing the implementation of FacebookAuthPlatform.

in a new version of flutter_facebook_auth (let's call that version 3.5.8 for the sake of argument), 3.5.7 still exists. So imagine you then need to add a new method to flutter_facebook_auth_platform_interface. If you did so, and published it as 2.8.0, the situation you would have is that:

• 3.5.8 + 2.8.0 would work
• 3.5.7 + 2.8.0 would fail to compile

But according to semver 3.5.7 + 2.8.0 is a perfectly valid combination, because 2.8.0 isn't marked as a breaking change; the flutter_facebook_auth_platform_interface: ^2.7.1 constraint in 3.5.7 can easily resolve to 2.8.0.

So doing the refactor of the dependent package (what's I'm calling 3.5.8 in the example above) first doesn't actually help. You still need a major version change to the interface package.

[darwin-morocho]

@stuartmorgan

So Are you proposing that we need to use a version like 3.6.0 for the next release of flutter_facebook_auth?

[stuartmorgan]

So Are you proposing that we need to use a version like 3.6.0 for the next release of flutter_facebook_auth?

I'm saying that you need to use 3.0.0 for the release of flutter_facebook_auth_platform_interface that adds a call to verify, and that you cannot (without breaking everything) add any methods to FacebookAuthPlatform until you make that change.

But since you have https://github.com/darwin-morocho/flutter-facebook-auth/blob/5aaf6f23c8d00db17755e5da568056a2457cf673/facebook_auth/lib/flutter_facebook_auth.dart#L4

(for reasons that are not clear to me), you will need to update flutter_facebook_auth to 4.0.0 when it adopts flutter_facebook_auth_platform_interface 3.0.0, because a breaking change in a package you re-export is a breaking change for the exporting package too.

(I would strongly encourage you to, as @collinjackson suggested, take that opportunity to stop exporting flutter_facebook_auth_platform_interface from your app-facing package; that isn't standard practice, and is almost certainly not something you should actually be doing. I'm curious why you did that in the first place.)

[darwin-morocho]

@stuartmorgan I did this

 export 'package:flutter_facebook_auth_platform_interface/flutter_facebook_auth_platform_interface.dart'; 

because I need to expose to devs the LoginResult and the AccessToken class in the flutter_facebook_auth_platform_interface package

[stuartmorgan]

Ah, in that case you should absolutely use shows to export only the parts you explicitly want. That would avoid this kind of version coupling in the future.

[darwin-morocho]

Ah, in that case you should absolutely use shows to export only the parts you explicitly want. That would avoid this kind of version coupling in the future.

Great I'll do that

[stuartmorgan]

But to be clear, you also have to do that as part of a breaking change to flutter_facebook_auth, because reducing the set of things you export is itself a breaking change.

[darwin-morocho]

But to be clear, you also have to do that as part of a breaking change to flutter_facebook_auth, because reducing the set of things you export is itself a breaking change.

I got it

[darwin-morocho]

@collinjackson flutter_facebook_auth: ^4.0.0 has been released with your suggested changes.

[collinjackson]

I reviewed the changes that relate to plugin_platform_interface and it looks all good. I think this issue can be resolved. Thanks for the fast turnaround!