🐛 [firebase_analytics] minor release has breaking changes

[mockturtl]

Running flutter pub upgrade bumped my firebase_analytics from ^10.0.7 to 10.1.0.

I see new errors:

E/flutter ( 9611): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: 'package:firebase_analytics/src/firebase_analytics.dart': 
Failed assertion: line 88 pos 9: 'value is String || value is num': 'string' OR 'number' 
must be set as the value of the parameter: foo_isBar

The changelog points to #9520, which says:

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change.

Indeed, the diff shows new assertions in firebase_analytics.dart.

I believe the package's major version should have been incremented to 11.0.0, as required by semver.

@russellwheatley @Lyokone @pr-Mais @mateusfccp @victor-tinoco

---

Separately: as a user,

1. I would like to see the "why & how" of this migration documented more clearly. It seems to revert fix(firebase_analytics): support complex data structures like list and map on Android #4394.

2. Would you consider just serializing value (replace it with '$value'), instead of making the type assertion? I don't have (or want) type tests for all the map values I pass to logEvent.

3. If the answer is "no," the doc comment on logEvent needs to call out that it is NOT safe to pass an arbitrary Map<String, Object?> parameters, in spite of the method signature.

[russellwheatley]

Hi @mockturtl, the reason we added the asserts is because only primitive values are accepted by the Analytic's server. Unfortunately, the native APIs don't throw an exception when you pass in more complex data structures (lists or maps for example) which give you the illusion they are successfully registered. We implemented the asserts to stop users from making that mistake.

If you check the PR notes, you will have a more comprehensive understanding of the issue.

I understand your concern for better documentation. I've created a PR to clear confusion on accepted values.

[mockturtl]

@russellwheatley @Lyokone

Semver guarantees that minor package upgrades (10.0.7 -> 10.1.0) are safe:

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API.

The new assert constitutes a breaking API change. "My code worked before, now it throws errors."

I understand this package wraps a clunky Android API, and the assert gets stripped in release mode. Thank you for the doc updates.

Please consider yanking 10.1.0 and re-releasing as 11.0.0. 💙