aws-amplify / amplify-flutter

A declarative library with an easy-to-use interface for building Flutter applications on AWS.
https://docs.amplify.aws
Apache License 2.0
1.32k stars 246 forks source link

Publishing Amplify Flutter packages breaks apps that refer to older versions of the SDK #373

Closed kjones closed 3 years ago

kjones commented 3 years ago

When a new version of amplify-flutter is released apps that depend on older versions of the SDK end up pulling artifacts from the new SDK.

The pubspec.yaml being used to build the SDK refers to its own version using '^' instead of a specific version. Ex:

dependencies:
  flutter:
    sdk: flutter
  amplify_auth_plugin_interface: ^0.0.2-dev.2
  amplify_core: ^0.0.2-dev.2
  plugin_platform_interface: ^1.0.1

Therefore any app that refers to say 0.0.2-dev.1 may instead get 0.0.2-dev.2 artifacts pulled in for some transitive dependencies.

kjones commented 3 years ago

Example build where you see amplify_auth_cognito-0.0.2-dev.1 fail due to building with transitive dependencies that got pulled in from 0.0.2-dev.2.

../../programs/flutter/.pub-cache/hosted/pub.dartlang.org/amplify_auth_cognito-0.0.2-dev.1/lib/method_channel_auth_cognito.dart:285:13: Error: The getter 'AmplifyDartExceptions' isn't defined for the class 'AmplifyAuthCognitoMethodChannel'.
 - 'AmplifyAuthCognitoMethodChannel' is from 'package:amplify_auth_cognito/method_channel_auth_cognito.dart' ('../../programs/flutter/.pub-cache/hosted/pub.dartlang.org/amplify_auth_cognito-0.0.2-dev.1/lib/method_channel_auth_cognito.dart').
Try correcting the name to the name of an existing getter, or defining a getter or field named 'AmplifyDartExceptions'.
      throw(AmplifyDartExceptions.formatException(methodName: method, field: "nextStep"));
            ^^^^^^^^^^^^^^^^^^^^^
fjnoyp commented 3 years ago

Thanks for bringing this to our attention @kjones

Could you share your pubspec.yaml here? Do you import your dependencies like (1) or (2)?

(1) amplify_flutter: '<1.0.0' amplify_auth_cognito: '<1.0.0' amplify_analytics_pinpoint: '<1.0.0'

(2) amplify_flutter: ^0.0.2-dev.1 amplify_auth_cognito: ^0.0.2-dev.1 amplify_analytics_pinpoint:^0.0.2-dev.1

kjones commented 3 years ago

(3) we specify versions explicitly:

name: some_app
publish_to: none
version: 1.2.0

environment:
  sdk: ">=2.10.4 <3.0.0"

dependencies:
  flutter:
    sdk: flutter

  cupertino_icons: 1.0.0
  material_design_icons_flutter: 4.0.5855

  date_range_picker: 1.0.6
  smart_select: 4.3.2

  firebase_core: 0.7.0
  firebase_crashlytics: 0.4.0+1
  firebase_analytics: 7.0.1

  auto_route: 0.6.9
  cached_network_image: 2.5.0
  carousel_slider: 2.3.1
  equatable: 1.2.5
  flutter_bloc: 6.1.1
  freezed_annotation: 0.12.0
  get_it: 5.0.6
  injectable: 1.0.7
  intl: 0.16.1
  kt_dart: 0.8.0
  logger: 0.9.4
  rxdart: 0.25.0
  shared_preferences: 0.5.12+4
  sprintf: 5.0.0
  tuple: 1.0.3
  uni_links: 0.4.0
  url_launcher: 5.7.10
  uuid: 2.2.2
  webview_flutter: 1.0.7
  package_info: 0.4.3+2
  auto_size_text: 2.1.0
  scrollable_positioned_list: 0.1.9
  youtube_player_iframe: 1.1.0
  youtube_explode_dart: 1.7.5
  visibility_detector: 0.1.5
  flutter_speed_dial: 1.2.5

  amplify_flutter: 0.0.2-dev.2
  amplify_auth_cognito: 0.0.2-dev.2
  amplify_datastore: 0.0.2-dev.2

dev_dependencies:

  # Test dependencies.
  bdd_widget_test:
  golden_toolkit:
  gherkin:
  mockito:
  glob:
  test:
  flutter_test:
    sdk: flutter

  # Build and analyzer dependencies.
  lint:
  build_runner:
  auto_route_generator: 0.6.10
  freezed: 0.12.7
  json_serializable: 3.5.1
  injectable_generator: 1.0.7

flutter:
  uses-material-design: true

Edit: This is using 0.0.2-dev.2 since the easiest fix for us was to just upgrade and update our exception handling. Earlier today we were at 0.0.2-dev.1 when the build error listed above happened.

fjnoyp commented 3 years ago

Thanks for sharing that @kjones

We now recommend that you import the Amplify Flutter pubspec dependencies using:

amplify_flutter: '<1.0.0' amplify_auth_cognito: '<1.0.0' amplify_analytics_pinpoint: '<1.0.0'

instead of hardcoded 0.0.2-dev.2 / 0.0.2-dev.1 etc.

kjones commented 3 years ago

amplify_flutter: '<1.0.0' amplify_auth_cognito: '<1.0.0' amplify_analytics_pinpoint: '<1.0.0'

But won't this give me the latest every time you release whether I'm ready for it or not?

Ashish-Nanda commented 3 years ago

@kjones is it recommended/common to pin versions of libraries in flutter apps? This is just for a better understanding how our customers use our plugins.

In this case I believe the is the issue more to do with the fact that in the -dev releases we are not being as strict about breaking changes.

But my assumption would be that developers would use a carat in their pubspec.yaml like so to intentionally pull in the latest minor version of the library:

amplify_datastore: ^0.0.2-dev.2
kjones commented 3 years ago

This isn't really an issue about how I'm specifying package versions. It's about the way you are specifying package dependencies between your own packages within each release.

Note that the build error shown above is not due to our code needing to be updated. This shows an error within your own library where a 0.0.2-dev.1 artifact attempts to build using a dependency that resolved to a 0.0.2-dev.2 artifact.

To clarify, the minute you published 0.0.2-dev.2 with breaking changes you invalidated your own previously published 0.0.2-dev.1 packages due to the way 0.0.2-dev.1 packages specified their own internal transitive dependencies as ^0.0.2-dev.1.

I'm not sure the caret (^) syntax is the right call on transitive dependencies for packages that are all grouped together as a release. If you stick with the current ^ references for internal dependencies, this would mean amplify_auth_cognito 1.0.0 has the potential to pull in any version of amplify_core <2.0.0.

Ashish-Nanda commented 3 years ago

This isn't really an issue about how I'm specifying package versions. It's about the way you are specifying package dependencies between your own packages within each release.

Yes I totally agree, this is unrelated to the issue. I was asking out of curiosity if this is a common practice in Flutter (we can update our docs accordingly)

The problem here is that along with using the ^ we also did a breaking change in the next release without bumping the major version. I noted this above, and it was a miss on our part:

In this case I believe the is the issue more to do with the fact that in the -dev releases we are not being as strict about breaking changes.

I agree with you that at this point we do not need to use the ^ and can simply pin the versions when we group the package releases. This is likely the correct approach, however I'm not sure if package releases will always be grouped together in the future. This is why by following SemVer we would want the latest packages for the dependencies for a given major version of the a plugin, and those should also not have breaking changes. So like you say:

this would mean amplify_auth_cognito 1.0.0 has the potential to pull in any version of amplify_core <2.0.0

I realize the issue we caused here, and apologize for the inconvenience. Thanks for bringing it to our attention. We will review this among the team and update the thread with the changes that we plan to make to mitigate this in the future.

kjones commented 3 years ago

I was asking out of curiosity if this is a common practice in Flutter (we can update our docs accordingly)

Using pinned versions is a decision we came to after getting bit several times by package developers who don't correctly update version numbers when introducing breaking changes. It also helps us make sure that we are shipping the exact same code that was tested with the exact same dependency versions. After a release, we'll manually update dependencies so they get tested throughout the next release cycle. The manual update can be accomplished in a matter of minutes with tools like VersionLens.

Ashish-Nanda commented 3 years ago

@kjones We released version 0.1.0 yesterday and have pinned the versions of the dependencies that we own.

We plan to do this going forward, and will share an update with the community if anything changes.

Pinning the version should work correctly now. Will go ahead and close the issue, but let us know if you encounter issues and I'll re-open.

kjones commented 3 years ago

Thanks @Ashish-Nanda - Congrats on the release and thanks for the hard work to the whole team.

SpuriousGeek commented 3 years ago

@kjones Really appreciate your comments on this issue. I experienced the exact same problem earlier today and I've also ended up opting to use pinned versions (again because it was the simpler option in my case). I hadn't used VersionLens previously but it's a great tool that I will using to do manual updates going forward.