Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Intercom v6 for iOS 17 needed #72

Open
wcastand opened this issue Oct 16, 2023 · 29 comments
Open

Intercom v6 for iOS 17 needed #72

wcastand opened this issue Oct 16, 2023 · 29 comments

Comments

@wcastand
Copy link

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.

@GabrielDierks
Copy link
Contributor

GabrielDierks commented Nov 9, 2023

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/intercom-react-native@5.3.1"

@wcastand
Copy link
Author

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.

@GabrielDierks
Copy link
Contributor

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!

@julianpensionjar
Copy link

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.

@wcastand
Copy link
Author

wcastand commented Nov 10, 2023

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

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

@julianpensionjar
Copy link

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?

@wcastand
Copy link
Author

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

@julianpensionjar
Copy link

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/intercom-react-native@5.3.1 which appears to still support Android API level 33, but if not, I'll look at moving up to 34.

@yonitou
Copy link

yonitou commented Nov 22, 2023

Hi @wcastand ,

Does your custom plugin supports push notifications and EU region ?

@julianpensionjar
Copy link

julianpensionjar commented Nov 22, 2023

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/intercom-react-native@5.3.1 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.

@yonitou
Copy link

yonitou commented Nov 22, 2023

Hi @julianpensionjar

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

@julianpensionjar
Copy link

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).

@GabrielDierks
Copy link
Contributor

GabrielDierks commented Nov 22, 2023

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

@yonitou
Copy link

yonitou commented Nov 22, 2023

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

@julianpensionjar
Copy link

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.

@GabrielDierks
Copy link
Contributor

GabrielDierks commented Nov 22, 2023

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

This one from @cmaycumber

@julianpensionjar
Copy link

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 #72 (comment)
Are you certain no hard app crashes?!

@julianpensionjar
Copy link

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?

@GabrielDierks
Copy link
Contributor

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.

@julianpensionjar
Copy link

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.

@GabrielDierks
Copy link
Contributor

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
Copy link
Contributor

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

@julianpensionjar
Copy link

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.

@yonitou
Copy link

yonitou commented Nov 22, 2023

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 ?

@GabrielDierks
Copy link
Contributor

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 :)

@yonitou
Copy link

yonitou commented Nov 22, 2023

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

@julianpensionjar
Copy link

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!

@yonitou
Copy link

yonitou commented Nov 22, 2023

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 :(

@enagorny
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants