feat(functions): migrate cloud functions Apple implementation to Swift #17232
Conversation
There was a problem hiding this comment.
Need to test this via SPM build. Remove old .h and .m cloud functions files. Also need to update the Package.swift file, removing these: https://github.com/firebase/flutterfire/blob/feat/cloud_functions_stream_support/packages/cloud_functions/cloud_functions/ios/cloud_functions/Package.swift#L108-L113
| // found in the LICENSE file. | ||
|
|
||
| // Auto-generated file. Do not edit. | ||
| public let versionNumber = "5.3.4" |
There was a problem hiding this comment.
cloud_functions will need adding here to update Constants.swift when we publish release: https://github.com/firebase/flutterfire/blob/feat/cloud_functions_stream_support/scripts/generate_versions_spm.dart#L134
| s.ios.deployment_target = '13.0' | ||
|
|
||
| s.swift_version = '5.5' |
There was a problem hiding this comment.
can probably lower this to 5.0
There was a problem hiding this comment.
You will have to symlink the new Swift file in macOS.
- In terminal, root to directory you want to symlink to (i.e. macos/cloud_functions/....),
- run command:
ln -s [RELATIVE PATH TO IOS FILE YOU WANT TO SYMLINK] [NAME OF FILE YOU WANT TO SYMLINK IN MACOS]
Here is an example to help you:
ln -s ../../../packages/firebase_core/firebase_core/ios/firebase_core/Sources/firebase_core/include/firebase_core/messages.g.h messages.g.h
Do this for both new Swift files; Constants.swift, FirebaseFunctionsPlugin.swift.
You will also need to edit the podspec and Package.swift file in macos/ directory like what you have done for iOS.
There was a problem hiding this comment.
Also need to edit both podspecs this property: pod_target_xcconfig so it looks like this as we now set them from within Swift code: https://github.com/firebase/flutterfire/blob/main/packages/firebase_app_installations/firebase_app_installations/ios/firebase_app_installations.podspec#L38-L40
Also test macOS on cocoapods and SPM build. Thanks 😄
| } | ||
|
|
||
| @objc public func firebaseLibraryName() -> String { | ||
| "flutter-fire-functions" |
There was a problem hiding this comment.
| "flutter-fire-functions" | |
| "flutter-fire-fn" |
This needs to match what it was previously in podspec
| } catch { | ||
| completion(nil, FlutterError( | ||
| code: "illegal_argument", | ||
| message: error.localizedDescription, | ||
| details: nil | ||
| )) | ||
| } |
There was a problem hiding this comment.
Is this do/catch necessary? It wasn't in previous implementation and we catch or pass exception back via completion handler
| throw NSError( | ||
| domain: "com.firebase.functions", | ||
| code: -1, | ||
| userInfo: [ | ||
| NSLocalizedDescriptionKey: "Either functionName or functionUri must be set", | ||
| ] |
There was a problem hiding this comment.
this should just pass exception to completion handler like in previous implementation
| if let origin, !origin.isEmpty { | ||
| if let url = URL(string: origin) { | ||
| functions.useEmulator(withHost: url.host ?? "", port: url.port ?? 5001) | ||
| } | ||
| } |
There was a problem hiding this comment.
should also check that origin and port are defined rather than coalescing to empty string and passing default port.
...ns/cloud_functions/ios/cloud_functions/Sources/cloud_functions/FirebaseFunctionsPlugin.swift
Outdated
Show resolved
Hide resolved
russellwheatley
changed the title
Description
Replace this paragraph with a description of what this PR is doing. If you're modifying existing behavior, describe the existing behavior, how this PR is changing it, and what motivated the change.
Related Issues
Replace this paragraph with a list of issues related to this PR from the issue database. Indicate, which of these issues are resolved or fixed by this PR. Note that you'll have to prefix the issue numbers with flutter/flutter#.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?