expo / config-plugins

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

feat: create react-native-branch plugin #28

Closed giautm closed 2 years ago

giautm commented 2 years ago

Close https://github.com/expo/expo/issues/12906

giautm commented 2 years ago

Hey @EvanBacon, Can you look a bit into this PR? Thank you,

giautm commented 2 years ago

In our apps, I still using the old plugin with CodeMod instead of the new expo module system. I accept the risk if they conflict (but they aren't). So, just take the time to complete this plugin as we expect. No rush. :smile:

giautm commented 2 years ago

I have been tested this plugin on our production app and it working fine. But, you can do internal testing before releasing it.

Just to remind you, we still locking by https://github.com/BranchMetrics/react-native-branch-deep-linking-attribution/pull/683 to be able to build the iOS app. Workaround by patch-package in this PRs.

Kudo commented 2 years ago

the pr looks really well to me. thanks @giautm for the awesome work. let's see when the react-native-branch pr getting landed.

Aryk commented 2 years ago

Branch support is the main thing blocking me from trying EAS. Is it possible to use it from the giautm/branch? Or will this get merged and released soon?

@giautm Just curious, if this doesn't get merged, would this config plugin still work for EAS? It doesn't seem to be getting merged any time soon.

cc @Kudo if you have any ideas how I might be able to get react-native-branch working on EAS until this config plugin is all sorted out.

gerwim commented 2 years ago

@Aryk I've used @giautm's code and Branch works on EAS. What you need to do is patching the 5.2.1 version manually (see https://github.com/giautm/config-plugins/commit/e52e49f70f27e42f43403777c4d9fcad963e26de) and install giatum's config plugin: https://www.npmjs.com/package/@giautm/react-native-branch.

dmitryame commented 2 years ago

@Aryk I've used @giautm's code and Branch works on EAS. What you need to do is patching the 5.2.1 version manually (see giautm@e52e49f) and install giatum's config plugin: https://www.npmjs.com/package/@giautm/react-native-branch.

I'm confused, are we supposed to be using react-native-branch (and configure the plugin) going forward, or expo-branch (which is referenced in expo documentation)?

gerwim commented 2 years ago

@dmitryame expo-branch is not supported in EAS (see https://docs.expo.dev/versions/latest/sdk/branch/).

Aryk commented 2 years ago

@gerwim so if I'm understanding correctly, what I need to do is:

  1. Add "@giautm/react-native-branch"
  2. Make sure "react-native-branch" dependency has 5.2.1 version specified
  3. Add this file to my patches folder and make sure patch-package runs after calling 'yarn'.
  4. Remove from my app.json the apiKey from "android" and "ios" sections for branch and...
  5. Add to my app.json
    {
    "expo": {
    "plugins": [
      [
        "@config-plugins/react-native-branch",
        {
          "apiKey": "key_live_f9f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8",
        }
      ]
    ]
    }
    }
  6. Build with EAS

Is that right? Anything I'm missing?

gerwim commented 2 years ago

@Aryk That's about right. You still need to add your key to the plugins config. See the example here: https://github.com/giautm/config-plugins/tree/giautm/branch/packages/react-native-branch#example

Aryk commented 2 years ago

I guess it's confusing why we need to specify the api_key a third time in the app.json. I guess we don't need it in the other places? Also it says it's optional there...but I guess it's required? Maybe should be changed to say "required"?

gerwim commented 2 years ago

I guess it's confusing why we need to specify the api_key a third time in the app.json. I guess we don't need it in the other places? Also it says it's optional there...but I guess it's required? Maybe should be changed to say "required"?

I'm not sure, I have not done extensive testing. I've just read @giautm's code and his example :).

giautm commented 2 years ago

This commit respects old-config from expo-branch from app.json, so you don't have to put it in the plugin config.

Bellow config with work if you ready have the expo-branch's config in app.json. If not, it will show the error about missing config

{
  "expo": {
    "plugins": ["@config-plugins/react-native-branch"]
  }
}
Aryk commented 2 years ago

Thanks for that clarification @giautm !

Btw, does what I wrote in the comment look good to you? https://github.com/expo/config-plugins/pull/28#issuecomment-1013433576

giautm commented 2 years ago

Thanks for that clarification @giautm !

Btw, does what I wrote in the comment look good to you? https://github.com/expo/config-plugins/pull/28#issuecomment-1013433576

Yes, that's correct.

Aryk commented 2 years ago

I was getting an error when running with

{
  "expo": {
    "plugins": [
      [
        "@config-plugins/react-native-branch",
        {
          "apiKey": "key_live_f9f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8",
        }
      ]
    ]
  }
}

and had to change it to:

{
  "expo": {
    "plugins": [
      [
        "@giautm/react-native-branch",
        {
          "apiKey": "key_live_f9f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8",
        }
      ]
    ]
  }
}

@giautm is that right?

gerwim commented 2 years ago

@Aryk That is right. You have installed the package @giautm/react-native-branch ;-). Once this PR is merged, you should install (and use) @config-plugins/react-native-branch instead.

Aryk commented 2 years ago

@giautm @gerwim I was able to get EAS working and followed the instructions here: https://github.com/expo/config-plugins/pull/28#issuecomment-1013433576.

Branch.subscribe does not run at all on a cold-start when the app is killed on iOS (Android works fine). It does run when foregrounded however on iOS. I have the Branch.subscribe call in a useEffect that runs immediately when the top level component of the app is called. I even tried putting it outside so it runs when the app loads in. On the initial bootup Branch does not check at all for a deep link, but works fine once the app is already opened.If I background the app and click a deep link it will work.

Aryk commented 2 years ago

@giautm @gerwim I think this is the issue: https://github.com/BranchMetrics/ios-branch-deep-linking-attribution/issues/1160

Is there anyway to incorporate that code into this fix?

giautm commented 2 years ago

@giautm @gerwim I think this is the issue: https://github.com/BranchMetrics/ios-branch-deep-linking-attribution/issues/1160

Is there anyway to incorporate that code into this fix?

Please share your code. In our apps, it working fine even on the cold-start.

Aryk commented 2 years ago

This works when i foreground the app in that I get some kind of Alert which means the code inside subscribe was run. On a cold start I do not get an Alert at all which means that the subscribe was not run at all.

Let me know what other code you'd like me to share. I also tried putting the Branch subscribe on the outside of the component and got the same behavior.

const Branch = constants.inExpo ? undefined : require("react-native-branch").default;
export default () => {

  useEffect(
    () => Branch?.subscribe(({error, params}) => {
      Alert.alert(undefined, JSON.stringify({params, error}))
    }),
    []
  );

  return <View>
    <Text>
      Hi there
    </Text>
  </View>
}

This package added: https://github.com/giautm/config-plugins/commit/e52e49f70f27e42f43403777c4d9fcad963e26de#diff-34c2e6e73cbb4fc9ecfb1932a2581f8cde2ae3b6486bc4b65625256ede2e0bcc

In package.json:

"@giautm/react-native-branch": "^0.0.9",
    "react-native-branch": "^5.2.1",

In app.json:


{
  "expo": {
    "scheme": "tribefy",
    "slug": "tribefy",
    "owner": "tribefy",
    "name": "Tribefy",
    "version": "2.0",
    "userInterfaceStyle": "automatic",
    "updates": {
      "enabled": true,
      "checkAutomatically": "ON_LOAD"
    },
    "orientation": "portrait",
    "primaryColor": "#cccccc",
    "packagerOpts": {},
    "platforms": [
      "android",
      "ios"
    ],
    "assetBundlePatterns": [
      "assets/store_local/**/*"
    ],
    "splash": {
      "image": "./assets/app_json/splash-screen.png",
      "resizeMode": "cover",
      "backgroundColor": "#F26566"
    },
    "androidStatusBar": {
      "barStyle": "light-content",
      "translucent": true
    },
    "notification": {
      "iosDisplayInForeground": false,
      "androidMode": "default",
      "icon": "./assets/app_json/android-tray-grayscale-transparent.png",
      "color": "#F26566"
    },
    "icon": "./assets/app_json/app-icon.png",
    "plugins": [
      "sentry-expo",
      "./with-firebase-automatic-screen-reporting-disabled",
      [
        "@giautm/react-native-branch",
        {
          "apiKey": "key_live_XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
          "iosAppDomain": "join.tribefy.com"
        }
      ]
    ],
    "hooks": {
      "postPublish": [
        {
          "file": "sentry-expo/upload-sourcemaps",
          "config": {
            "organization": "tribefy",
            "project": "tribefy-react-native",
            "authToken": "authtoken"
          }
        }
      ]
    }
  }
}
giautm commented 2 years ago

I do the same setup as your code. So, if you need help, please DM me via telegram/Twitter.

Screen Shot 2022-01-20 at 15 02 12
gerwim commented 2 years ago

@giautm @gerwim I was able to get EAS working and followed the instructions here: #28 (comment).

Branch.subscribe does not run at all on a cold-start when the app is killed on iOS (Android works fine). It does run when foregrounded however on iOS. I have the Branch.subscribe call in a useEffect that runs immediately when the top level component of the app is called. I even tried putting it outside so it runs when the app loads in. On the initial bootup Branch does not check at all for a deep link, but works fine once the app is already opened.If I background the app and click a deep link it will work.

We've conducted extensive testing this week on Branch and can confirm: iOS does not work from cold start and Android works perfect.

Our code is exactly like @Aryk's.

It could also just be a Branch issue:

Aryk commented 2 years ago

@gerwim It's good to know that I'm not crazy here. I have no clue if this was introduced in the newer code or if it's something from branch's servers but based on the people complaining about it, seems like this issue has been around in some shape or form for the past few years.

@giautm, it's interesting that you haven't encountered this issue. Do you have iOS test devices like the "iphone X" or "iphone 8" and they work from a cold start?

giautm commented 2 years ago

Yes, I tested on a real device - an iPhone 12 mini without any issue. Can you share a video for your testing?

P/s: I really to discuss this issue, but it looks off-topic. So, we move to somewhere else?

Aryk commented 2 years ago

@giautm sure, happy to move anywhere else.

Here is a video of the issue: https://www.dropbox.com/s/sztm8t9wp7550ic/branch_issue.MP4?dl=0

This is my entire app:

const Branch = constants.inExpo ? undefined : require("react-native-branch").default;
export default () => {

  useEffect(
    () => Branch?.subscribe(({error, params}) => {
      Alert.alert(undefined, JSON.stringify({params, error}))
    }),
    []
  );

  return <View>
    <Text>
      Hi there
    </Text>
  </View>
}

if subscribe was working, I'd atleast get an Alert message showing that it executed the subscribe callback but I get nothing when I open the app.

Happy to take this discussion elsewhere, wherever is most convenient for you. Even happy to zoom so I can screenshare if that will help us diagnose this faster.

giautm commented 2 years ago

Here is my testing, note I don't need to press the Download button, just reload the page and it works.

PS: react-native-branch version is 5.2.1

https://user-images.githubusercontent.com/12751435/150324832-9e965a7f-f867-482e-82ed-05f29727f107.MOV

Aryk commented 2 years ago

Yup I also have the 5.2.1 version. That's great that it works for you. I'm not really sure what else to try given these issues. Maybe it will work for other users and not my phone. @gerwim please keep me posted if you guys figure out the issue. Maybe it's a setting somewhere in the branch console.

giautm commented 2 years ago

Yup I also have the 5.2.1 version. That's great that it works for you. I'm not really sure what else to try given these issues. Maybe it will work for other users and not my phone. @gerwim please keep me posted if you guys figure out the issue. Maybe it's a setting somewhere in the branch console.

Please check this config, do you have the same config as me?

Screen Shot 2022-01-20 at 20 22 02
arjen1981 commented 2 years ago

Hello I'm a colleague of @gerwim,

@giautm we had our URI Scheme Deep Link Mode set to Intelligent mode but we see no changes setting the settings exactly like yours.

@Aryk We have tested the situation with several iPhones and all give us the same issue with branch.subscribe( not being hit on a cold start. It works all the time on Android.

We also tried setting branch.initSessionTtl = 10000; right before the subscribe but this didn't resolve anything either.

We are also on:

"@giautm/react-native-branch": "^0.0.9", "react-native-branch": "5.2.1"

Which expo SDK are you both on @giautm @Aryk? We are on SKD 44

"expo": "^44.0.0",

And what IOS versions are installed on your phones?

There must be something that is different between our setups?

giautm commented 2 years ago

When was the last time you built the app?

I think it may related to this issue. Evan just said the templated was updated few days ago. Please re-build your app, or run 'expo run:ios' to verify.

https://github.com/expo/expo/pull/15684

Ps: my device running iOS 15.2.1

Aryk commented 2 years ago

@giautm I think I rebuilt the app like 5 times in the last 24 hours, so I don't think it's the template. I also had eas-cli updated as well to eas-cli/0.45.1 darwin-x64 node-v16.13.1. Maybe try it on phone with 15.2. That could be the reason yours is working.

@arjen1981 - Thanks for checking on this. This is literally the last thing I need to fix before I can deploy and would love to resolve this quickly. 👍

I'm also using "Conversative Mode" but I don't have the UTM trackers on. I just tried turning that on, retested and still broken.

I have two iPhones: Phones: iOS 15.2.1 - iPhone 8 and X SDK 44 Managed "expo": "^44.0.0", : Branch.initSessionTtl = 20000; "@giautm/react-native-branch": "^0.0.9", "react-native-branch": "5.2.1"

@giautm - Is there anything I need to do to get the new template besides just building? Maybe a library to upgrade? If others also have it working on iOS from a cold-start, maybe you guys can chime in as I don't think there are a material number affected with this issue.

giautm commented 2 years ago

Is there anything I need to do to get the new template besides just building?

You can verify that you have effect by the template issue or not, just running expo run:ios in local to build native app and run it on your device.

Aryk commented 2 years ago

@arjen1981 @giautm . One last question here. It started working for me after a build. I "think" i know what it could be. Can you guys report what you have in app.json for fallbackToCacheTimeout on updates? Do you have the key defined? I'm willing to bet that @giautm does not and @arjen1981 does... let's see if I'm right. :)

giautm commented 2 years ago

fallbackToCacheTimeout

{
"updates": {
"fallbackToCacheTimeout": 0
}
}
arjen1981 commented 2 years ago

It is now set to 30 seconds.

    "updates": {
      "fallbackToCacheTimeout": 30000
    },

I will try this out.

arjen1981 commented 2 years ago

@Aryk you're absolutely right! We fixed the problem by setting fallbackToCacheTimeout to 0. Our branch.subscribe is being hit now. I also tried setting the value of initSessionTtl 10 seconds higher than the fallbackToCacheTimeout but that didn't help. We would like to activate fallbackToCacheTimeout in the future, but this is a good workaround for now. Thanks everyone!

Aryk commented 2 years ago

Yeah, I mean it's totally obvious that that would have fixed it right? 😜

https://github.com/expo/expo/issues/15443#issuecomment-996307303

I only knew to check this bc it caused issues with expo-branch. Maybe someone from expo can either fix it or atleast add in expo-branch as a note:

fallbackToCacheTimeout could cause issues with expo-branch and react-native-branch. If you are experiencing issues with branch, try removing this key or setting it to zero.

I'm willing to bet that adding it atleast temporarily to the documentation would save other poor souls a cumulative 100 hours of time wasted. I, alone, probably wasted 10 hours picking apart my app figuring this out.

cc @Kudo @brentvatne

Dakuan commented 2 years ago

think this'll be merged soon?

Dakuan commented 2 years ago

I attempted to run it from https://www.npmjs.com/package/@giautm/react-native-branch but get

The Swift pod `ExpoAdapterBranch` depends upon `react-native-branch`, which does not define modules. To opt into those targets generating module maps (which is necessary to import them from Swift when building as static libraries), you may set `use_modular_headers!` globally in your Podfile, or specify `:modular_headers => true` for particular dependencies.
[stderr] [!] `<PBXResourcesBuildPhase UUID=`13B07F8E1A680F5B00A75B9A`>` attempted to initialize an object with an unknown UUID. `C310C450CCD04E1EAEF00263` for attribute: `files`. This can be the result of a merge and the unknown UUID is being discarded.
pod exited with non-zero code: 1
gerwim commented 2 years ago

@Dakuan Looks like you didn't apply the custom patch. It merged recently upstream, but for now please check https://github.com/expo/config-plugins/pull/28#issuecomment-1013433576.

giautm commented 2 years ago

react-native-branch just released a new version (5.3.0) a few hours ago, so we don't need to patch it anymore. Now, this PR is ready to release.

cc @EvanBacon

giautm commented 2 years ago

thanks for the awesome pr 🔥

I need to learn more, thanks for the help, ❤️

fparhat-rbi commented 2 years ago

Quick question guys, @giautm

Going over the docs it states:

This module only works in standalone apps on the classic build service (expo build). It is not compatible with EAS Build. Work is in progress to add managed EAS Build support for react-native-branch. Once this is ready, expo-branch will be deprecated.

And points to this PR. Is this PR fixing expo-branch to work in EAS builds or should we wait and use something else after this PR is checked in for enabling Branch in our expo app?

giautm commented 2 years ago

Is this PR fixing expo-branch to work in EAS builds or should we wait and use something else after this PR is checked in for enabling Branch in our expo app?

This PR creates a new plugin for react-native-branch, so you can directly use that module.

mersiades commented 2 years ago

Thanks for the work you've all put into this plugin (especially @giautm !)

I tried to add it to my app today, but eas build -p ios --profile development failed (Android is good, though).

The Swift pod `ExpoAdapterBranch` depends upon `react-native-branch`, which does not define modules. To opt into those targets generating module maps (which is necessary to import them from Swift when building as static libraries), you may set `use_modular_headers!` globally in your Podfile, or specify `:modular_headers => true` for particular dependencies.
[stderr] [!] use_native_modules! skipped the react-native dependency 'react-native-locale-detector'. No podspec file was found.
[stderr] 
[stderr] [!] `<PBXResourcesBuildPhase UUID=`13B07F8E1A680F5B00A75B9A`>` attempted to initialize an object with an unknown UUID. `2C32E4B609FD4864BC08B4CF` for attribute: `files`. This can be the result of a merge and the unknown UUID is being discarded.

From what I've read above, I would expect that error if I was using an older version of react-native-branch (or hadn't applied the patch), but I'm using version 5.3.1, and in node_modules/react-native-branch/react-native-branch.podspec I can see

  # Swift/Objective-C compatibility
  s.pod_target_xcconfig = {
    'DEFINES_MODULE' => 'YES'
  }

so it should be good. Does anyone have any ideas what's going wrong there?

(Expo 44)

mersiades commented 2 years ago

I tried to add it to my app today, but eas build -p ios --profile development failed (Android is good, though).

I got the build passing after I removed a dependency called fiction-expo-restart. Removing that dependency also removed @unimodules/core, so the working theory is that the presence of @unimodules/core was messing with the pod linking.

manuelmhtr commented 2 years ago

I had the same problem with @config-plugins/react-native-branch version 1.0.2 and react-native-branch version 5.0.0 which are the versions installed using expo install.

Tried with:

yarn add react-native-branch @config-plugins/react-native-branch

which installed react-native-branch version 5.4.0 and the error was fixed.

(In my case @unimodules/core wasn't a dependency)

jaredramirez commented 1 year ago

I have branch working completely in my app, but when I build with expo start, it prints a warning:

Some dependencies are incompatible with the installed expo package version:
 - react-native-branch - expected version: 5.0.0 - actual version installed: 5.5.0

Does a peer dependency need to be updated somewhere? This error is a bit misleading, as react-native-branch@5.0.0 doesn't actually work with this plugin. You need at least 5.3.0 (see https://github.com/expo/config-plugins/pull/28#issuecomment-1021845591)

gerwim commented 1 year ago

I have branch working completely in my app, but when I build with expo start, it prints a warning:


Some dependencies are incompatible with the installed expo package version:

 - react-native-branch - expected version: 5.0.0 - actual version installed: 5.5.0

Does a peer dependency need to be updated somewhere? This error is a bit misleading, as react-native-branch@5.0.0 doesn't actually work with this plugin. You need at least 5.3.0 (see https://github.com/expo/config-plugins/pull/28#issuecomment-1021845591)

AFAIK the message can be ignored with 5.3.0. I assume the same for 5.5.0. Just test if it works (but you'll need a new native dev build if you didn't do that yet). :-)