AppsFlyerSDK / appsflyer-react-native-plugin

AppsFlyer plugin for React Native
MIT License
284 stars 202 forks source link

6.14.3 breaking android change #551

Open CTOverton opened 6 months ago

CTOverton commented 6 months ago

Report

There seems to be an update made for the sdk that conflicts with android and react native android side of things. @swrobel provided more details here https://github.com/AppsFlyerSDK/appsflyer-react-native-plugin/pull/546#discussion_r1588556448

The affected pr is here https://github.com/AppsFlyerSDK/appsflyer-react-native-plugin/pull/546

Plugin Version

6.14.3

On what Platform are you having the issue?

Android

What did you do?

  1. Update react-native-appsflyer to 6.14.3
  2. Build react native app for Android

Notice gradle will fail to build with this error:

Manifest merger failed : Attribute application@allowBackup value=(false) from AndroidManifest.xml:26:9-36
is also present at [com.appsflyer:af-android-sdk:6.14.0] AndroidManifest.xml:32:9-35 value=(true).
Suggestion: add 'tools:replace="android:allowBackup"' to <application> element at AndroidManifest.xml:7:5-117 to override.

What did you expect to happen?

Android to successfully build

What happened instead?

Android failed to build

swrobel commented 6 months ago

In order to fix this, I set android:allowBackup="true" in AndroidManifest.xml. I don't see any harm in changing this setting; it seems to have been set as a default years ago in RN without any explanation. Someone questioned the choice of false as a default, but no explanation was ever provided. Honestly, true sounds better for user experience.

CTOverton commented 6 months ago

Thanks @swrobel, is this something that will be fixed in the future so this step won't be necessary? Or will this need to be added to the setup documentation for appsflyer?

swrobel commented 6 months ago

Beats me... I'm just a user like you 🤷🏼‍♂️

TomasSestak commented 6 months ago

this is definetely bug allow_backup = false must be usable

mblarsen commented 6 months ago

We need allowBackup to be false too. It is not an option for us to enable backups.

I've opened an issue on the Android repo https://github.com/AppsFlyerSDK/appsflyer-android-sdk/issues/70 which is where the setting comes from.

cc @TomasSestak please upvote the issue

victoralmeidadev commented 5 months ago

As an alternative I kept the library version at 6.13.0, but I still need the fix for future versions.

servercimen commented 4 months ago

Adding the following to my react apps AndroidManifest.xml's <application> tag fixed this issue for me:

tools:replace="android:allowBackup"

But I don't know the full implications of this.

mblarsen commented 3 months ago

Adding the following to my react apps AndroidManifest.xml's <application> tag fixed this issue for me:

tools:replace="android:allowBackup"

But I don't know the full implications of this.

This will turn on backups, which we do not want.

https://developer.android.com/identity/data/backup

CptFabulouso commented 3 months ago

Adding the following to my react apps AndroidManifest.xml's <application> tag fixed this issue for me: tools:replace="android:allowBackup" But I don't know the full implications of this.

This will turn on backups, which we do not want.

https://developer.android.com/identity/data/backup

I don't think you are right. Can't see anything relevant in the link you posted. The tools:replace="android:allowBackup" should force the allowBackup to be the one we are specifying, which is false. If I build the project with this setup and look at android/app/build/intermediates/merged_manifest/debug/AndroidManifest.xml file, it indeed has line android:allowBackup="false"

crazywook commented 3 months ago

This omission caused unexpected conflicts during the build process, which required manual intervention to resolve. It would be very helpful if the release notes could be updated to include this information, ensuring other developers are aware of the necessary changes.

Thank you.

MAsadIlyasNajum commented 2 months ago

I'm also facing same problem

mblarsen commented 2 months ago

Adding the following to my react apps AndroidManifest.xml's <application> tag fixed this issue for me: tools:replace="android:allowBackup" But I don't know the full implications of this.

This will turn on backups, which we do not want. developer.android.com/identity/data/backup

I don't think you are right. Can't see anything relevant in the link you posted. The tools:replace="android:allowBackup" should force the allowBackup to be the one we are specifying, which is false. If I build the project with this setup and look at android/app/build/intermediates/merged_manifest/debug/AndroidManifest.xml file, it indeed has line android:allowBackup="false"

@CptFabulouso Could you provide a more complete example of you AndroidManiferst.xml? When I tried build failed stating the plugin had a conflicting setting (allow = true).

CptFabulouso commented 2 months ago

Sure, here it is:

<manifest 
  xmlns:android="http://schemas.android.com/apk/res/android" 
  xmlns:tools="http://schemas.android.com/tools" // <-- Add this line if it's not there already, so you can use tools:replace
>
  <application 
    android:name=".MainApplication" 
    android:label="@string/app_name" 
    android:icon="@mipmap/ic_launcher" 
    android:roundIcon="@mipmap/ic_launcher_round" 
    android:theme="@style/AppTheme" 
    android:allowBackup="false" // <-- Set the value you want, presumably false
    tools:replace="android:allowBackup" // <-- Force the value you have set, ignoring value of dependencies
>
    </application>
</manifest>

In case you are using Expo, I wrore this plugin: file plugins/react-native-appsflyer.ts, written in typescript, make sure to compile it to JS before using (yarn tsc plugins/react-native-appsflyer.ts --watch --skipLibCheck. The part manipulating the AndroidManifest is just the withAndroidManifest function. Note that the withMainActivity is written for Expo 51, where kotlin is used, older versions use Java and this code won't work.

import { ConfigPlugin, withAndroidManifest, withMainActivity } from '@expo/config-plugins';

function withAppsFlyerFix(expoConfig: Parameters<ConfigPlugin>[0]) {
  expoConfig = withAndroidManifest(expoConfig, (config) => {
    // make sure we have the tools namespace
    if (!config.modResults.manifest.$['xmlns:tools']) {
      config.modResults.manifest.$['xmlns:tools'] = 'http://schemas.android.com/tools';
    }
    // force our own value for allowBackup
    if (Array.isArray(config.modResults.manifest.application)) {
      if (config.modResults.manifest.application?.[0]?.$) {
        config.modResults.manifest.application[0].$['tools:replace'] = 'android:allowBackup';
      }
    }
    return config;
  });

  // modify main activity according to https://dev.appsflyer.com/hc/docs/rn_expodeeplinkintegration#implementation-for-expo
  expoConfig = withMainActivity(expoConfig, (config) => {
    const fileLines = config.modResults.contents.split('\n');
    const fileLength = fileLines.length;

    // add import statement
    const classSpecificationLine = fileLines.findIndex((line) => line.startsWith('class MainActivity '));
    if (classSpecificationLine === -1) {
      throw new Error('Could not find the class specification line in the Android MainActivity file.');
    }
    const newImportLines = ['import android.content.Intent'];
    fileLines.splice(classSpecificationLine - 1, 0, ...newImportLines);

    // add the new method
    const newFunctionLines = [
      '',
      '  /**',
      '   * required by react-native-appsflyer',
      '   */',
      '  override fun onNewIntent(intent: Intent) {',
      '        super.onNewIntent(intent);',
      '        setIntent(intent);',
      '  }',
    ];
    fileLines.splice(fileLength - 1, 0, ...newFunctionLines);

    // replace content
    config.modResults.contents = fileLines.join('\n');
    return config;
  });

  return expoConfig;
}

export default withAppsFlyerFix;
miladdev85 commented 2 months ago

More info herre: https://dev.appsflyer.com/hc/docs/install-android-sdk