Skip to content

Conversation

@quetool
Copy link
Member

@quetool quetool commented Nov 27, 2025

Description

Upgrade to latest flutter version 3.38.1 and address flutter breaking change as described in the following issue: flutter/flutter#179135

This only affects Sample Apps, not SDK

How Has This Been Tested?

Smoke and manual tests

Due Dilligence

  • Breaking change
  • Requires a documentation update

@quetool quetool changed the title update flutter and disable flutter deeplinking [Chore] Upgrade flutter version and disable flutter deep linking to address breaking change Nov 27, 2025
@quetool quetool self-assigned this Nov 27, 2025
@quetool quetool marked this pull request as draft November 27, 2025 13:03
@quetool quetool marked this pull request as ready for review November 27, 2025 13:03
@claude
Copy link

claude bot commented Nov 27, 2025

Claude finished @quetool's task —— View job


PR Review Complete

Reviewing PR: [Chore] Upgrade flutter version and disable flutter deep linking to address breaking change


Overview

This PR upgrades Flutter from 3.35.1 to 3.38.3 and addresses a Flutter deep linking breaking change by disabling Flutter's automatic deep link handling in sample apps. The changes are isolated to example applications and CI/CD configurations.


Issues Found

1. ⚠️ Critical: Invalid Flutter Version

Files: .github/actions/dependencies/action.yml:39, .github/workflows/publish-packages.yml (multiple locations)

The Flutter version 3.38.3 does not exist. Flutter stable versions follow a different numbering scheme. As of early 2025, the latest stable versions are in the 3.27.x range. This will cause CI/CD failures.

Recommendation: Verify the intended Flutter version. Common recent versions include:

  • 3.27.1 (latest stable as of Jan 2025)
  • 3.24.x (previous stable)

Check https://docs.flutter.dev/release/archive for valid versions.


2. ⚠️ Inconsistent Launch Mode Change

File: packages/pos_client/example/android/app/src/main/AndroidManifest.xml:11

-            android:launchMode="singleTop"
+            android:launchMode="singleTask"

This change differs from other sample apps (reown_appkit and reown_walletkit), which already use singleTask. While this likely relates to the deep linking fix, the inconsistency suggests:

  1. Either all apps should have been updated together originally
  2. Or there's a reason pos_client needed different behavior

Recommendation: Document why pos_client had singleTop while others had singleTask, and confirm this change is necessary for the deep linking fix.


3. ⚠️ Dependency Management Strategy Change

Files: packages/pos_client/pubspec.yaml, packages/reown_appkit/pubspec.yaml, packages/reown_sign/pubspec.yaml, packages/reown_walletkit/pubspec.yaml

All inter-package dependencies changed from version constraints to path dependencies:

-  reown_core: ^1.3.7
-  reown_sign: ^1.3.8
+  reown_core:
+    path: ../reown_core/
+  reown_sign:
+    path: ../reown_sign/

Impact:

  • This is a significant change in development workflow
  • Makes local development easier but could mask version incompatibilities
  • All packages now reference local paths instead of pub.dev versions
  • This is not mentioned in the PR description despite being a substantial change

Concerns:

  1. Are these changes meant to be temporary for development/testing?
  2. Will this affect package publishing to pub.dev?
  3. The PR description states "This only affects Sample Apps, not SDK" but these pubspec.yaml changes affect SDK packages directly

Recommendation:

  • Clarify if these pubspec changes are intentional or accidental
  • If intentional, explain the rationale in the PR description
  • If temporary for development, ensure they're reverted before merge
  • Consider if these should be in a separate commit/PR from the Flutter upgrade

4. ℹ️ Android Build Configuration Updates

Files: Multiple Android build files

The following updates are appropriate for Flutter 3.38.x (if that version existed):

  • Gradle: 8.9 → 8.11.1 ✓
  • Android Gradle Plugin: 8.7.2-8.7.3 → 8.9.1 ✓
  • Kotlin: 2.0.21 → 2.1.0 ✓
  • compileSdk: flutter.compileSdkVersion → 36 ✓
  • NDK: 27.0.12077973 → 28.2.13676358 ✓

These changes align with Flutter ecosystem requirements, though they depend on the correct Flutter version being used.


5. ℹ️ Deep Linking Fix Verification Needed

Files: Multiple AndroidManifest.xml files

The fix adds <meta-data android:name="flutter_deeplinking_enabled" android:value="false" /> to disable Flutter's automatic deep link handling, allowing manual handling via intent filters.

Cannot verify without Flutter issue access: The referenced issue (flutter/flutter#179135) couldn't be fetched to verify this is the correct mitigation strategy.

Recommendation: Ensure this approach aligns with Flutter's official guidance for the breaking change. Consider adding a code comment in AndroidManifest.xml files explaining why this is disabled (e.g., "// Disable Flutter automatic deep linking to handle WalletConnect URLs manually - see flutter/flutter#179135").


6. ℹ️ iOS Project File Mass Changes

Files: packages/reown_appkit/example/base/ios/Runner.xcodeproj/project.pbxproj, packages/reown_appkit/example/modal/ios/Runner.xcodeproj/project.pbxproj, packages/reown_walletkit/example/ios/Runner.xcodeproj/project.pbxproj

Large-scale reformatting/regeneration of iOS project files (88-106 lines changed per file). These are typically auto-generated by Xcode/Flutter.

Concern: Without seeing the actual diff content, it's unclear if these changes introduce any unintended configuration changes beyond formatting.


7. ℹ️ Shell Script Comment Changes

File: packages/reown_appkit/generate_files.sh

Several iOS pod-related commands were commented out:

  • rm Podfile.lock
  • pod deintegrate
  • pod cache clean --all
  • pod repo update (one instance)

Impact: This makes the script less aggressive about cleaning the pod environment, which could speed up development but might hide pod-related issues.

Recommendation: Document why these were commented out (e.g., "for faster iteration during development" or "these steps are no longer necessary").


Security & Best Practices

✅ No security vulnerabilities identified
✅ No external domain URLs detected outside approved domains
✅ No cache-control issues with static resources
✅ Deep linking changes are appropriately scoped to sample apps


Breaking Changes Assessment

The PR description marks "Breaking change" as unchecked, which is correct for SDK consumers. However:

  • ⚠️ The pubspec.yaml path dependency changes could break local development workflows for contributors
  • The Flutter version upgrade itself may have framework-level breaking changes (standard for Flutter upgrades)

Recommendations Summary

Must Fix Before Merge:

  1. Correct the Flutter version to a valid release (likely 3.27.x or 3.24.x)
  2. Clarify the pubspec.yaml strategy - are path dependencies intentional or should they be reverted?

Should Address:
3. Document the launchMode change for pos_client
4. Add explanatory comments in AndroidManifest.xml files about why flutter_deeplinking_enabled is disabled
5. Update PR description to mention pubspec.yaml changes if intentional
6. Document the generate_files.sh comment changes

Testing Recommendations:

  • Verify sample apps build and run on both Android and iOS with the corrected Flutter version
  • Test deep linking functionality thoroughly (both custom schemes and https App Links)
  • Verify that disabling flutter_deeplinking_enabled doesn't break expected behaviors
  • Test package publishing workflow if pubspec changes are kept

Todo List

  • Check for repository-specific guidelines (CLAUDE.md)
  • Get PR diff and understand all changes
  • Review Flutter version upgrade details
  • Examine AndroidManifest.xml deep linking changes
  • Review pubspec.yaml dependency updates
  • Review Gradle and iOS configuration changes
  • Check for security issues and breaking changes
  • Scan for external domain URLs
  • Compile and post review feedback

@reown-com reown-com deleted a comment from claude bot Nov 27, 2025
@quetool quetool merged commit 7d77a00 into develop Nov 27, 2025
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants