Skip to content

fix(Android): fixed deprecated APPLICATION_ID for failed builds#3710

Closed
FrenchBully wants to merge 4 commits intoinvertase:masterfrom
FrenchBully:bug/deprecated-application-id
Closed

fix(Android): fixed deprecated APPLICATION_ID for failed builds#3710
FrenchBully wants to merge 4 commits intoinvertase:masterfrom
FrenchBully:bug/deprecated-application-id

Conversation

@FrenchBully
Copy link
Copy Markdown
Contributor

@FrenchBully FrenchBully commented May 28, 2020

Description

This PR is to resolve the issue with deprecated APPLICATION_ID on Android documented here

Unable to build App to Android:

node_modules@react-native-firebase\app\android\src\reactnative\java\io\invertase\firebase\app\ReactNativeFirebaseAppInitProvider.java:25: error: cannot find symbol
    BuildConfig.APPLICATION_ID + ".reactnativefirebaseappinitprovider";
               ^
  symbol:   variable APPLICATION_ID
  location: class BuildConfig
1 error

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':@react-native-firebase_app:compileDebugJavaWithJavac'.

Related issues

Fixes #3254
Issue - 3254: deprecated APPLICATION_ID

Release Summary

  • fix for deprecated APPLICATION_ID to use LIBRARY_PACKAGE_NAME

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

Screen Shot 2020-05-28 at 1 47 38 PM

Screen Shot 2020-05-28 at 1 48 20 PM

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 28, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 28, 2020

The other way to fix this was to refactor and take BuildConfig out, vs using these statics, I'm not sure the LIBRARY_PACKAGE_NAME is the right direction? Is it a breaking change even, since it would be a different constant I think?

From @Salakar on linked issue

Thanks @mikehardy for investigating.

We could probably just refactor this not to rely on BuildConfig, I think there's an underlying gradle change where sub projects no longer have application id generated into the build config.

We can switch this out for Context.getPackageName() in the base class, should solve the issue. PRs welcome otherwise we'll get to this as soon as possible.

@FrenchBully
Copy link
Copy Markdown
Contributor Author

FrenchBully commented May 29, 2020

@mikehardy does the recent changes fit the intended use or are you expecting another way to fetch package name? Sorry, I forgot to mention I'm not a Java engineer. Just learning through it. Thanks for any feedback and insights.

@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 29, 2020

@Salakar Android Studio 4.0 just came out today so everyone is going to break android builds unless either this is merged or you refactor out in the way you mentioned and I quoted above

@mikehardy mikehardy requested a review from Salakar May 29, 2020 02:59
@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 29, 2020

@FrenchBully I think Mike (Salakar) was thinking of something different but we've run out of time since Android Studio 4 just went general release. We'll see what he says

@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 29, 2020

Actually your last commit looks like what was sketched out (sorry I didn't see it until now) investigating why it didn't compile. Must be something simple - this isn't a crazy change and I am a Java programmer :-), give me a minute

mikehardy
mikehardy requested changes May 29, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you commit my suggestion then the symbol will be found and it will work

@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 29, 2020

..actually I had the ability to commit it, we'll see
(sorry for bumbling through this - I haven't gone through a react-native-firebase change myself in a while, I'm a bit rusty)

@Salakar Salakar mentioned this pull request May 29, 2020
Merged
8 tasks
@mikehardy
Copy link
Copy Markdown
Collaborator

mikehardy commented May 29, 2020

My attempt only made it worse ;-), @Salakar just fixed it for real and released - @FrenchBully thanks for giving it a go either way, definitely got it moving and just in time - much appreciated

@mikehardy mikehardy closed this May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikehardy mikehardy mikehardy requested changes

@Salakar Salakar Awaiting requested review from Salakar

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cannot find symbol BuildConfig.APPLICATION_ID

3 participants

@FrenchBully @CLAassistant @mikehardy