config-plugin-react-native-intercom icon indicating copy to clipboard operation
config-plugin-react-native-intercom copied to clipboard

Intercom v6 for iOS 17 needed

Open wcastand opened this issue 1 year ago • 29 comments

https://github.com/intercom/intercom-react-native/releases/tag/6.0.0

update v6 is necessary for iOS17, getting crashes on sentry for iOS 17 users.

wcastand avatar Oct 16 '23 14:10 wcastand

Unfortunately versions 6.+ are not working with this config plugin.

On android, the key values are not set.

On iOS the Intercom presentMessageComposer seems to not work, displaying:

[UIKitCore] Unable to simultaneously satisfy constraints. Probably at least one of the constraints in the following list is one you don't want. Try this: (1) look at each constraint and try to figure out which you don't expect; (2) find the code that added the unwanted constraint or constraints and fix it. ( "<NSLayoutConstraint:0x6000023943c0 IntercomSDK_FBShimmeringView:0x13ab58000.width == 150 (active)>", "<NSLayoutConstraint:0x6000023940a0 H:|-(-17)-[UIStackView:0x12aa392c0] (active, names: '|':UIView:0x12aa2eac0 )>", "<NSLayoutConstraint:0x600002394000 UIStackView:0x12aa392c0.trailing == UIView:0x12aa2eac0.trailing + 17 (active)>", "<NSLayoutConstraint:0x600002396c60 'UISV-alignment' IntercomSDK_FBShimmeringView:0x12aa39890.leading == IntercomSDK_FBShimmeringView:0x13ab58000.leading (active)>", "<NSLayoutConstraint:0x600002396990 'UISV-canvas-connection' UIStackView:0x12aa392c0.leading == IntercomSDK_FBShimmeringView:0x12aa39890.leading (active)>", "<NSLayoutConstraint:0x600002396c10 'UISV-canvas-connection' H:[_UILayoutSpacer:0x600003da12c0'UISV-alignment-spanner']-(0)-| (active, names: '|':UIStackView:0x12aa392c0 )>", "<NSLayoutConstraint:0x600002396b70 'UISV-spanning-boundary' _UILayoutSpacer:0x600003da12c0'UISV-alignment-spanner'.trailing >= IntercomSDK_FBShimmeringView:0x13ab58000.trailing (active)>", "<NSLayoutConstraint:0x600002396d00 'UIView-Encapsulated-Layout-Width' UIView:0x12aa2eac0.width == 0 (active)>" )

As mentioned, this update is super important as it is adding support for android 14 /compileSdkVersion 34 and especially iOS 17! We also have some crashlytic reports for ios 17 devices using "config-plugin-react-native-intercom": "^1.10.1" and "@intercom/[email protected]"

GabrielDierks avatar Nov 09 '23 14:11 GabrielDierks

to be honest we use our own internal config plugin because we didn't need all the stuff this one does and we wanted to be in control when we need to update.

wcastand avatar Nov 10 '23 10:11 wcastand

Yeah that might be the best solution, but it would also makes sense to have a maintained project ideally as an official expo config plugin, since react native intercom is quite an important sdk!

GabrielDierks avatar Nov 10 '23 11:11 GabrielDierks

to be honest we use our own internal config plugin because we didn't need all the stuff this one does and we wanted to be in control when we need to update.

Anything you can share, or quick pointers to how to achieve a minimal solution for this? I'm in the same boat and have crashes with intercom 4.0.1 on iOS with Expo SDK 49.

julianpensionjar avatar Nov 10 '23 11:11 julianpensionjar

if that can help anyone https://gist.github.com/wcastand/1d70b0044cd78eeeea60f418ea121b9a

currently on "@intercom/intercom-react-native": "6.0.1",

wcastand avatar Nov 10 '23 13:11 wcastand

if that can help anyone https://gist.github.com/wcastand/1d70b0044cd78eeeea60f418ea121b9a

currently on "@intercom/intercom-react-native": "6.0.1",

Thanks @wcastand - I was able to create a new iOS development build using that gist and it seems to have stopped the crash, however the Android development build is now failing on EAS - something I don't understand in Run gradlew step:

FAILURE: Build failed with an exception.

* What went wrong:

Execution failed for task ':app:checkDebugAarMetadata'.

> A failure occurred while executing com.android.build.gradle.internal.tasks.CheckAarMetadataWorkAction

   > 9 issues were found when checking AAR metadata:
...

I need to investigate further, but did you experience anything like this?

julianpensionjar avatar Nov 10 '23 19:11 julianpensionjar

Hmm sorry don't remember having an issue on that, maybe you need to upgrade min version to 34 or something like that but honestly don't remember this error

wcastand avatar Nov 11 '23 21:11 wcastand

Hmm sorry don't remember having an issue on that, maybe you need to upgrade min version to 34 or something like that but honestly don't remember this error

Thanks...think this was a case of me not reading the Intercom release docs properly. I'm experimenting with @intercom/[email protected] which appears to still support Android API level 33, but if not, I'll look at moving up to 34.

julianpensionjar avatar Nov 13 '23 10:11 julianpensionjar

Hi @wcastand ,

Does your custom plugin supports push notifications and EU region ?

yonitou avatar Nov 22 '23 09:11 yonitou

Hmm sorry don't remember having an issue on that, maybe you need to upgrade min version to 34 or something like that but honestly don't remember this error

Thanks...think this was a case of me not reading the Intercom release docs properly. I'm experimenting with @intercom/[email protected] which appears to still support Android API level 33, but if not, I'll look at moving up to 34.

Just as update on this part of the thread - I have used a "local" config plugin using the gist above, but I couldn't find a way to make things work with @intercom/intercom-react-native6.x.x as EAS refused to build, complaining about android sdk 34, and no matter what I have tried in app.config.js around compile/target SDKs, I can't get it to build.

So, I'm currently running @intercom/intercom-react-native5.3.1, which unfortunately has the crash situation on iOS. My workaround to prevent the hard crash is the following sequence of calls:

  await Intercom.loginUnidentifiedUser()
  await Intercom.logout()
  await Intercom.setUserHash(hash)
  await Intercom.loginUserWithUserAttributes(...)
  await Intercom.updateUser(...)

The key seems to be the call to loginUnidentifiedUser() before logout. Whilst this prevents the hard crash on iOS, it does throw another non-fatal error (I think only on Android). I'm having to live with that for now, but at some point will be investigating platform-specific code to only call loginUnidentifiedUser() for iOS.

All very irritating to be honest - it would be nice to have a clean way to use 6.x.x on Expo.

julianpensionjar avatar Nov 22 '23 10:11 julianpensionjar

Hi @julianpensionjar

Thanks for the infos. Can you tell me if the gust provided by @wcastand does support Push Notifications ?

yonitou avatar Nov 22 '23 11:11 yonitou

https://github.com/intercom/intercom-react-native/releases/tag/6.0.0

update v6 is necessary for iOS17, getting crashes on sentry for iOS 17 users.

Just re-reading the original post in this thread (it which wasn't the reason I came across it). Does this mean that the 5.3.1 Intercom libraries do NOT support iOS 17?! Do you know if these cause hard app crashes, or just Sentry errors (the latter could probably be lived with if there is no other bad outcome).

julianpensionjar avatar Nov 22 '23 11:11 julianpensionjar

I can confirm we are using 5.3.1 with this plugin (the one from @cmaycumber ) and it works for iOS 17. But we have Sentry / Crashlytics errors, meaning crashes that appear on iOS 17, which lead to believe we have to update to the latest 6.2.0 to avoid those.

Example crash on 17.0.3:

Fatal Exception: NSInvalidArgumentException -[IntercomSDKPrivate.ConversationViewController contextMenuPreviewPresentationController:didChangePreviewContentSize:]: unrecognized selector sent to instance 0x11f0c5800

GabrielDierks avatar Nov 22 '23 11:11 GabrielDierks

Which plugin are you talking about @GabrielDierks ? The one from the repo or the one from @wcastand ?

yonitou avatar Nov 22 '23 11:11 yonitou

Hi @julianpensionjar

Thanks for the infos. Can you tell me if the gust provided by @wcastand does support Push Notifications ?

Sorry Intercom push notifications still on our backlog, so we haven't tried that yet.

julianpensionjar avatar Nov 22 '23 11:11 julianpensionjar

Which plugin are you talking about @GabrielDierks ? The one from the repo or the one from @wcastand ?

This one from @cmaycumber

GabrielDierks avatar Nov 22 '23 11:11 GabrielDierks

I can confirm we are using 5.3.1 with this plugin and it works for iOS 17. But we have Sentry / Crashlytics errors, meaning crashes that appear on iOS 17, which lead to believe we have to update to the latest 6.2.0 to avoid those.

Example crash on 17.0.3:

Fatal Exception: NSInvalidArgumentException -[IntercomSDKPrivate.ConversationViewController contextMenuPreviewPresentationController:didChangePreviewContentSize:]: unrecognized selector sent to instance 0x11f0c5800

Hmm, these certainly look like the same messages as the hard app crashed on 5.3.1 (without the work-around function sequence I posted in https://github.com/cmaycumber/config-plugin-react-native-intercom/issues/72#issuecomment-1822549085 Are you certain no hard app crashes?!

julianpensionjar avatar Nov 22 '23 11:11 julianpensionjar

Which plugin are you talking about @GabrielDierks ? The one from the repo or the one from @wcastand ?

This one from @cmaycumber

Are you able to give us a complete view of your setup, things such as:

  • Using Expo? And if so which SDK version
  • Version of the @cmaycumber config plugin
  • Sequence of function calls you make to Intercom libs (on 5.3.1)
  • Anything else relevant?

julianpensionjar avatar Nov 22 '23 11:11 julianpensionjar

No as I mentioned we do have hard app crashes and this was one example of a crash. But for most of our customers on iOS 17 it works fine and we could not reproduce why these crashes appear apart from not having updated the react-native-intercom package.

GabrielDierks avatar Nov 22 '23 11:11 GabrielDierks

No as I mentioned we do have hard app crashes and this was one example of a crash. But for most of our customers on iOS 17 it works fine and we could not reproduce why these crashes appear apart from not having updated the react-native-intercom package.

Ah ok - sorry, misunderstood you! So, it sounds like you're in the same boat as me. You might want to try the work-around sequence of calls I posted above - so far this has avoided the crashes on iOS, although I need to check for explicit testing on iOS 17. It does however produce other non-fatal crashes, which seem to be exclusively on Android.

julianpensionjar avatar Nov 22 '23 12:11 julianpensionjar

Which plugin are you talking about @GabrielDierks ? The one from the repo or the one from @wcastand ?

This one from @cmaycumber

Are you able to give us a complete view of your setup, things such as:

  • Using Expo? And if so which SDK version
  • Version of the @cmaycumber config plugin
  • Sequence of function calls you make to Intercom libs (on 5.3.1)
  • Anything else relevant?

Sure:

  • Expo managed workflow with dev-client with following versions:

    "expo": "^49.0.11", "react-native": "^0.72.5", "config-plugin-react-native-intercom": "^1.5.0", "@intercom/intercom-react-native": "^5.3.1",

  • App.json integration:

[ "config-plugin-react-native-intercom", { "iosApiKey": "<your-api-key>", "androidApiKey": "<your-api-key>", "appId": "<your-app-id>", "isPushNotificationsEnabledIOS": true, "isPushNotificationsEnabledAndroid": true, "intercomEURegion": "true" } ],

  • Order of calls:

await Intercom.loginUnidentifiedUser(); await Intercom.sendTokenToIntercom(newToken);

on refresh token:

await Intercom.setUserHash(data.token); await Intercom.loginUserWithUserAttributes({ email, userId, }); await Intercom.updateUser({ ..props });

GabrielDierks avatar Nov 22 '23 12:11 GabrielDierks

Seems like adding a logout prior to the hash could improve our experience 😎

GabrielDierks avatar Nov 22 '23 12:11 GabrielDierks

Seems like adding a logout prior to the hash could improve our experience 😎

Possibly, although my suspicion is that there is race condition in the underlying libs - we get the crashes when calling logout first. One issue is there is no way (that I know of at least) to know what the state is in the lib - i.e. you can't ask "is there already a user", and if so call logout. We had to try and design the code so we definitively knew whether there is already a user, hence the sequence I posted - first force an unidentified user (so we know we have one), then logout, then login the real (Identified) user.

julianpensionjar avatar Nov 22 '23 12:11 julianpensionjar

Does a patch package on this plugin would be enough ? So we just update the version of intercom to v6 ? Or do we need to make other changes in the config plugin to make it work ? Also, if you use the v4, do you have the same crashes in iOS 17 ?

yonitou avatar Nov 22 '23 13:11 yonitou

Does a patch package on this plugin would be enough ? So we just update the version of intercom to v6 ? Or do we need to make other changes in the config plugin to make it work ? Also, if you use the v4, do you have the same crashes in iOS 17 ?

As I stated at the beginning, changing the version to 6.0.0+ breaks opening the messenger using this plugin. I created a request for the expo team here to make this an official config plugin to get more maintainers, which I also think was in @cmaycumber s interest! Would be great to have some +1's there :)

GabrielDierks avatar Nov 22 '23 16:11 GabrielDierks

On my side, I have a big issue, everytime I'm launching my app on android (v31) I'm experiencing this message and a crash : TypeError: Cannot read property 'UNREAD_COUNT_CHANGE_NOTIFICATION' of null, js engine: hermes

Did you manage to solve it ? It's driving me nuts

yonitou avatar Nov 22 '23 17:11 yonitou

On my side, I have a big issue, everytime I'm launching my app on android (v31) I'm experiencing this message and a crash : TypeError: Cannot read property 'UNREAD_COUNT_CHANGE_NOTIFICATION' of null, js engine: hermes

Did you manage to solve it ? It's driving me nuts

I feel like I've seen that at some point when going through the pain of upgrading the intercom libsraries..this could way off, but I seem to recall there were some changes in the naming of event listeners at some point that I had to tweak. e.g. if you add an event listener to handle a change in the number of unread messages:

Intercom.addEventListener('IntercomUnreadConversationCountDidChangeNotification', callback)

I think it was the exact string being passed that was different - I can see in commit history I previously had to switch on platform between:

  • IOS: 'IntercomUnreadConversationCountDidChangeNotification'
  • Android: 'IntercomUnreadCountDidChange' I think it is the same for both now (the first one).

Hope it helps!

julianpensionjar avatar Nov 22 '23 17:11 julianpensionjar

I'm not using it :/ I think it's related to the v5 version because I'm using a v31 Android. It only works if I remove all imports of Intercom. What a mess really ... How such a company can have such a poor support on mobile integration :(

yonitou avatar Nov 22 '23 18:11 yonitou

I've put a PR in expo/config-plugins . The solution is based on @wcastand gist and extended to support regions. Push notifications aren't there yet.

enagorny avatar Nov 24 '23 16:11 enagorny