react-native: "Breaking - remove unused registration of JS modules" really breaks stuff

Is this a bug report?

no. but as already discussd below this can lead to mayn bug reports in downstream projects

Have you read the Bugs section of the How to Contribute guide?

yes

Environment

react-native -v: v0.47.0-rc.5
node -v: v7.0.0
npm -v: 3.10.9

Target Platform: Android
Development Operating System: Win 10
Build tools: command line

Steps to Reproduce

Create a RN 0.47rc-5 project that uses https://github.com/ideacreation/react-native-barcodescanner or one of many other 3rd-party libs. Try to compile and run.

Expected Behavior

App will run as in 0.46

Actual Behavior

Compile error.

D:\Work\835a948d\0\products\msu.Reading\node_modules\react-native-barcodescanner\android\src\main\java\com\eguma\barcodescanner\BarcodeScannerPackage.java:20: error: method does not override or implement a method from a supertype
    @Override
    ^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
1 error
:react-native-barcodescanner:compileReleaseJavaWithJavac FAILED

FAILURE: Build failed with an exception.

Background

In https://github.com/facebook/react-native/commit/ce6fb337a146e6f261f2afb564aa19363774a7a8 @javache removed the JS registration because it’s now unnecessary. The issue is that a lot of the RN third-party components now need recompilation or just break (see https://github.com/ideacreation/react-native-barcodescanner/issues/79 for one example but quick google search shows more). That alone is not a problem but some components didn’t have updates dor a long time and maybe authors are hard to reach.

The question is: Is this really needed? What if we keep the abstract method is still there and just doesn’t call anymore. Then components wouldn’t need to recompile, right? This could save a lot of trouble in the ecosystem.

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 2
Comments: 18 (15 by maintainers)

Commits related to this issue

Fix Android compilation error with RN 0.47 Method createJSModules was removed from RN 0.47. Which leads to compilation error due to @override annotation. This method can be removed right now (https:/... — committed to YauhenBichel/react-native-bottomsheet by YauhenBichel 7 years ago
Fix Android compilation error with RN 0.47 Method createJSModules was removed from RN 0.47. Which leads to compilation error due to @override annotation. This method can be removed right now (faceboo... — committed to YauhenBichel/react-native-linear-gradient by YauhenBichel 7 years ago
Update ReactNativeRabbitMqPackage.java Fix for React Native >= 0.47 See issue https://github.com/facebook/react-native/issues/15232 — committed to treavis/react-native-rabbitmq by treavis 7 years ago
Fix compile error with RN >= 0.47 Slightly adjust the Android package according to a breaking change in RN 0.47. Just removing the `@Override` annotation is the suggested backwards-compatible fix; se... — committed to uniqlabs/react-native-audio-streamer by yelworc 6 years ago

Most upvoted comments

It is unfortunate that this has to be done in a breaking manner.A lot of third party packages are affected. I understand the technical issue, and while I don’t want to come across whining, it does seem that every release of react-native has some issue or another that prevents upgrading. 0.46 for example was unbuildable on iOS devices in the first release. Now this. It does create quite a bit of friction in the developer experience unfortunately 😦

These days, I dread doing a react-native upgrade…

ujwal-setlur on Aug 4, 2017

@hramos, I understand the challenges. I really love react-native and want to see it get even more awesome. Some thoughts:

I think you guys need to have a more formal ecosystem partnership with third party package providers so that they understand what’s coming down the pipe. A lot of packages are slow to catch up to changes to react-native interfaces. For example, your own react-native-fbsdk oAuth module itself had not caught up with this change. I had to pull from master. If this mechanism is already in place, it needs to get better.
Some packages seem just abandoned, things that are quite critical to a modern app. This is the dark side of open source. A proliferation of forks come about because the package maintainer is MIA. I myself have forked a few because I needed critical fixes. I don’t know what the solution to this is, but maybe Facebook can take more ownership of some critical native modules.
All the issues that I found, Facebook should have found too given that you consume this for your apps. I have been been regularly upgrading to official releases, but have stayed away from rc-releases. I am building a production app and am nervous about switching to non official releases, but I am willing to try on some of myth other projects.

All in all, I want my developer experience to improve, and I am willing to help. I now have modified my build system by creating a script that patches third party boo-boos post npm install 😃

ujwal-setlur on Aug 4, 2017

We’re working on making this better. Several things have happened over the past couple of months that have led to this:

We haven’t had enough people using the release candidate versions of React Native. As a result, show-stopper issues are not always found until a stable branch is cut and released to everyone.
The switch to the React 16 alpha broke Enzyme, which prevented some of our larger and most vocal core contributors from upgrading to the last few React Native release candidates, which leads back to my previous point on surfacing serious issues.
Finally, we’ve had a serious case of issues going out of control. For the last few months, the number of open issues has remained in the ~1,200 range. We’ve found that some of these show stoppers were indeed reported, but they’re quickly drowned out by the other 100 issues that get opened each week. Thankfully we’re now down to 600 open issues, and being stricter about following the template has helped us slow down the number of new issues that get opened without sufficient information.

There’s a few ways you can help us here: use the release candidates! Upgrade to the RC and let us know ASAP if you run into any issues. When opening an issue, make sure to follow the template, and make note that the issue is a show-stopper that needs to be addressed before the next stable release is cut.

If you’re not able to use the release candidates, you can also help us by triaging open issues and flagging those that need to be looked at before a stable branch is cut.

This last point helps more than you might think. We use React Native from master here at Facebook, and issues that affect our own apps are addressed quickly. By helping us triage open issues, you may discover something that is already fixed in master but needs to be cherry-picked into the release candidate prior to the branch cut.

hramos on Aug 4, 2017

For maintainers of those packages: looks like the best change is just to remove the @Override line, while still implementing the function.

That will ensure compatibility with both 0.46.x and 0.47.x.

See e.g. https://github.com/geektimecoil/react-native-onesignal/pull/294

Also see react-native-code-push which removed the whole function, then was forced to release a new version that was broken on 0.46.x.

lukewlms on Aug 2, 2017