Skip to content

[breaking] Remove deprecated Android v1 embedding references #1005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 24, 2025

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Feb 19, 2025

Description

One Line Summary

Drop support for Android v1 embedding, which is quite old.

Details

The main changes are in one commit: 60dcfc while others are nits.

  • Remove references to the now deprecated v1 Android embedding.
  • The SDK used classes PluginRegistry.Registrar and io.flutter.view.FlutterNativeView
  • However, to support newer Flutter versions, we now need to completely drop v1 support.
  • Investigated other PRs in open-source repositories

Motivation

Reported in #928

Users targeting Flutter master and beta/stable releases after (but not including) 3.27 will fail to compile if the plugin has references any v1 embedding classes.

Scope

  • Drop support for Android v1
  • How many Flutter apps are actively maintained today that still use v1? Unsure but I can't imagine many at all.
  • From the issue reported above:

The v1 embedding was deprecated around 6 and a half years ago. In Flutter 3.22, the Flutter tool dropped support for building v1 apps entirely.

Testing

Unit testing

Manual testing

  1. Created new sample app on Flutter 3.29.0 and reproduced the reported build error.
  2. Change the dependency to the changes in this PR.
  3. App builds and runs, logs print, SDK works.
  4. Note that our embedded example app is on v2 and has been since PR Android plugin API updated to support v2 Embedding #486

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes
  • Deprecation changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

@nan-li
Copy link
Contributor Author

nan-li commented Feb 19, 2025

We will have to decide what kind of version bump we want to use. I don't think it's necessary to bump to 6.x.x, would prefer not to go to a major version. I think so few people are on v1 and it has been deprecated for 6 years that a minor bump is sufficient? Thoughts?

* Remove references to the now deprecated v1 Android embedding. `PluginRegistry.Registrar` and `io.flutter.view.FlutterNativeView` classes so that the SDK can be used on newer Flutter versions.
* Most imports were unused and leftover from when v2 support was added.
* However, we now need to completely drop v1 support.
* Now that it is not a Registrar, the name is misleading, rename to `FlutterMessengerResponder`
@nan-li nan-li force-pushed the android_remove_deprecated_v1_embedding branch from 01b4bfe to df09f09 Compare February 19, 2025 21:47
@nan-li nan-li requested a review from sherwinski February 20, 2025 20:21
@jkasten2
Copy link
Member

jkasten2 commented Feb 21, 2025

We will have to decide what kind of version bump we want to use. I don't think it's necessary to bump to 6.x.x, would prefer not to go to a major version. I think so few people are on v1 and it has been deprecated for 6 years that a minor bump is sufficient? Thoughts?

ya given the age I think a minor version bump is safe here.

@nan-li nan-li merged commit 2cfde0f into main Feb 24, 2025
2 checks passed
@nan-li nan-li deleted the android_remove_deprecated_v1_embedding branch February 24, 2025 17:38
@nan-li nan-li mentioned this pull request Feb 25, 2025
@stayallive
Copy link

We are still on v3 of the Flutter SDK because we are not quite ready to do the work to migrate. I couldn't find a quick and clear answer if the version of the SDK is still supported but we would highly appreciate it if it's possible to back port this change so we can at least keep up-to-date with Flutter while we work on migrating to 5.x of the OneSignal SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants