wix / react-native-navigation

A complete native navigation solution for React Native
https://wix.github.io/react-native-navigation/
MIT License
13.04k stars 2.67k forks source link

[v2] android build failure with react-native 0.56.0 #3505

Closed ujwal-setlur closed 6 years ago

ujwal-setlur commented 6 years ago

Issue Description

android build fails with:

> Task :react-native-navigation:compileReactNative55DebugJavaWithJavac FAILED
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/JsDevReloadHandler.java:16: error: JsDevReloadHandler is not abstract and does not override abstract method onSuccess(NativeDeltaClient) in DevBundleDownloadListener
public class JsDevReloadHandler implements DevBundleDownloadListener {
       ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/JsDevReloadHandler.java:38: error: method does not override or implement a method from a supertype
    @Override
    ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/ReloadHandler.java:7: error: ReloadHandler is not abstract and does not override abstract method onSuccess(NativeDeltaClient) in DevBundleDownloadListener
public class ReloadHandler implements JsDevReloadHandler.ReloadListener, DevBundleDownloadListener {
       ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/ReloadHandler.java:26: error: method does not override or implement a method from a supertype
    @Override
    ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/DevBundleDownloadListenerAdapter.java:7: error: DevBundleDownloadListenerAdapter is not abstract and does not override abstract method onSuccess(NativeDeltaClient) in DevBundleDownloadListener
public class DevBundleDownloadListenerAdapter implements DevBundleDownloadListener {
       ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/DevBundleDownloadListenerAdapter.java:8: error: method does not override or implement a method from a supertype
    @Override
    ^
/Users/ujwal/Work/seamlz/seamlz-mobile/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationReactNativeHost.java:33: error: method onSuccess in interface DevBundleDownloadListener cannot be applied to given types;
                bundleListener.onSuccess();
                              ^
  required: NativeDeltaClient
  found: no arguments
  reason: actual and formal argument lists differ in length
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
7 errors

Environment

jp928 commented 6 years ago

+1

ujwal-setlur commented 6 years ago

iOS build is fine.

ujwal-setlur commented 6 years ago

@emiyake, I tried your patch, and the compilation goes through, but I am getting an error on android when the app loads. On iOS, it is just fine. I am not sure whether this is related to this issue

screen shot 2018-07-06 at 1 41 19 pm
ujwal-setlur commented 6 years ago

It seems this is more of a react issue, but it happens only on Android, and the exact same code was working on react-native 0.55.4 and rnn-2.0.2397

emiyake commented 6 years ago

I tested it on react-native@0.56.0.

If you're using react-native@0.55.4 you might want to downgrade to react-native-navigation@2.0.2395

ujwal-setlur commented 6 years ago

Will try that later tonight. Did something go into 2.0.2397 that would cause this?

Just to reiterate, react-native 0.55.4 and rnn-2.0.2397 works just fine.

When I used react-native 0.56.0, I saw the problem I originally reported where I could not even build. With your patch applied, I can build with react-native 0.56.0, but then get the react error shown in the screenshot.

emiyake commented 6 years ago

Yes, this is the issue: https://github.com/wix/react-native-navigation/commit/50fd77ec82a0d287799f15193d81fb9544f0bb96#diff-9fa2730ac48819277a0f96ccb7b11a9e

ujwal-setlur commented 6 years ago

@emiyake, I tried rnn-2.0.2395, and I could build it with rn-0.56.0 fine without having to apply your pull-request, but I still get the same react error on app load as seen in the screenshot above. I was not getting this with rn-0.55.4. This is happening only on android, and not iOS.

ujwal-setlur commented 6 years ago

OK, I think this is something specific to my environment. I stripped it down to a very basic app, and it loads ok.

ujwal-setlur commented 6 years ago

It turns out there was a polyfill I was using on android that was causing this:

import "core-js/es6/symbol";
emiyake commented 6 years ago

@ujwal-setlur Yes.rnn-2.0.2395 doesn't require the pull request. rnn-2.0.2395 with rn-0.56.0 works fine without any patch.

Only rnn-2.0.2397 causes the error you described in the beginning of this thread.

ujwal-setlur commented 6 years ago

Yep, that's resolved with your change. The other problem I described was due to my use of a polyfill.

https://github.com/facebook/react-native/issues/18542

stokesbga commented 6 years ago

Im seeing this on 2.0.2404

alexxsanchezm commented 6 years ago

To deal with this issue, I did:

// path: node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/JsDevReloadHandler.java
/* modify onSuccess() with onSuccess(@Nullable NativeDeltaClient nativeDeltaClient)
@Override
    public void onSuccess(@Nullable NativeDeltaClient nativeDeltaClient) {
        UiUtils.runOnMainThread(reloadListener::onReload);
    }
//path: node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/DevBundleDownloadListenerAdapter.java
/* change onSuccess() with onSuccess(@Nullable NativeDeltaClient nativeDeltaClient) */
@Override
    public void onSuccess(@Nullable NativeDeltaClient nativeDeltaClient) {
    }
//path: node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationReactNativeHost.java
/* Here just passed down the nativeDeltaClient instance */
@Override
public void onSuccess(NativeDeltaClient nativeDeltaClient) {
    if (bundleListener != null) {
        bundleListener.onSuccess(nativeDeltaClient);
    }
}
//path: node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/ReloadHandler.java
/* add nativeDeltaClient to onSuccess() */
@Override
public void onSuccess(@Nullable NativeDeltaClient nativeDeltaClient) {
    onReloadListener.run();
}

That solved the issue, off course I'm just guessing... but worked for me. But it seemed like the interfaces signatura have changed but not its implementations, looks like a bad merge.

stokesbga commented 6 years ago

I made the same changes in my own fork of 2.0.2404 and its all good. yeah must be bad merge ^

ujwal-setlur commented 6 years ago

I don't believe @emiyake's PR has been merged.

dorthwein commented 6 years ago

Just to clarify - is there a pending fix for this??

ujwal-setlur commented 6 years ago

I hope so, because otherwise, rnn can't build with rn-0.56.0. I am currently using @emiyake's patch with patch-package

haswalt commented 6 years ago

Looks like the patch was merged and a version with the change is on NPM however I still get the error.

ujwal-setlur commented 6 years ago

I will check in just a bit, but make sure you actually got the latest version. That has tripped me up sometimes, particularly if you have yarn.lock or package-lock.json lying around

ujwal-setlur commented 6 years ago

@haswalt, you are correct. I just checked with 2.0.2418, and the build fails.

ujwal-setlur commented 6 years ago

@haswalt, please set:

missingDimensionStrategy "RNN.reactNativeVersion", "reactNative56"

in android/app/build.gradle

Build goes through then.

ujwal-setlur commented 6 years ago

This is now resolved in 2.0.2418, but please note comment above. Documentation needs to be updated

haswalt commented 6 years ago

@ujwal-setlur I still can't get this to build. Tried a fresh project, followed setup instructions with change above for reactNative56 and i get the same error messages.

I see that Task :react-native-navigation:compileReactNative56ReleaseJavaWithJavac runs correctly first followed by the failing Task :react-native-navigation:compileReactNative51ReleaseJavaWithJavac FAILED

app/build.gradle

compileSdkVersion rootProject.ext.compileSdkVersion
    buildToolsVersion rootProject.ext.buildToolsVersion

    defaultConfig {
        applicationId "com.kitchensink"
        minSdkVersion rootProject.ext.minSdkVersion
        targetSdkVersion rootProject.ext.targetSdkVersion
        missingDimensionStrategy "RNN.reactNativeVersion", "reactNative56"
        versionCode 1
        versionName "1.0"
        ndk {
            abiFilters "armeabi-v7a", "x86"
        }
    }
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }

build.gradle

ext {
    buildToolsVersion = "27.0.3"
    minSdkVersion = 25
    compileSdkVersion = 27
    targetSdkVersion = 26
    supportLibVersion = "26.1.0"
}
haswalt commented 6 years ago

note this is the issue reported here: https://github.com/wix/react-native-navigation/issues/3388#issuecomment-402637025

haswalt commented 6 years ago

If I delete the productFlavours that i'm not using from the lib I have no issues building, unfortunately I don't know enough about android and gradle to work out why al productFlavors are being built

ujwal-setlur commented 6 years ago

@haswalt, are you doing a debug build or release build? Let me try on a fresh protect as well

haswalt commented 6 years ago

@ujwal-setlur Release build.

For those hitting this issue I'm using patch-package and this patch:

patch-package
--- a/node_modules/react-native-navigation/lib/android/app/build.gradle
+++ b/node_modules/react-native-navigation/lib/android/app/build.gradle
@@ -48,12 +48,6 @@ android {
     }
     flavorDimensions "RNN.reactNativeVersion"
     productFlavors {
-        reactNative51 {
-            dimension "RNN.reactNativeVersion"
-        }
-        reactNative55 {
-            dimension "RNN.reactNativeVersion"
-        }
         reactNative56 {
             dimension "RNN.reactNativeVersion"
         }
haswalt commented 6 years ago

same with debug build

sytolk commented 6 years ago

@haswalt this is different issue. Try this fix: https://github.com/wix/react-native-navigation/issues/3388#issuecomment-399071604

billel-boudchicha commented 6 years ago

hi @sytolk just update to 2.0.2435 still the issue downgrade to 2.0.2425 fix it

herarya commented 6 years ago

solved 👍 "react-native": "0.56.0", "react-native-navigation": "^2.0.2427",

ujwal-setlur commented 6 years ago

I will close this as this seems to be working. Works for me at least...

abury commented 6 years ago

I'm still getting this issue. I'm running RNN 2.0.2427 with RN 0.56.0 and debug build works fine, however a release build fails with the above issues. (And yes I'm using the right build flavour)

abury commented 6 years ago

we were using app:assembleDebug but weren't passing the app prefix for the release build. We've added this and the build issues seem to have been resolved 👍

ionush commented 6 years ago

I'm also still getting this issue. Running RN 0.56 with RNN 2.0.2577. Also tried latest version 2.0.2582. All the above solutions don't work. Any suggestions?

phantom1299 commented 5 years ago

For me, debug was working but release was failing with above issue. The solution was to add filter for reactNative57 too because the error message was containing stuff with

generateReactNative57ReleaseRFile