Skip to content

RU-T50 Build fix#245

Merged
ucswift merged 1 commit into
masterfrom
develop
Jun 18, 2026
Merged

RU-T50 Build fix#245
ucswift merged 1 commit into
masterfrom
develop

Conversation

@ucswift

@ucswift ucswift commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Chores
    • Updated iOS build configuration for Xcode 26+ compatibility.
    • Enabled React Native source building and Firebase static framework settings in iOS builds.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two changes extend Xcode 26+ iOS build compatibility. app.config.ts enables buildReactNativeFromSource: true in the expo-build-properties plugin. plugins/withWebRTCFrameworkFix.js gains a new Podfile patch that conditionally inserts $RNFirebaseAsStaticFramework = true after the prepare_react_native_project! anchor.

Changes

Firebase Static Framework Compatibility

Layer / File(s) Summary
Podfile injection for $RNFirebaseAsStaticFramework
plugins/withWebRTCFrameworkFix.js
Expands the plugin header comment from three to four Xcode 26+ fix classes; adds conditional patching logic that locates prepare_react_native_project! in the Podfile, throws if absent, and inserts $RNFirebaseAsStaticFramework = true immediately after it.
buildReactNativeFromSource in app config
app.config.ts
Sets buildReactNativeFromSource: true in the iOS expo-build-properties plugin block with inline comments explaining the static-framework macro re-export requirement.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • Resgrid/Unit#241: Modifies the same plugins/withWebRTCFrameworkFix.js file to patch the iOS Podfile for Xcode 26+ build issues, directly related to this PR's Podfile injection approach.
  • Resgrid/Unit#244: Also modifies plugins/withWebRTCFrameworkFix.js to address Xcode 26+ React Native Firebase iOS build problems, overlapping at the same plugin codepath.
  • Resgrid/Unit#243: Modifies plugins/withWebRTCFrameworkFix.js using the same Podfile-anchored injection pattern to insert additional Xcode/RN build settings.

Suggested reviewers

  • github-actions

🐇 A bunny hops through the build maze,
Planting flags where Firebase goes,
$RNFirebaseAsStaticFramework = true!
Now the macros bloom and Xcode glows,
No more compile errors in Podfile rows. 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'RU-T50 Build fix' is vague and generic, using a ticket reference without describing the actual changes made to the codebase. Replace with a more descriptive title that explains the specific build fixes, such as 'Enable React Native source builds and Firebase static framework configuration for iOS' or 'Fix iOS build compatibility for Xcode 26+ with RN source builds and Firebase static frameworks'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/withWebRTCFrameworkFix.js`:
- Around line 10-22: The header comment in withWebRTCFrameworkFix.js numbers the
fixes as 1, 2, 3, 4 (CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES,
use_modular_headers, RCTPromiseRejectBlock, and $RNFirebaseAsStaticFramework)
but the code comments below number them as 0, 1, 2, 3 with a different execution
order. Renumber and reorder the header comment to match the actual code
execution order: 0=$RNFirebaseAsStaticFramework, 1=use_modular_headers,
2=CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES, 3=RCTPromiseRejectBlock
patch, ensuring the description order aligns with how these fixes are actually
applied in the code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3ef6e35f-250a-4fc1-9b07-4a2c64e996de

📥 Commits

Reviewing files that changed from the base of the PR and between 5c9b3e7 and f3709c5.

📒 Files selected for processing (2)
  • app.config.ts
  • plugins/withWebRTCFrameworkFix.js

Comment on lines +10 to +22
* Fixes four classes of errors:
* 1. "include of non-modular header inside framework module" — sets
* CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES for all pods.
* 2. "declaration of 'X' must be imported from module 'Y' before it is required"
* — adds use_modular_headers! so #import statements resolve correctly across
* framework module boundaries.
* 3. "RCTPromiseRejectBlock must be imported from module 'RNFBApp.RNFBAppModule'"
* — patches RNFBMessaging source files to import RNFBAppModule explicitly.
* 4. "unknown type name 'RCT_EXTERN'" / "duplicate declaration of method
* 'RCT_CONCAT'" in RNFBMessagingModule.m (RCT_EXPORT_MODULE fails to expand)
* — sets $RNFirebaseAsStaticFramework = true so the @react-native-firebase
* pods build as static frameworks, letting React's macro headers resolve
* inside the Firebase module.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the numbering inconsistency between the header comment and code comments.

The header comment lists fixes as 1, 2, 3, 4, but the code comments below number them as 0, 1, 2, 3 (lines 31, 53, 65, 110). Additionally, the execution order doesn't match the header order:

  • Header: 1=CLANG_ALLOW, 2=use_modular_headers, 3=RCTPromiseRejectBlock, 4=$RNFirebaseAsStaticFramework
  • Code execution: 0=$RNFirebaseAsStaticFramework, 1=use_modular_headers, 2=CLANG_ALLOW, 3=RCTPromiseRejectBlock

Consider renumbering the header to match the code's execution order (0, 1, 2, 3) for clarity.

📝 Suggested fix to align numbering
- * Fixes four classes of errors:
- * 1. "include of non-modular header inside framework module" — sets
- *    CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES for all pods.
- * 2. "declaration of 'X' must be imported from module 'Y' before it is required"
- *    — adds use_modular_headers! so `#import` statements resolve correctly across
- *    framework module boundaries.
- * 3. "RCTPromiseRejectBlock must be imported from module 'RNFBApp.RNFBAppModule'"
- *    — patches RNFBMessaging source files to import RNFBAppModule explicitly.
- * 4. "unknown type name 'RCT_EXTERN'" / "duplicate declaration of method
- *    'RCT_CONCAT'" in RNFBMessagingModule.m (RCT_EXPORT_MODULE fails to expand)
+ * Fixes four classes of errors (in execution order):
+ * 0. "unknown type name 'RCT_EXTERN'" / "duplicate declaration of method
+ *    'RCT_CONCAT'" in RNFBMessagingModule.m (RCT_EXPORT_MODULE fails to expand)
  *    — sets $RNFirebaseAsStaticFramework = true so the `@react-native-firebase`
  *    pods build as static frameworks, letting React's macro headers resolve
  *    inside the Firebase module.
+ * 1. "declaration of 'X' must be imported from module 'Y' before it is required"
+ *    — adds use_modular_headers! so `#import` statements resolve correctly across
+ *    framework module boundaries.
+ * 2. "include of non-modular header inside framework module" — sets
+ *    CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = YES for all pods.
+ * 3. "RCTPromiseRejectBlock must be imported from module 'RNFBApp.RNFBAppModule'"
+ *    — patches RNFBMessaging source files to import RNFBAppModule explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/withWebRTCFrameworkFix.js` around lines 10 - 22, The header comment
in withWebRTCFrameworkFix.js numbers the fixes as 1, 2, 3, 4
(CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES, use_modular_headers,
RCTPromiseRejectBlock, and $RNFirebaseAsStaticFramework) but the code comments
below number them as 0, 1, 2, 3 with a different execution order. Renumber and
reorder the header comment to match the actual code execution order:
0=$RNFirebaseAsStaticFramework, 1=use_modular_headers,
2=CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES, 3=RCTPromiseRejectBlock
patch, ensuring the description order aligns with how these fixes are actually
applied in the code.

@ucswift

ucswift commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Approve

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR is approved.

@ucswift ucswift merged commit 0f26983 into master Jun 18, 2026
19 of 20 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.

1 participant