Better error messages for ReactPackage missing empty list implementation

[Nkzn]

Motivation

When we create custom ReactPackage for Native Modules or Native UI Components, we often forget to add Collections.emptyList() and produce NullPointerException repeatedly. This error log remind us to add empty code.

And also, the NPE message from this probrem is ambiguous. This fix is useful for RN beginners.

Test Plan

This test can do in the RNTester manually.

Test ReactInstanceManager

To show error log in ReactInstanceManager, add ReactPackage into RNTesterApplication like below.

@Override
public List<ReactPackage> getPackages() {
  return Arrays.<ReactPackage>asList(
    new MainReactPackage(),
    new ReactPackage() {
        @Override
        public List<NativeModule> createNativeModules(ReactApplicationContext reactContext) {
        return Collections.emptyList();
        }

        @Override
        public List<ViewManager> createViewManagers(ReactApplicationContext reactContext) {
        return null;
        }
    }
  );
}

And run RNTester as Android App, the error log will show in LogCat. (with app crashes)

Test NativeModuleRegistryBuilder

Same as test for NativeModuleRegistryBuilder, write like below.

@Override
public List<ReactPackage> getPackages() {
  return Arrays.<ReactPackage>asList(
    new MainReactPackage(),
    new ReactPackage() {
        @Override
        public List<NativeModule> createNativeModules(ReactApplicationContext reactContext) {
        return null;
        }

        @Override
        public List<ViewManager> createViewManagers(ReactApplicationContext reactContext) {
        return Collections.emptyList();
        }
    }
  );
}

And run RNTester as Android App, the error log will show in LogCat. (without app crashes)

Release Notes

[ANDROID] [ENHANCEMENT] [ReactPackage] - Add error log for ReactPackage missing empty list implementation

[Nkzn]

Bad Credentials? 😕 > ci/circleci: analyze_pr

[Nkzn]

Why tvos ci fails... I edited only android....

[facebook-github-bot]

@Nkzn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[Nkzn]

thanks bot! I rebased.

[Nkzn]

why ci fail...? i rebased again.

[facebook-github-bot]

@Nkzn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[facebook-github-bot]

@Nkzn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[facebook-github-bot]

@Nkzn I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[hey99xx]

better to use reactPage.getClass().getSimpleName() to make it easy to identify which package got it wrong.

[cpojer]

Thank you for this Pull Request. This is definitely useful. We are currently evaluating how to standardize and make it easier to create native modules and will make sure that error handling works better, see react-native-community/discussions-and-proposals#96. Unfortunately the code here is outdated (I'm sorry we didn't get to it earlier) so I have to close the PR :(