- Notifications
You must be signed in to change notification settings - Fork3.6k
[google_maps_flutter] Convert plugin class to Swift#10545
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
[google_maps_flutter] Convert plugin class to Swift#10545
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Converts the top-level plugin class and its supporting platform viewfactory from Objective-C to Swift. This changes very littleimplementation, but lays the groundwork for future incrementalconversions.Part offlutter/flutter#119108
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Code Review
This pull request successfully converts the top-level plugin class and its associated platform view factory from Objective-C to Swift. The changes are well-contained and include necessary updates to the podspec, module definitions, and test files to support the new Swift implementation. The core logic, including the handling of sharedGMSServices, is correctly preserved.
My review includes one suggestion to improve the robustness of the new Swift code by replacing force-unwrapping and force-casting withguard statements. This will make the plugin more resilient to unexpected inputs and prevent potential crashes.
| // Precache shared map services, if needed. Initializing this prepares GMSServices | ||
| // on a background thread controlled by the GoogleMaps framework. | ||
| letmapServices=GoogleMapFactory.sharedMapServices | ||
| returnFLTGoogleMapController( | ||
| frame: frame, viewIdentifier: viewId, | ||
| creationParameters: argsas!FGMPlatformMapViewCreationParams, registrar: registrar!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The use of force casting (as!) and force unwrapping (!) can make the app crash if the assumptions aboutargs type orregistrar being non-nil are violated. It's safer to useguard let with conditional casting (as?) and optional binding to validate these before use. This provides more robust error handling and clearer failure messages if something goes wrong, which is especially useful for plugin consumers.
Also, using_ = ... is more idiomatic for calling something for its side effects while ignoring the result, which avoids an 'unused variable' warning.
// Precache shared map services, if needed. Initializing this prepares GMSServices // on a background thread controlled by the GoogleMaps framework. _=GoogleMapFactory.sharedMapServicesguardlet creationParams= argsas?FGMPlatformMapViewCreationParamselse{ // This is a programmer error on the Dart side, so crashing is reasonable.fatalError("Invalid creation parameters for Google Map:\(String(describing: args))")}guardlet registrar=self.registrarelse{ // This should not happen in a normal lifecycle.fatalError("Registrar is nil when creating Google Map.")}return FLTGoogleMapController( frame: frame, viewIdentifier: viewId, creationParameters: creationParams, registrar: registrar)
| XCTAssertEqual(mapView.frameObserverCount,0); | ||
| } | ||
| - (void)testMapsServiceSync { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
stuartmorgan-g commentedDec 2, 2025
Well, this is fun. We appear to have a SwiftPM-style problem with CocoaPods as well if we adopt any Swift in the target: It goes away if I change the podspec's |
Converts the top-level plugin class and its supporting platform view factory from Objective-C to Swift. This changes very little implementation, but lays the groundwork for future incremental conversions.
Part offlutter/flutter#119108
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to thepub versioning philosophy, or I have commented below to indicate whichversion change exemption this PR falls under1.CHANGELOG.mdto add a description of the change,following repository CHANGELOG style, or I have commented below to indicate whichCHANGELOG exemption this PR falls under1.///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.↩↩2↩3