Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

Android build error with [email protected] #158

Closed
jpapillon opened this issue Jul 27, 2016 · 42 comments
Closed

Android build error with [email protected] #158

jpapillon opened this issue Jul 27, 2016 · 42 comments

Comments

@jpapillon
Copy link

jpapillon commented Jul 27, 2016

I get this error when building with [email protected]:

node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:64: error: method does not override or implement a method from a supertype
    @Override
    ^
1 error

Quick link to line

@SpaceK33z
Copy link

I'm having the same error.

@rturk
Copy link

rturk commented Jul 27, 2016

Are you using other external packages that require push? Like Firebase?

@SpaceK33z
Copy link

@rturk, good idea, I disabled all other external packages. Unfortunately the error is still present.

@skunkworker
Copy link

Same. I'm encountering this error with rn 0.30.0 and 2.0.2 of the app. Has anyone gotten this to work correctly?

@finalquest
Copy link

same here

@skunkworker
Copy link

Rolling back to 2.0.1 fixed it for now, while still being on rn 0.30.0
"react-native-push-notification": "2.0.1"

@clintonjrobinson
Copy link
Contributor

Had this problem also, had to

npm cache clear
rm -Rf node_modules
npm install

After than 2.0.2 + rn 0.30.0 works properly.

@ssancheza227
Copy link

I'm having the same error :( : "react-native": "^0.30.0", "react-native-push-notification": "^2.0.1"

error: method does not override or implement a method from a supertype
@OverRide
^

@SpaceK33z
Copy link

Did you guys all upgrade from a previous version of RN? For me the error was fixed after I ran react-native upgrade, and made sure all files in android/ were up-to-date.

@jpapillon
Copy link
Author

jpapillon commented Aug 5, 2016

I tried [email protected] with 2.0.2 (commit #0b019670) and it requires to remove the @Override keyword and leave the onNewIntent function where it is.

@jamespearson
Copy link

Re-running the upgrade, and ensuring I got everything, worked for me.

@jpapillon
Copy link
Author

jpapillon commented Aug 15, 2016

Everything is updated and this is the error I get trying to compile it with [email protected]:

./node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:64: error: method does not override or implement a method from a supertype
    @Override
    ^
1 error
:react-native-push-notification:compileReleaseJavaWithJavac FAILED

@jpapillon
Copy link
Author

I found out that my configuration in ./android/build.gradle was not correctly set. It needs to be set to:
url "$rootDir/../node_modules/react-native/android"
instead of
url "$projectDir/../../node_modules/react-native/android"

@cosmith
Copy link

cosmith commented Sep 7, 2016

I have the same issue on 0.33-rc but it works on 0.32. Not sure what it's coming from...

@julienvincent
Copy link

I am getting this with RN0.33.0 and neither rn upgrade nor nuking node_modules solved this.

@mauron85
Copy link

mauron85 commented Sep 10, 2016

I've solved this by opening project in Android studio. Open RNPushNotification.java. Hit Ctrl+I (Implement methods). Choose onActivityResult. Delete former onActivityResult and save.

@julienvincent
Copy link

I looked at your commit - as far as I can see, nothing was actually changed except some new lines. Not sure how that is going to fix this issue?

@mauron85
Copy link

mauron85 commented Sep 10, 2016

@julienvincent that is actually the strange part. Compiler was complaining about not implementing method onActivityResult. So I've opened Android Studio -> implements method. It created new method onActivityResult and I deleted old declaration. Compiler was happy. And just after that I've realised the method declaration is same, so actually only empty lines were added. Very strange. However when I install module from npm, it happens again.

/Users/finch/dev/mapilary/Courier/node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:30: error: RNPushNotification is not abstract and does not override abstract method onActivityResult(Activity,int,int,Intent) in ActivityEventListener
public class RNPushNotification extends ReactContextBaseJavaModule implements ActivityEventListener {
       ^
../node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:168: error: method does not override or implement a method from a supertype
    @Override
    ^
2 errors

@mauron85
Copy link

mauron85 commented Sep 10, 2016

Ok. I've found that npm version is not same as master on github (but version numbers are the same). I've only checked build.gradle and it differs. Maybe that is the reason. Installed from github instead of npm and no compilation error occured.

So I would say npm version should be reuploaded.

@julienvincent
Copy link

The github version would differ due to the repo owner doing additional work on the project since last release. But I agree, it would be nice if @zo0r could release a new patch version to fix this problem. In the meantime I have found that using 2.0.1 does indeed fix this as mentioned by @skunkworker.

@mauron85
Copy link

Ok got you. So the problem is already fixed in master. So publishing new version to npm will also fix this problem.

@julienvincent
Copy link

Yes it would seem so :) I think I will switch to installing from github as well for now.

@zo0r
Copy link
Owner

zo0r commented Sep 10, 2016

Just published 2.1.0

@julienvincent
Copy link

@zo0r Thanks! 👍

@luisfuertes
Copy link

luisfuertes commented Sep 12, 2016

I have v2.0.1 and RN 0.29.1

And same error:

/Users/finch/dev/mapilary/Courier/node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:30: error: RNPushNotification is not abstract and does not override abstract method onActivityResult(Activity,int,int,Intent) in ActivityEventListener
public class RNPushNotification extends ReactContextBaseJavaModule implements ActivityEventListener {
       ^
../node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java:168: error: method does not override or implement a method from a supertype
    @Override
    ^
2 errors

How can i fix it?

@julienvincent
Copy link

@zo0r just commented saying he published 2.1.0 so give that version a try.

@luisfuertes
Copy link

luisfuertes commented Sep 12, 2016

But 2.1.0is for RN >= 0.33. I have RN v0.29.1 and cant upgrade at the moment the RN version

@julienvincent
Copy link

2.1.0 may still work on rn29, though I am not too sure.

@npomfret
Copy link
Contributor

I'm in the process of merging a different PR but can look at this after. Think it should be possible to get it working on both 33 and earlier.

@luisfuertes
Copy link

luisfuertes commented Sep 12, 2016

I upgrade the RNPN v2.1.0 on same RN v0.29.1. It works on iOS but on Android have the same problem.

@zo0r some idea of can i fix it?

@zo0r
Copy link
Owner

zo0r commented Sep 12, 2016

@luisfuertes for RN v0.29.1 try using 2.0.1
npm install [email protected]

@npomfret
Copy link
Contributor

I've just pushed PR #172 to master, along with a load of fixes to it. Included was a temporary change to make it compile on rn 0.32. Can someone who's on RN 0.33 please check if master still works for them?

@zo0r
Copy link
Owner

zo0r commented Sep 12, 2016

@npomfret nice, but I think there is a breaking change for RN 0.33 that not work for RN 0.32.

Also I compiled and tested it on RN 0.33 and its working. Just let me know when you want to release it to NPM.

@luisfuertes
Copy link

@zo0r before 2.1.0 i tried with 2.0.1 (I posted comment on this thread) but same problem.

@npomfret
Copy link
Contributor

npomfret commented Sep 12, 2016

That's good that it works on RN0.33 because I've been testing on rn0.32 (haddn't tried 33 yet). Hold off on the release though, it was quite a big change. Wouldn't mind getting a few people to try it first.

To fix the incompatibility problem I took out the @Override annotations, and left both implementations of the problematic method in. So I think thats why it works with RN33 and earlier now. It was a guess though, apologies if its not correct.

The annotations are only a compile-time tool and have no effect at runtime. In a sense they're totally optional. IMO it's always a good idea to use them, commenting them out is a hack, but until its decided that this project only supports the latests RN (maybe?) then I guess we can leave both implementations of the method in.

I'd suggest that supporting more than 1 version of RN is going to be a nightmare. Perhaps each time RN becomes incompatible we simply make a branch for the old one, and keep master compatible with the latest RN? Then just link to the branch on the README file for this people who want the old version.

@julienvincent
Copy link

julienvincent commented Sep 12, 2016

In my opinion there is no point for later versions of the project to support earlier versions of RN. As long as which package versions support which versions of RN is documented, then it will be better to provide support for only the current version of RN in master.

@npomfret
Copy link
Contributor

I'm inclined to agree. If people want new features and fixes they need to upgrade their version or RN. Otherwise they're free to fork and maintain their own version.

@luisfuertes
Copy link

luisfuertes commented Sep 13, 2016

For compile project i have to delete one parameter in OnActivityResult
../node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/modules/RNPushNotification.java

I change

@Override
public void onActivityResult(Activity activity, int requestCode, int resultCode, Intent data) {
...
}

for

@Override
public void onActivityResult(int requestCode, int resultCode, Intent data) {
...
}

Now project compile, but i have to try if it works.

EDIT: function onRegister never is called. I think it doesnt works now
EDIT2: Ok i had bad senderID, with other app senderID it return a token. I have to try to send me a push

@npomfret
Copy link
Contributor

Master currently contains both methods, but as I think the approach is to only support the latest RN that won't be the case going forward.

@juanpasolano
Copy link

was using RN 0.33 and upgrading to react-native-push-notification 2.1.0 worked, thank you!

@ftcvlad
Copy link

ftcvlad commented Aug 18, 2017

I am using React native 0.47.1 with react-native-push-notification 3.0.0. When trying to build I get

/node_modules/react-native-push-notification/android/src/main/java/com/dieam/reactnativepushnotification/ReactNativePushNotificationPackage.java:20: error: method does not override or implement a method from a supertype

works with rn 0.46.4

@pewh
Copy link

pewh commented Aug 18, 2017

@ftcvlad #526 is waiting to be merged. For temporary, you may install with npm install ximenean/react-native-push-notification#patch-1, then recompile android.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests