-
Notifications
You must be signed in to change notification settings - Fork 42
[SDK-237] Set up embedded for iOS #795
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: loren/embedded/master
Are you sure you want to change the base?
[SDK-237] Set up embedded for iOS #795
Conversation
|
Coverage Impact This PR will not change total coverage. 🚦 See full report on Qlty Cloud »🛟 Help
|
2 new issues
|
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.
Pull request overview
This PR adds embedded messaging support for iOS in the React Native SDK. The changes enable the enableEmbeddedMessaging configuration flag on iOS to match the existing Android implementation, add necessary WebView entitlements to the example app, and include a workaround for an iOS login initialization issue.
Changes:
- Added iOS support for the
enableEmbeddedMessagingconfiguration flag in the native Swift code - Added web-browser entitlements to the example iOS app to support WebView functionality
- Implemented a workaround in the example app for an iOS initialization issue
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ios/RNIterableAPI/Serialization.swift | Added deserialization of enableEmbeddedMessaging flag from config dictionary |
| example/src/hooks/useIterableApp.tsx | Added Platform import and iOS-specific error handling workaround for initialization |
| example/ios/ReactNativeSdkExample/ReactNativeSdkExample.entitlements | Created new entitlements file with web-browser capability |
| example/ios/ReactNativeSdkExample.xcodeproj/project.pbxproj | Added entitlements file reference and build configuration, plus formatting changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Platform.OS === 'ios' && getUserId()) { | ||
| setIsInitialized(true); | ||
| return login(); | ||
| } |
Copilot
AI
Jan 12, 2026
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.
This workaround silently swallows initialization errors on iOS and proceeds with login regardless of the actual error. This could mask legitimate initialization failures (such as network issues, invalid configuration, or API key problems) and lead to unpredictable behavior in production.
Instead of catching all errors and assuming initialization succeeded, consider:
- Logging the specific error for debugging purposes
- Only applying this workaround for the specific iOS initialization issue mentioned in the PR description
- Adding a comment explaining what "weird iOS login issue" this addresses
- Investigating and properly fixing the underlying iOS initialization issue rather than working around it
| OTHER_LDFLAGS = "$(inherited) "; | ||
| OTHER_LDFLAGS = ( | ||
| "$(inherited)", | ||
| " ", |
Copilot
AI
Jan 12, 2026
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 change from a string to an array with just a space character seems unusual. This formatting change appears to be converting OTHER_LDFLAGS = "$(inherited) "; to an array format with a space element.
Consider verifying if this change is intentional and necessary, or if it's an artifact from Xcode auto-formatting. If the space is not needed for linking, it would be cleaner to use just "$(inherited)" without the extra space element.
| " ", |
| OTHER_LDFLAGS = "$(inherited) "; | ||
| OTHER_LDFLAGS = ( | ||
| "$(inherited)", | ||
| " ", |
Copilot
AI
Jan 12, 2026
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 change from a string to an array with just a space character seems unusual. This formatting change appears to be converting OTHER_LDFLAGS = "$(inherited) "; to an array format with a space element.
Consider verifying if this change is intentional and necessary, or if it's an artifact from Xcode auto-formatting. If the space is not needed for linking, it would be cleaner to use just "$(inherited)" without the extra space element.
| " ", |
| if (Platform.OS === 'ios' && getUserId()) { | ||
| setIsInitialized(true); | ||
| return login(); | ||
| } |
Copilot
AI
Jan 12, 2026
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.
This change adds a missing documentation comment explaining what the workaround addresses. The PR description mentions "weird iOS login issue" but doesn't provide specifics about what the issue is or why this workaround is needed.
Add a comment here explaining:
- What specific iOS initialization issue this addresses
- Whether this is a known bug in the Iterable iOS SDK or a React Native bridge issue
- Any relevant ticket numbers or references for tracking the root cause

🔹 JIRA Ticket(s) if any
✏️ Description
Testing
Setup
yarn installyarn installbundle exec pod installwatchman watch-del-allyarn start --reset-cacheopen ReactNativeSdkExample.xcworkspace