Skip to content

Conversation

cesar-sosa-hol
Copy link
Collaborator

@cesar-sosa-hol cesar-sosa-hol commented Jun 29, 2025

Summary

This PR updates the Flutter plugin to support Flutter Embedding v2 and newer Flutter versions (specifically Flutter 3.29.3+ with Dart 3.7.2+). The main issue being solved was a compilation error: cannot find symbol class Registrar that occurred when developers upgraded to newer Flutter versions.

Key Changes

  1. Android Plugin Architecture Migration
    Removed deprecated Flutter v1 embedding support (PluginRegistry.Registrar)
    Migrated to Flutter v2 embedding using FlutterPlugin and ActivityAware interfaces
    Fixed the "cannot find symbol class Registrar" error by removing references to the deprecated PluginRegistry.Registrar
  2. Main Plugin Class Changes (SquareInAppPaymentsFlutterPlugin.java)
    Before:
    Used deprecated registerWith() method for v1 embedding
    Had static MethodChannel and complex workarounds for double initialization
    Mixed v1 and v2 embedding code
    After:
    Clean v2 embedding implementation with proper lifecycle management
    Non-static MethodChannel with proper initialization in onAttachedToEngine()
    Simplified method call handling using switch statements instead of if-else chains
    Better context management with applicationContext field
    Removed workaround comments about double initialization issues
  3. Module Refactoring (CardEntryModule.java & GooglePayModule.java)
    CardEntryModule:
    Removed v1 embedding constructor that took PluginRegistry.Registrar
    Simplified to single constructor taking Context and MethodChannel
    Inline activity result listener instead of separate class
    Lambda expressions for cleaner callback handling
    Removed copyright header (likely for brevity)
    GooglePayModule:
    Similar constructor simplification (Context + MethodChannel)
    Inline activity result handling with lambda expressions
    Cleaner error message strings
    Removed deprecated v1 embedding code
  4. Example App Modernization
    Android Build System:
    Migrated from Groovy to Kotlin DSL (build.gradle → build.gradle.kts)
    Updated to modern Flutter Gradle Plugin (dev.flutter.flutter-gradle-plugin)
    Updated Gradle version (8.7 → 8.12)
    Simplified build configuration
    App Structure:
    Updated package structure (sqip.flutter.example → com.sqip.flutter.square_in_app_payments_example)
    Modern Android manifest with proper Flutter v2 embedding configuration
    Updated themes and styles for modern Android
    Added proper debug/profile manifest variants
    Removed Legacy Files:
    Old custom themes, colors, and drawable resources
    Legacy launcher icons and configurations
    Deprecated build configurations
  5. Project Configuration Updates
    Updated .gitignore with modern Flutter patterns
    Updated .metadata with Flutter migration tracking
    Modernized Gradle properties with better JVM settings
    Technical Impact
    Compatibility: Now supports Flutter 3.29.3+ and Dart 3.7.2+
    Architecture: Cleaner, more maintainable code following Flutter v2 embedding best practices
    Performance: Removed workarounds and simplified initialization logic
    Future-proofing: Uses current Flutter plugin development patterns
    Breaking Changes
    Drops support for Flutter v1 embedding (very old Flutter versions)
    Requires Flutter 3.0+ (as documented in their requirements)

@cesar-sosa-hol cesar-sosa-hol requested a review from fka3 June 29, 2025 03:18
@CLAassistant
Copy link

CLAassistant commented Jun 29, 2025

CLA assistant check
All committers have signed the CLA.

@cesar-sosa-hol cesar-sosa-hol requested a review from Armaxis July 17, 2025 16:15
Comment on lines 73 to 75
cardEntryModule = null;
googlePayModule = null;
channel = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need these three lines? Is the issue resolved that required this workaround?

public void onAttachedToEngine(FlutterPluginBinding binding) {
this.applicationContext = binding.getApplicationContext();
channel = new MethodChannel(binding.getBinaryMessenger(), "square_in_app_payments");
// MethodCallHandler is set in onAttachedToActivity to avoid NPE issues
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is this going to be a valuable comment in 6 months?

I assume onAttachedTo... are Flutter things, and if the change described is for an NPE, it sounds as though the "normal" thing (no comment needed) should be to move this to the attach-to-activity moment?

googlePayModule.attachActivityResultListener(activityPluginBinding, channel);
cardEntryModule.attachActivityResultListener(activityPluginBinding, channel);
public void onReattachedToActivityForConfigChanges(ActivityPluginBinding activityBinding) {
onAttachedToActivity(activityBinding);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably light enough not to matter, but there is a reason a config change breaks the two modules here? That is, could we not discard them in all three of detachedFromActivity, detachedFromEngine, and detachedFromActivityForConfigChanges and re-create what I assume is the same thing all those times?

At the least, I bet you never detach from engine except after detaching from activity, so you could instead check(cardEntryModule == null) in onDetachFromEngine, because it should already have been done?


@Override
public void onMethodCall(MethodCall call, Result result) {
switch (call.method) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 the move to switch

public void onAttachedToEngine(FlutterPluginBinding flutterPluginBinding) {
channel = new MethodChannel(flutterPluginBinding.getBinaryMessenger(), "square_in_app_payments");

// KNOWN ISSUE: OnAttachedToEngine can be called twice which may be due to https://github.com/flutter/flutter/issues/69721
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we believe this issue resolved?

@@ -1,29 +1,24 @@
/*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should either have the license header or not, throughout? If it's inconsistent today, don't change that in this PR, though.

private static final String FL_MESSAGE_GOOGLE_PAY_NOT_INITIALIZED = "Please initialize google pay before you can call other methods.";
private static final String FL_MESSAGE_GOOGLE_PAY_RESULT_ERROR = "Failed to launch google pay, please make sure you configured google pay correctly.";
private static final String FL_MESSAGE_GOOGLE_PAY_UNKNOWN_ERROR = "Unknown google pay activity result status.";
private static final String FL_MESSAGE_GOOGLE_PAY_NOT_INITIALIZED = "Please initialize Google Pay before you can call other methods.";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but shouldn't these be resource strings for translation?

@cesar-sosa-hol cesar-sosa-hol self-assigned this Jul 22, 2025
@cesar-sosa-hol
Copy link
Collaborator Author

i will solve @fka3 comments in another PR

@cesar-sosa-hol cesar-sosa-hol merged commit e1a42d3 into master Jul 22, 2025
3 checks passed
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.

Flutter 3.29 can not build with this plugin since it needs v2 embedding, not v1
4 participants