Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove allowBackup & supportsRtl from AndroidManifest #180

Merged
merged 1 commit into from May 21, 2018

Conversation

christianchown
Copy link
Contributor

RN 0.54 changes the AndroidManifest.xml setting for android:allowBackup from true to false: facebook/react-native@d7a9ca2

As react-native-splash-screen specifies android:allowBackup="true", this causes the following linker error for all new RN projects (and those that specify: android:allowBackup="false")

Attribute application@allowBackup value=(false) from AndroidManifest.xml
is also present at [react-native-splash-screen] AndroidManifest.xml value=(true).

Suggestion: add 'tools:replace="android:allowBackup"' to <application> 
element at AndroidManifest.xml

BUILD FAILED

This PR removes this default from the library, along with the default for android:supportsRtl, as per this suggestion: https://medium.com/@elye.project/2-lines-in-manifest-to-remove-when-sharing-your-android-library-565d4c4af04a

@lucidtheory
Copy link

Please merge this

1 similar comment
@hoyajigi
Copy link

Please merge this

@WhatUsersLove
Copy link

WhatUsersLove commented Mar 19, 2018

Please merge... What is the work around for this right now?

@lucidtheory
Copy link

In development you can just edit the AndroidManifest.xml in the node_modules

If this isn't merged before you deploy you can fork the repo with the updates and use that as your source

@WhatUsersLove
Copy link

Great. So change android:allowBackup to true?

@lucidtheory
Copy link

lucidtheory commented Mar 19, 2018

Delete the line completely.

Also delete the line that says android:supportsRtl

Be sure you are removing it from the AndroidManifest.xml that is in the splash screen node_modules folder and not your main application one

@WhatUsersLove
Copy link

Now I get it. Thanks.

@WhatUsersLove
Copy link

So when am deploying for production I will fork the repo with the updates then how do I make it my source as I am reinstalling the library? (Im from a web dev background)

@lucidtheory
Copy link

that's ok,

in your package.json:

"dependenceis":{
      ...other packages
      "react-native-splash-screen": "github:yourGithubUserName/yourForkedRepoName"
}

so if you forked it under the same repo name it would be
"react-native-splash-screen": "github:TechyTimo/react-native-splash-screen"

@WhatUsersLove
Copy link

Excellent! Thanks

@christianchown
Copy link
Contributor Author

@esdrasetrenne @TechyTimo

Cleaner workaround - keep package.json etc. as they were

AndroidManifest.xml

<application
      ...
      android:allowBackup="false"
      tools:replace="android:allowBackup"
>

@lucidtheory
Copy link

Oh nice, I have seen that before.

That would go in the main app's AndroidManifest.xml yea?

Also, I think with this xmlns:tools="http://schemas.android.com/tools" needs to be added to the manifest tag at the top if it is not already there

@christianchown
Copy link
Contributor Author

@esdrasetrenne - yes, in your app's AndroidManifest.xml: and you're right about the tools schema too

@RWOverdijk
Copy link

Yes please. 👍

@tasarsu
Copy link

tasarsu commented May 11, 2018

For the ones who had this error;

The prefix "tools" for attribute "tools:replace" associated with an element type "application" is not bound.

add xmlns:tools attributes to manifest tag

<manifest 
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    ...
>

@adlondon
Copy link

still crashing for me with all changes made through tasarsu's suggestion editing manfifest... no build errors, app just crashes on launch.

@dannevesdantas
Copy link

dannevesdantas commented May 20, 2018

@adlondon make sure your color.xml exists in android/app/src/main/res/values

@adlondon
Copy link

colors.xml did it. Thanks all!

@crazycodeboy crazycodeboy merged commit a5c46f7 into crazycodeboy:master May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants