Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions .github/workflows/tests_e2e_ios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,21 @@ jobs:
// we want to test debug and release - they generate different code
let buildmode = ['debug', 'release'];

// Test both SPM and CocoaPods dependency resolution modes
let depResolution = ['spm', 'cocoapods'];

return {
"iteration": iterationArray,
"buildmode": buildmode
"buildmode": buildmode,
"dep-resolution": depResolution
}
- name: Debug Output
run: echo "${{ steps.build-matrix.outputs.result }}"

# This uses the matrix generated from the matrix-prep stage
# it will run unit tests on whatever OS combinations are desired
ios:
name: iOS (${{ matrix.buildmode }}, ${{ matrix.iteration }})
name: iOS (${{ matrix.buildmode }}, ${{ matrix.dep-resolution }}, ${{ matrix.iteration }})
runs-on: macos-15
needs: matrix_prep
# TODO matrix across APIs, at least 11 and 15 (lowest to highest)
Expand Down Expand Up @@ -182,7 +186,7 @@ jobs:
- uses: hendrikmuhs/ccache-action@v1
name: Xcode Compile Cache
with:
key: ${{ runner.os }}-${{ matrix.buildmode }}-ios-v3 # makes a unique key w/related restore key internally
key: ${{ runner.os }}-${{ matrix.buildmode }}-${{ matrix.dep-resolution }}-ios-v3 # makes a unique key w/related restore key internally
save: "${{ github.ref == 'refs/heads/main' }}"
create-symlink: true
max-size: 1500M
Expand Down Expand Up @@ -214,8 +218,21 @@ jobs:
continue-on-error: true
with:
path: tests/ios/Pods
key: ${{ runner.os }}-ios-pods-v3-${{ hashFiles('tests/ios/Podfile.lock') }}
restore-keys: ${{ runner.os }}-ios-pods-v3
key: ${{ runner.os }}-ios-pods-v3-${{ matrix.dep-resolution }}-${{ hashFiles('tests/ios/Podfile.lock') }}
restore-keys: ${{ runner.os }}-ios-pods-v3-${{ matrix.dep-resolution }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I prefer the variable interpolations at the front and all together and the various naming / key "sauce" at the end

Suggested change
restore-keys: ${{ runner.os }}-ios-pods-v3-${{ matrix.dep-resolution }}
restore-keys: ${{ runner.os }}-${{ matrix.dep-resolution }}-ios-pods-v3


- name: Configure Dependency Resolution Mode
run: |
if [[ "${{ matrix.dep-resolution }}" == "cocoapods" ]]; then
echo "Configuring CocoaPods-only mode (disabling SPM)"
cd tests/ios
sed -i '' "s/^linkage = 'dynamic'/linkage = 'static'/" Podfile
printf '%s\n' '$RNFirebaseDisableSPM = true' | cat - Podfile > Podfile.tmp && mv Podfile.tmp Podfile
sed -i '' "/SWIFT_ENABLE_EXPLICIT_MODULES/d" Podfile
echo "Podfile configured for CocoaPods-only mode"
else
echo "Using default SPM mode (dynamic linkage)"
fi

- name: Pod Install
uses: nick-fields/retry@v3
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/tests_jest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ jobs:
retry_wait_seconds: 60
max_attempts: 3
command: yarn
- name: Test Firebase SPM Helper
run: ruby packages/app/__tests__/firebase_spm_test.rb
- name: Jest
run: yarn tests:jest-coverage
- uses: codecov/codecov-action@v5
Expand Down
1,135 changes: 1,135 additions & 0 deletions DOCUMENTACION_SPM_IMPLEMENTACION.md

Large diffs are not rendered by default.

760 changes: 760 additions & 0 deletions docs/ios-spm.md

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions docs/sidebar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- '/migrating-to-v24'
- - TypeScript
- '/typescript'
- - iOS SPM Support
- '/ios-spm'
- - Platforms
- '/platforms'
- - Release Notes
Expand Down
44 changes: 31 additions & 13 deletions packages/analytics/RNFBAnalytics.podspec
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'json'
require '../app/firebase_spm'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maintainer note (mirrored in similar note in firebase_spm.rb itself) to verify this is okay in monorepo (possibly hoisted)/pnpm/etc cases - there is likely a known-good inter-package local file resolution mechanism and it is likely not a simple ../<packagename>/ path unfortunately

package = JSON.parse(File.read(File.join(__dir__, 'package.json')))
appPackage = JSON.parse(File.read(File.join('..', 'app', 'package.json')))

Expand Down Expand Up @@ -45,24 +46,41 @@ Pod::Spec.new do |s|
end

# Firebase dependencies
s.dependency 'FirebaseAnalytics/Core', firebase_sdk_version
if defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && ($RNFirebaseAnalyticsWithoutAdIdSupport == true)
Pod::UI.puts "#{s.name}: Not installing FirebaseAnalytics/IdentitySupport Pod, no IDFA will be collected."
# Analytics has conditional dependencies that vary between SPM and CocoaPods.
# SPM: use FirebaseAnalyticsWithoutAdIdSupport when $RNFirebaseAnalyticsWithoutAdIdSupport = true
# to avoid GoogleAppMeasurement APM symbols that require FirebaseRemoteConfig (linker error).
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

was it FirebasePerformance symbols or FirebaseRemoteConfig symbols missing at link time? The comment on the PR and the comment here in code and slightly lower in code are inconsistent 🤔

# CocoaPods: IdentitySupport is a separate subspec controlled by $RNFirebaseAnalyticsWithoutAdIdSupport.
if defined?(spm_dependency) && !defined?($RNFirebaseDisableSPM) &&
defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && $RNFirebaseAnalyticsWithoutAdIdSupport
# FirebaseAnalyticsCore uses GoogleAppMeasurementCore (no IDFA, no APM objects).
# FirebaseAnalytics uses GoogleAppMeasurement which has APMETaskManager/APMMeasurement
# cross-references that cause linker errors when FirebasePerformance is not linked.
Pod::UI.puts "#{s.name}: Using FirebaseAnalyticsCore SPM product (no IDFA, uses GoogleAppMeasurementCore)."
firebase_dependency(s, firebase_sdk_version, ['FirebaseAnalyticsCore'], 'FirebaseAnalytics/Core')
else
if !defined?($RNFirebaseAnalyticsWithoutAdIdSupport)
Pod::UI.puts "#{s.name}: Using FirebaseAnalytics/IdentitySupport with Ad Ids. May require App Tracking Transparency. Not allowed for Kids apps."
Pod::UI.puts "#{s.name}: You may set variable `$RNFirebaseAnalyticsWithoutAdIdSupport=true` in Podfile to use analytics without ad ids."
end
s.dependency 'FirebaseAnalytics/IdentitySupport', firebase_sdk_version
firebase_dependency(s, firebase_sdk_version, ['FirebaseAnalytics'], 'FirebaseAnalytics/Core')
end

# Special pod for on-device conversion
if defined?($RNFirebaseAnalyticsEnableAdSupport) && ($RNFirebaseAnalyticsEnableAdSupport == true)
Pod::UI.puts "#{s.name}: Adding Apple AdSupport.framework dependency for optional analytics features"
s.frameworks = 'AdSupport'
unless defined?(spm_dependency)
# CocoaPods-only: conditional IdentitySupport subspec
if defined?($RNFirebaseAnalyticsWithoutAdIdSupport) && ($RNFirebaseAnalyticsWithoutAdIdSupport == true)
Pod::UI.puts "#{s.name}: Not installing FirebaseAnalytics/IdentitySupport Pod, no IDFA will be collected."
else
if !defined?($RNFirebaseAnalyticsWithoutAdIdSupport)
Pod::UI.puts "#{s.name}: Using FirebaseAnalytics/IdentitySupport with Ad Ids. May require App Tracking Transparency. Not allowed for Kids apps."
Pod::UI.puts "#{s.name}: You may set variable `$RNFirebaseAnalyticsWithoutAdIdSupport=true` in Podfile to use analytics without ad ids."
end
s.dependency 'FirebaseAnalytics/IdentitySupport', firebase_sdk_version
end
end

# Special pod for on-device conversion
# AdSupport framework (works with both SPM and CocoaPods)
if defined?($RNFirebaseAnalyticsEnableAdSupport) && ($RNFirebaseAnalyticsEnableAdSupport == true)
Pod::UI.puts "#{s.name}: Adding Apple AdSupport.framework dependency for optional analytics features"
s.frameworks = 'AdSupport'
end

# GoogleAdsOnDeviceConversion (CocoaPods only, not available in firebase-ios-sdk SPM)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there some documentation on how to access on-device conversion in the SPM case? This is surprising to me as I thought on-device conversion was one of the newer features of firebase-ios-sdk which creates the expectation in me that it should be available there somehow ? Perhaps just built in to core SPM dep I'm not sure

if defined?($RNFirebaseAnalyticsGoogleAppMeasurementOnDeviceConversion) && ($RNFirebaseAnalyticsGoogleAppMeasurementOnDeviceConversion == true)
Pod::UI.puts "#{s.name}: GoogleAdsOnDeviceConversion pod added"
s.dependency 'GoogleAdsOnDeviceConversion'
Expand Down
5 changes: 5 additions & 0 deletions packages/analytics/ios/RNFBAnalytics/RNFBAnalyticsModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
*
*/

#if __has_include(<Firebase/Firebase.h>)
#import <Firebase/Firebase.h>
#else
@import FirebaseCore;
@import FirebaseAnalytics;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Taking note of your comment here on the FirebasePerformance symbols missing at link time if ad ids are disabled does this FirebaseAnalytics import still work here in the FirebaseAnalyticsCore dependency case? #8933 (comment)

#endif
#import <React/RCTUtils.h>

#if __has_include(<RNFBAnalytics/RNFBAnalytics-Swift.h>)
Expand Down
3 changes: 2 additions & 1 deletion packages/app-check/RNFBAppCheck.podspec
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'json'
require '../app/firebase_spm'
package = JSON.parse(File.read(File.join(__dir__, 'package.json')))
appPackage = JSON.parse(File.read(File.join('..', 'app', 'package.json')))

Expand Down Expand Up @@ -44,7 +45,7 @@ Pod::Spec.new do |s|
end

# Firebase dependencies
s.dependency 'Firebase/AppCheck', firebase_sdk_version
firebase_dependency(s, firebase_sdk_version, ['FirebaseAppCheck'], 'Firebase/AppCheck')

if defined?($RNFirebaseAsStaticFramework)
Pod::UI.puts "#{s.name}: Using overridden static_framework value of '#{$RNFirebaseAsStaticFramework}'"
Expand Down
128 changes: 71 additions & 57 deletions packages/app-check/ios/RNFBAppCheck/RNFBAppCheckModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,15 @@
*
*/

#if __has_include(<Firebase/Firebase.h>)
#import <Firebase/Firebase.h>
#import <FirebaseAppCheck/FIRAppCheck.h>
#elif __has_include(<FirebaseAppCheck/FirebaseAppCheck.h>)
#import <FirebaseAppCheck/FirebaseAppCheck.h>
Comment on lines +18 to +21
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

<Firebase/Firebase.h> will always be found in the CocoaPods case won't it? It's the most fundamental header if I understand correctly. Does it transitively include <FirebaseAppCheck/FIRAppCheck.h> such that the explicit import is no longer required then (or maybe was never required?). Surprising this compiles as I think that preprocessor branch is likely the only __has_include branch that will ever be taken, and it makes me think the #elif __has_include(<FirebaseAppCheck/FirebaseAppCheck.h>) branch may not even be needed, I can't see how <Firebase/Firebase.h> won't be found

#import <FirebaseCore/FirebaseCore.h>
#else
@import FirebaseCore;
@import FirebaseAppCheck;
#endif

#import <React/RCTUtils.h>

Expand Down Expand Up @@ -53,7 +60,8 @@ + (instancetype)sharedInstance {
: (BOOL)isTokenAutoRefreshEnabled
: (RCTPromiseResolveBlock)resolve rejecter
: (RCTPromiseRejectBlock)reject) {
DLog(@"deprecated API, provider will be deviceCheck / token refresh %d for app %@",
DLog(@"deprecated API, provider will be deviceCheck / token refresh %d for "
@"app %@",
Comment on lines -56 to +64
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changing spacing unrelated to functional changes is not recommended, it's enforced with a project-wide config and a lint check via yarn lint:ios:check (and yarn lint:ios:fix). This will likely fail that check

isTokenAutoRefreshEnabled, firebaseApp.name);
[[RNFBAppCheckModule sharedInstance].providerFactory configure:firebaseApp
providerName:@"deviceCheck"
Expand Down Expand Up @@ -86,8 +94,8 @@ + (instancetype)sharedInstance {
appCheck.isTokenAutoRefreshEnabled = isTokenAutoRefreshEnabled;
}

// Not present in JS or Android - it is iOS-specific so we only call this in testing - it is not in
// index.d.ts
// Not present in JS or Android - it is iOS-specific so we only call this in
// testing - it is not in index.d.ts
RCT_EXPORT_METHOD(isTokenAutoRefreshEnabled
: (FIRApp *)firebaseApp
: (RCTPromiseResolveBlock)resolve rejecter
Expand All @@ -105,33 +113,36 @@ + (instancetype)sharedInstance {
: (RCTPromiseRejectBlock)reject) {
FIRAppCheck *appCheck = [FIRAppCheck appCheckWithApp:firebaseApp];
DLog(@"appName %@", firebaseApp.name);
[appCheck
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same comment on spacing - I won't repeat it for others, but in general never change spacing unrelated to the PR, it creates larger / less relevant diffs and will need a revert as it will fail lint anyway

tokenForcingRefresh:forceRefresh
completion:^(FIRAppCheckToken *_Nullable token, NSError *_Nullable error) {
if (error != nil) {
// Handle any errors if the token was not retrieved.
DLog(@"RNFBAppCheck - getToken - Unable to retrieve App Check token: %@", error);
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-error",
@"message" : [error localizedDescription],
}];
return;
}
if (token == nil) {
DLog(@"RNFBAppCheck - getToken - Unable to retrieve App Check token.");
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-null",
@"message" : @"no token fetched",
}];
return;
}

NSMutableDictionary *tokenResultDictionary = [NSMutableDictionary new];
tokenResultDictionary[@"token"] = token.token;
resolve(tokenResultDictionary);
}];
[appCheck tokenForcingRefresh:forceRefresh
completion:^(FIRAppCheckToken *_Nullable token, NSError *_Nullable error) {
if (error != nil) {
// Handle any errors if the token was not retrieved.
DLog(@"RNFBAppCheck - getToken - Unable to retrieve App "
@"Check token: %@",
error);
[RNFBSharedUtils
rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-error",
@"message" : [error localizedDescription],
}];
return;
}
if (token == nil) {
DLog(@"RNFBAppCheck - getToken - Unable to retrieve App "
@"Check token.");
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-null",
@"message" : @"no token fetched",
}];
return;
}

NSMutableDictionary *tokenResultDictionary = [NSMutableDictionary new];
tokenResultDictionary[@"token"] = token.token;
resolve(tokenResultDictionary);
}];
}

RCT_EXPORT_METHOD(getLimitedUseToken
Expand All @@ -140,32 +151,35 @@ + (instancetype)sharedInstance {
: (RCTPromiseRejectBlock)reject) {
FIRAppCheck *appCheck = [FIRAppCheck appCheckWithApp:firebaseApp];
DLog(@"appName %@", firebaseApp.name);
[appCheck limitedUseTokenWithCompletion:^(FIRAppCheckToken *_Nullable token,
NSError *_Nullable error) {
if (error != nil) {
// Handle any errors if the token was not retrieved.
DLog(@"RNFBAppCheck - getLimitedUseToken - Unable to retrieve App Check token: %@", error);
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-error",
@"message" : [error localizedDescription],
}];
return;
}
if (token == nil) {
DLog(@"RNFBAppCheck - getLimitedUseToken - Unable to retrieve App Check token.");
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-null",
@"message" : @"no token fetched",
}];
return;
}

NSMutableDictionary *tokenResultDictionary = [NSMutableDictionary new];
tokenResultDictionary[@"token"] = token.token;
resolve(tokenResultDictionary);
}];
[appCheck
limitedUseTokenWithCompletion:^(FIRAppCheckToken *_Nullable token, NSError *_Nullable error) {
if (error != nil) {
// Handle any errors if the token was not retrieved.
DLog(@"RNFBAppCheck - getLimitedUseToken - Unable to retrieve App Check "
@"token: %@",
error);
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-error",
@"message" : [error localizedDescription],
}];
return;
}
if (token == nil) {
DLog(@"RNFBAppCheck - getLimitedUseToken - Unable to retrieve App Check "
@"token.");
[RNFBSharedUtils rejectPromiseWithUserInfo:reject
userInfo:(NSMutableDictionary *)@{
@"code" : @"token-null",
@"message" : @"no token fetched",
}];
return;
}

NSMutableDictionary *tokenResultDictionary = [NSMutableDictionary new];
tokenResultDictionary[@"token"] = token.token;
resolve(tokenResultDictionary);
}];
}

@end
9 changes: 8 additions & 1 deletion packages/app-check/ios/RNFBAppCheck/RNFBAppCheckProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,15 @@
*
*/

#if __has_include(<Firebase/Firebase.h>)
#import <Firebase/Firebase.h>
#import <FirebaseAppCheck/FIRAppCheck.h>
#elif __has_include(<FirebaseAppCheck/FirebaseAppCheck.h>)
#import <FirebaseAppCheck/FirebaseAppCheck.h>
#import <FirebaseCore/FirebaseCore.h>
Comment on lines +18 to +22
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same questions on imports here - I note that Firebase vs FirebaseCore varies here though - I may have missed that above and that might explain the entire structure

#else
@import FirebaseCore;
@import FirebaseAppCheck;
#endif

@interface RNFBAppCheckProvider : NSObject <FIRAppCheckProvider>

Expand Down
3 changes: 2 additions & 1 deletion packages/app-distribution/RNFBAppDistribution.podspec
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'json'
require '../app/firebase_spm'
package = JSON.parse(File.read(File.join(__dir__, 'package.json')))
appPackage = JSON.parse(File.read(File.join('..', 'app', 'package.json')))

Expand Down Expand Up @@ -42,7 +43,7 @@ Pod::Spec.new do |s|
end

# Firebase dependencies
s.dependency 'Firebase/AppDistribution', firebase_sdk_version
firebase_dependency(s, firebase_sdk_version, ['FirebaseAppDistribution-Beta'], 'Firebase/AppDistribution')

if defined?($RNFirebaseAsStaticFramework)
Pod::UI.puts "#{s.name}: Using overridden static_framework value of '#{$RNFirebaseAsStaticFramework}'"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
*
*/

#if __has_include(<Firebase/Firebase.h>)
#import <Firebase/Firebase.h>
#else
@import FirebaseCore;
@import FirebaseAppDistribution;
#endif
#import <React/RCTUtils.h>

#import "RNFBApp/RNFBSharedUtils.h"
Expand Down
3 changes: 3 additions & 0 deletions packages/app/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,6 @@ android/.settings
type-test.ts
scripts
__tests__

# Force include generated version file (needed for linking)
!ios/RNFBApp/RNFBVersion.m
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this be added to the files array in app/package.json as well? .npmignore historically does work but can have subtle interaction with package.json::files element and I've had problems here before when the implied inclusion from .npmignore didn't result in the actual packaged file set that was desired

Loading
Loading