fix: compileDebugJavaWithJavac #31

UriMiranda · 2021-05-06T14:00:33Z

Propose to fix a build error in android. Missing constructor on FacebookProfile.

If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
#30

Test Plan:
In a project with react-native-fbsdk-next as dependency enter in android folder and run:
cd android && ./gradlew app:installDebug -PreactNativeDevServerPort=8081

mikehardy · 2021-05-07T16:41:52Z

Hi there!

This should not be merged without an adequate explanation why the build succeeds in CI: https://github.com/thebergamo/react-native-fbsdk-next/runs/2491258907?check_suite_focus=true

If CI is successfully compiling - the assumption must be that

there is a project configuration error somehow or
that CI is somehow incorrect and not detecting something

If CI is somehow missing something, that's important to know and the PR should include a change to this workflow file to show the failure, and then the PR should include the fix and this should merge - https://github.com/thebergamo/react-native-fbsdk-next/blob/master/.github/workflows/ci.yml

mikehardy

I love fixing compile errors, but as mentioned in my comment on the PR itself this should not be merged until there is a diagnosis on why CI was building successfully vs your experience of having a build failure

rsarv-boom · 2021-05-07T17:18:46Z

Build failed for me as well. Downgrading to 4.1.0 fixed the issue.

mikehardy · 2021-05-07T17:21:57Z

Would love to fix it: we must know why the example compiles in CI and your build fails first though

UriMiranda · 2021-05-07T17:25:15Z

I think that is Grande version, is the only thing that changes from my machine to CI

UriMiranda · 2021-05-07T17:27:05Z

But don't make sense, is just a simple constructor

mikehardy · 2021-05-07T17:27:30Z

@UriMiranda okay: can you try it in your project with the identical version to verify? Then we will know and we can make real decisions on how to handle

UriMiranda · 2021-05-07T17:29:09Z

Sure, I will try

UriMiranda · 2021-05-07T17:44:17Z

Is not Grandle version, I tried with the same version. Do u know what is the java version? Here is 1.8

thebergamo · 2021-05-07T20:46:01Z

From what I see here it uses Java 11:
ubuntu-latest

mikehardy · 2021-05-07T23:31:38Z

You don't have to guess, you can look at the action output.

For instance, you are attempting to fix a compile error but your PR does not actually compile: https://github.com/thebergamo/react-native-fbsdk-next/runs/2529266420?check_suite_focus=true - check the output and you see missing symbol

And the environment in general:
https://github.com/thebergamo/react-native-fbsdk-next/runs/2529266420?check_suite_focus=true
https://github.com/actions/virtual-environments/blob/ubuntu20/20210425.1/images/linux/Ubuntu2004-README.md
Yes Java 11 is the default on the runner and CI is using that. In fact it is the default for Android Studio 4.2 (came out this week) now.

I just made sure my project is using 4.2.0 here and JDK8, it works fine too.

With no reproduction, there's no issue I think.

mikehardy

Fails to compile in CI, project works without this in my project - need demonstration of failure in CI prior to merge if merged

delabiejochen · 2021-05-10T15:27:09Z

I have the same issue:

https://github.com/facebook/react-native/blob/v0.60.4/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java - React Native 0.60.4 does not provide a default constructor
https://github.com/facebook/react-native/blob/v0.64.1/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContextBaseJavaModule.java the latest React Native version (0.64.1) does

https://github.com/thebergamo/react-native-fbsdk-next/blob/master/android/src/main/java/com/facebook/reactnative/androidsdk/FBProfileModule.java is extending ReactContextBaseJavaModule and assuming there's a default constructor when in older versions there's not. That's why ceb97f5 is a fix for this

mikehardy · 2021-05-10T15:35:38Z

That explains it! An explanation is important.

So the default constructor was added for react-native 0.62 - facebook/react-native@e69be0a

That means a couple things at once:

the CHANGELOG can get an update for the already-released version indicating it has a requirement for minimum react-native version of 0.62 as an accidental breaking change
this repo can make a decision on what minimum version of react-native should be supported (0.60? Does it even correctly operate on ios14.5? Since you need Xcode 12.5 so sign apps for ios14.5 and Xcode 12.5 won't compile react-native 0.62 even without patching, I think not)
based on the decision of minimum react-native version support an issue can be logged here to have a separate CI task that does a test build against that minimum

With respect + understanding that it is not easy to update, I don't see react-native below 0.63 as viable even as that is the oldest one that was patched to work with Xcode 12.5 and that's a requirement for iOS development.

So I personally don't see react-native 0.60 / 0.61 compatibility as a reasonable target and I'd simply note the same in package.json / CHANGELOG / re-issue as a breaking change and counsel use of https://react-native-community.github.io/upgrade-helper/ so people get up to date. That is opinion, of course, but the Xcode 12.5 build thing is fact facebook/react-native#31179

UriMiranda · 2021-05-13T12:35:55Z

Hi, I guess the error is because of an annotation in construct (@NotNull), I saw others classes in code that has the same constructor without it.

UriMiranda · 2021-05-13T12:41:57Z

I removed it, 50% of chance that will work. lol

mikehardy · 2021-05-13T15:17:26Z

android/src/main/java/com/facebook/reactnative/androidsdk/FBSDKPackage.java

@@ -43,7 +43,7 @@
                new FBGraphRequestModule(reactContext),
                new FBLoginManagerModule(reactContext, mActivityEventListener),
                new FBMessageDialogModule(reactContext, mActivityEventListener),
-                new FBProfileModule(),
+                new FBProfileModule(reactContext),
                new FBSettingsModule(),


isn't FBSettingsModule going to have the same issue 🤔

Good question 🤔

UriMiranda · 2021-05-13T23:46:39Z

It works now!! Resolve ReactApplicationContext import.

mikehardy

I'm running CI on it now, and now that we know it's a react-version problem (so the problem is understood!) I'm +1 to get people moving.
I still argue (I think reasonably, with technical merit) that react-native 0.63 should be the minimum supported version going forward, but that's something that needs to be stated explicitly, and it has not been (yet)

thebergamo · 2021-05-23T06:26:36Z

I'm fine supporting only version 0.63 of react-native as per it won't compile in newer Xcode.

When I get in my computer I will add a notice in the readme stating this in this PR.

# [4.3.0](v4.2.0...v4.3.0) (2021-05-27) ### Bug Fixes * compileDebugJavaWithJavac ([#31](#31)) ([913e792](913e792)) * update React-Core pod ([#42](#42)) ([a1e1141](a1e1141)) ### Features * **ios, sdk:** upgrade to FBSDK version 9.3.x ([#45](#45)) ([b2ea364](b2ea364))

github-actions · 2021-05-27T22:02:51Z

🎉 This PR is included in version 4.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

UriMiranda changed the title ~~Fix react-native-fbsdk-next:compileDebugJavaWithJavac~~ 4.2.1 Fix react-native-fbsdk-next:compileDebugJavaWithJavac May 6, 2021

UriMiranda force-pushed the master branch from f4e00e2 to 00d296a Compare May 6, 2021 14:11

fix: react-native-fbsdk-next:compileDebugJavaWithJavac

ceb97f5

UriMiranda changed the title ~~4.2.1 Fix react-native-fbsdk-next:compileDebugJavaWithJavac~~ fix react-native-fbsdk-next:compileDebugJavaWithJavac May 6, 2021

UriMiranda changed the title ~~fix react-native-fbsdk-next:compileDebugJavaWithJavac~~ fix compileDebugJavaWithJavac May 6, 2021

UriMiranda changed the title ~~fix compileDebugJavaWithJavac~~ fix: compileDebugJavaWithJavac May 6, 2021

UriMiranda force-pushed the master branch from 00d296a to ceb97f5 Compare May 6, 2021 14:17

mikehardy requested changes May 7, 2021

View reviewed changes

fix: Make constructor equal to FBAppLinkModule

b393ccd

UriMiranda force-pushed the master branch from c19f1de to b393ccd Compare May 13, 2021 12:30

mikehardy reviewed May 13, 2021

View reviewed changes

UriMiranda force-pushed the master branch from 77a85c1 to b15d6d3 Compare May 13, 2021 23:38

fix: Importing ReactApplicationContext

1b96d06

UriMiranda force-pushed the master branch from b15d6d3 to 1b96d06 Compare May 13, 2021 23:42

mikehardy approved these changes May 14, 2021

View reviewed changes

mikehardy requested a review from thebergamo May 14, 2021 01:28

thebergamo approved these changes May 27, 2021

View reviewed changes

thebergamo merged commit 913e792 into thebergamo:master May 27, 2021

github-actions bot added the released label May 27, 2021

thebergamo mentioned this pull request May 27, 2021

[SOLVED: use react-native 0.63+ or patch locally] react-native-fbsdk-next:compileDebugJavaWithJavac #30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: compileDebugJavaWithJavac #31

fix: compileDebugJavaWithJavac #31

UriMiranda commented May 6, 2021 •

edited

Loading

mikehardy commented May 7, 2021

mikehardy left a comment

rsarv-boom commented May 7, 2021

mikehardy commented May 7, 2021

UriMiranda commented May 7, 2021

UriMiranda commented May 7, 2021

mikehardy commented May 7, 2021

UriMiranda commented May 7, 2021

UriMiranda commented May 7, 2021

thebergamo commented May 7, 2021

mikehardy commented May 7, 2021

mikehardy left a comment

delabiejochen commented May 10, 2021

mikehardy commented May 10, 2021

UriMiranda commented May 13, 2021

UriMiranda commented May 13, 2021

mikehardy May 13, 2021

UriMiranda May 13, 2021

UriMiranda commented May 13, 2021 •

edited

Loading

mikehardy left a comment

thebergamo commented May 23, 2021

github-actions bot commented May 27, 2021

fix: compileDebugJavaWithJavac #31

fix: compileDebugJavaWithJavac #31

Conversation

UriMiranda commented May 6, 2021 • edited Loading

mikehardy commented May 7, 2021

mikehardy left a comment

Choose a reason for hiding this comment

rsarv-boom commented May 7, 2021

mikehardy commented May 7, 2021

UriMiranda commented May 7, 2021

UriMiranda commented May 7, 2021

mikehardy commented May 7, 2021

UriMiranda commented May 7, 2021

UriMiranda commented May 7, 2021

thebergamo commented May 7, 2021

mikehardy commented May 7, 2021

mikehardy left a comment

Choose a reason for hiding this comment

delabiejochen commented May 10, 2021

mikehardy commented May 10, 2021

UriMiranda commented May 13, 2021

UriMiranda commented May 13, 2021

mikehardy May 13, 2021

Choose a reason for hiding this comment

UriMiranda May 13, 2021

Choose a reason for hiding this comment

UriMiranda commented May 13, 2021 • edited Loading

mikehardy left a comment

Choose a reason for hiding this comment

thebergamo commented May 23, 2021

github-actions bot commented May 27, 2021

UriMiranda commented May 6, 2021 •

edited

Loading

UriMiranda commented May 13, 2021 •

edited

Loading