expo / config-plugins

Out-of-tree Expo config plugins for packages that haven't adopted the config plugin system yet.
427 stars 91 forks source link

@config-plugins/react-native-branch Support for Expo SDK 50 #215

Closed alfjesus closed 4 months ago

alfjesus commented 5 months ago

Summary

Hi! Thanks for the great new release of Expo SDK 50. Unfortunately, the @config-plugins/react-native-branch doesn't seem to work properly on iOS. Works with no issue on SDK 49. I'm no export with config plugins but what I can see is that the ExpoModulesProvider.swift file doesn't get included properly when running the expo prebuild with @config-plugins/react-native-branch on Expo SDK 50.

on Expo SDK 49 image

on Expo SDK 50 ExpoModulesProvider.swift is missing image

Config Plugin

@config-plugins/react-native-branch

What platform(s) does this occur on?

iOS

SDK Version

50

Reproducible demo

  1. npx create-expo-app my-branch-app
  2. yarn add @config-plugins/react-native-branch
  3. add config plugin to app.config.js
    [
    '@config-plugins/react-native-branch',
        {
        apiKey: 'API_KEY',
        iosAppDomain: 'some.app.link',
      },
    ]
  4. npx expo prebuild -p ios --clean
fobos531 commented 5 months ago

Hello @alfjesus

Just as a heads up, there's also a compilation error on Android, for which I've submitted a PR to fix it: https://github.com/expo/config-plugins/pull/216

alfjesus commented 5 months ago

Hello @alfjesus

Just as a heads up, there's also a compilation error on Android, for which I've submitted a PR to fix it: #216

Yep, I experienced this as well. I have fixed this with a patch and it now compiles successfully on android 🙂 image

macsinte commented 4 months ago

@alfjesus I was able to build everything and make it work with Expo SDK 50 for iOS. As a matter a fact I didn't even notice any issues with it.

I did however experience failed builds with Android, and I fixed that with the patch that you mentioned above, which works like a charm.

What issues did you run into for iOS? Maybe I can help.

alfjesus commented 4 months ago

@macsint Thanks for you message. Would love some help. Sounds promising that you got react-native-branch to work on iOS and Expo SDK 50. I don't have any build/compile issues on iOS. Everything builds and completes successfully. However, when I test a branch.io deep link... the branch.subscribe() doesn't process the link... actually the branch.subscribe() have not been called at all the when the app starts. I don't seen any other error logs.

But I do see the branch library to load in the console logs. image

Worth mentioning... I'm using branch.io together with React Navigation with the linking prop in the NavigationContainer

macsinte commented 4 months ago

@alfjesus Ok so I guess I am personally doing things a little bit more differently in terms of processing incoming links:

 // handle incoming deep-links through the latest referring params of the Branch SDK
            branch !== null && branch.default !== null && !latestReferringParamsChecked && branch.default.getLatestReferringParams(false).then(async params => {
      // process these params and handle the link accordingly
});

This is how I currently do it and it seems to be working well, I've been doing this since I started implementing Branch in Expo SDK 48, which is what we were using before migrating to 50.

macsinte commented 4 months ago

@alfjesus Also as a side note, we probably have a success rate with processing incoming Branch links by doing it this way of like an 8-9 out of 10. Meaning that rarely we see a link not getting processed. Don't think that this is an issue with Expo SDK 50, as we were encountering that issue even before.

That's actually something I meant to ask you as well, if before 50 you saw a success rate of 100% in terms of handling and detecting incoming links?

alfjesus commented 4 months ago

@macsint Thanks for your input. However, I think I need a bit more context of your implementation to fully understand how you have done this.

I have been using branch.subscribe() in the Linking prop of the NavigationContainer with Expo 49 with no problem (migrated from Firebase Dynamic links). I would say that this, so far, has had a success rate of 100%. When I upgrade to Expo 50... the branch.subscribe() doesn't seem to called 🤷‍♂️. Unfortunately I get no error logs.

macsinte commented 4 months ago

@alfjesus to be honest with you the branch subscribe call never worked for me (I'm assuming that I'm doing something wrong here). I have multiple navigation containers throughout my app, and I use getLatestReferringParams in my Auth screen which is essentially one of the screens that I have as a possible route coming from my App root (the other ones being Registration and just an Overview screen).

That way essentially our logic is to capture any branch deep links and set our App URL param source in our global state accordingly, so after the user authenticates our app will route properly.

macsinte commented 4 months ago

@alfjesus just out of curiosity, would you mind pasting an example of your branch subscribe call and maybe tell us more in terms of where you're using it? I can try to do the same in our implementation and tell you if I can get it to work in Expo SDK 50.

alfjesus commented 4 months ago

@macsint My implementation follows the React Navigation docs https://reactnavigation.org/docs/deep-linking/#third-party-integrations But in the subscribe(listner) I have my branch.subscribe() as follows:

subscribe(listener) {
    const branchUnsubscribe = branch.subscribe({
      onOpenStart: ({ uri, cachedInitialEvent }) => {
        LOG.debug(
          'subscribe onOpenStart, will open ' +
            uri +
            ' cachedInitialEvent is ' +
            cachedInitialEvent
        );
      },
      onOpenComplete: ({ error, params, uri }) => {
        LOG.info('params: ', params);
        if (error) {
          LOG.error(
            'subscribe onOpenComplete, Error from opening uri: ' +
              uri +
              ' error: ' +
              error
          );
          return;
        } else if (params) {
          if (!params['+clicked_branch_link']) {
            if (params['+non_branch_link']) {
              LOG.debug('non_branch_link: ' + uri);
              // Route based on non-Branch links
              listener(params['+non_branch_link']);
              return;
            }
          } else {
            // Handle params
            LOG.debug('uri: ', uri);
            const deepLinkPath = params.$deeplink_path as string;
            const canonicalUrl = params.$canonical_url as string;
            LOG.debug('deepLinkPath: ', deepLinkPath);
            LOG.debug('canonicalUrl: ', canonicalUrl);
            // Route based on Branch link data
            listener(canonicalUrl);
            return;
          }
        }
      },
    });
return () => {
      // Clean up the event listeners
      branchUnsubscribe();
    };
  },

Then I include the linking object in the navigation container as:

const RootStack = () => {
  const isLoggedIn = useAppSelector((state) => state.auth.isLoggedIn);
  return (
    <SafeAreaProvider>
      <NavigationContainer
        linking={isLoggedIn ? linking : undefined}
        fallback={<ActivityIndicator color="blue" size="large" />}
        theme={MyTheme}
      >
        <RootNavigator />
      </NavigationContainer>
    </SafeAreaProvider>
  );
};

To get it work properly it's important to configure the config prop in the linking object to reflect your screen setup. Mine is for example

config: {
    screens: {
      MainStack: {
        screens: {
          FeedDetailPost: 'post/:postID',
          FeedPlantInfo: 'plant/:plantID',
          FeedProfile: 'profile/:profileID',
          FeedCollectionView: 'collection/:userId/:collectionId',
        },
      },
    },
  },

Let me know if you have any more questions

macsinte commented 4 months ago

Hey @alfjesus got it. I added the branch.subscribe({)} code block under our getLatestReferringParams block, and I introduced this as part of our latest release that's currently pending review approval on the App Stores.

I checked that this works as expected for Android (through an APK test), in that the branch subscription block works as expected, but it does not really work well in Test Flight for iOS. I noticed that even before Test Flight branch deep linking was not working well (or at all), so once we get approved for the App Store I'll let you know how that goes.

Also, you are right in using subscribe the way that you are by following the deep-linking docs. We had a period of time when we were doing linking exactly like that, but now we reverted to handling this manually.

We essentially have a global state param for appURL as mentioned before, and we are handling that accordingly instead of handling it inside of the linking block like you are. Not sure if that makes a difference in terms of how well branch.subscribe works or not, but if you are saying that it worked for previous Expo versions, then there really must be a bug for this latest Expo version in regards to it.

FYI we use Recoil for our state management throughout the app.

macsinte commented 4 months ago

@alfjesus I can confirm this issue as well on iOS for Expo SDK 50 for our App/Team as well. For some reason this was working for us in a development build but it is not working in production.

I deployed the app in the App Store and links are not working as expected anymore.

As I mentioned before, Android seems to be working as expected from our APK testing, but I'll wait on our latest Android version update to confirm (my hunch is that it'll work as expected).

alfjesus commented 4 months ago

@alfjesus I can confirm this issue as well on iOS for Expo SDK 50 for our App/Team as well. For some reason this was working for us in a development build but it is not working in production.

I deployed the app in the App Store and links are not working as expected anymore.

As I mentioned before, Android seems to be working as expected from our APK testing, but I'll wait on our latest Android version update to confirm (my hunch is that it'll work as expected).

Sorry to hear that, but it somehow feels good that I'm not the only one struggling with react-native-branch and Expo 50. At this point I can't tell if the root cause relies in the config plugin or the library itself. I hope someone can figure this out.

akkilaa commented 4 months ago

@alfjesus, I've experienced similar issues while integrating react-native-branch with react-navigation. Specifically, I found that Branch's subscribe method wasn't functioning properly. It could handle links when the app was running in the background, but failed to trigger when the app was off and a link was used to open the app.

To address this, I used two methods from the linking object in the NavigationContainer: getInitialURL and subscribe.

In the getInitialURL method, I implemented a call to getLatestReferringParams. This has been effective for managing links when the app is initially opened via a link and wasn't previously running.

As for the subscribe method in linking, I utilized Branch's subscribe method. It successfully triggers when the app is running in the background and a link is opened.

alfjesus commented 4 months ago

@alfjesus, I've experienced similar issues while integrating react-native-branch with react-navigation. Specifically, I found that Branch's subscribe method wasn't functioning properly. It could handle links when the app was running in the background, but failed to trigger when the app was off and a link was used to open the app.

To address this, I used two methods from the linking object in the NavigationContainer: getInitialURL and subscribe.

In the getInitialURL method, I implemented a call to getLatestReferringParams. This has been effective for managing links when the app is initially opened via a link and wasn't previously running.

As for the subscribe method in linking, I utilized Branch's subscribe method. It successfully triggers when the app is running in the background and a link is opened.

@akkilaa Okay, sounds that you are using the linking object in the same way as me. With the difference that the branch.subscribe doesn't get called inside the Linking.subscribe 🤷‍♂️

alfjesus commented 4 months ago

With the release of 7.0.0 of @config-plugins/react-native-branch I can confirm that react-native-branch works again on Expo 50 👏

Closing this issue.

jvgeee commented 6 days ago

Not sure if this is the best place to post this, but I'm having issues with RN branch and the expo config plugin.

My issue: Branch links correctly open my app, but my listeners can never get any data out of the link. @alfjesus I'm using pretty much the exact same code as you, but onOpenComplete never gets called.

Any ideas on what might be going on here?

(am on "@config-plugins/react-native-branch": "^8.0.0", and Expo 51

Update: This was an issue with the API key, I was using a test api key and this config plugin only allows for a live api key - this should be documented / fixed!

alfjesus commented 6 days ago

@jvgeee Right now I actually don't know since I haven't upgraded to SDK 51 yet. Last time there was an issue with this config plugin, it was fixed once a new version of the plugin was released. Maybe the current config plugin is right now only compatible with SDK 50?

Honestly, I'm thinking of moving away from branch.io and start using expo router with deep linking support 🤷‍♂️

jvgeee commented 5 days ago

@alfjesus Yeah it turned out it works fine with the current Branch / Expo versions, it's just that I was using a test key instead of live key for the Branch backend and the config plugin wasn't compatible with that.

We're only using Branch because of its deferred deep linking support, I'd prefer other providers for this too!

alfjesus commented 5 days ago

@jvgeee Yep.. if you are dependent on support for deferred deep links... I guess Branch is the only option right now