react-native-community / RNNewArchitectureLibraries

A collection of sample React Native Libraries that will show you how to use the New Architecture (Fabric & TurboModules) step-by-step.
MIT License
317 stars 25 forks source link

In an app based on the `feat/back-turbomodule-070` branch the "RN" prefix isn't auto-removed so the NativeModules property returns null #11

Open trevor-coleman opened 1 year ago

trevor-coleman commented 1 year ago

Summary:

Suggestion: Either change the example code to include the RN prefix, OR if the issue is with having a pascal-case module name then include a warning about selecting appropriate module names.

Background:

Per the react native docs:

If you do not specify a name, the JavaScript module name will match the Objective-C class name, with any "RCT" or "RK" prefixes removed.

In the example code the module is named RNCalculator but then accessed vial NativeModules.Calculator

But because the prefix is RN and not RCT or RK the prefix isn't removed.

My steps to reproduce

  1. Follow the example in the feat/back-turbomodule-070 branch, but changing the name from Calculator to TfLitening (And RNCalculator to RNTfLitening etc.)

  2. On the step [Native Module] Test The Native Module running the command npx react-native run-ios throws this error when you press the "Calculate" button.

 BUNDLE  ./index.js

 LOG  Running "OldArchitecture" with {"rootTag":21,"initialProps":{}}
 WARN  Possible Unhandled Promise Rejection (id: 0):
TypeError: Cannot read property 'add' of null
TypeError: Cannot read property 'add' of null
    at ?anon_0_ (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.OldArchitecture:93214:48)

I was able to correct the error after changing the code of my-module/src/index.js to include the RN prefix:

// tf-litening/src/index.js

import {NativeModules} from 'react-native'

export default NativeModules.RNTfLitening;

Notes

It's possible that the issue is that I have a PascalCase module name, or that it begins with a T?

If so then maybe changing the example code isn't necessary, but instead a small warning or hint about choosing the module name carefully could be helpful. Something like:

You can choose any module you like, but it may affect how the module is exported.

React Native will automatically remove some prefixes from module names (i.e. "RN" , "RCT", "RK", see the docs for more info) but if you choose a ["name in pascal case" | "name that begins with a T" | ... ] the prefix will not be removed and you'll need to include the prefix when you access it as a property of NativeModules.

Happy to submit a draft PR if this a seems worthwhile change.

cipolleschi commented 1 year ago

Yeah, I discovered that RCT_EXPORT_MODULE when used with a custom module name doesn't work well with TurboModules and we need to fix them, but we haven't had time to do so yet. Sorry for the disruption in the example.

It's possible that the issue is that I have a PascalCase module name, or that it begins with a T?

I don't think that that's the case. I think that it actually ignores the string that it is passed when the module is a TurboModule. My suggestion, so far is to use only the RCT_EXPORT_MODULE() version and export a turbomodule in javascript with the same name.

I was able to correct the error after changing the code of my-module/src/index.js to include the RN prefix:

// tf-litening/src/index.js
import {NativeModules} from 'react-native'
export default NativeModules.RNTfLitening;

Note that by doing this (☝️) you are not using TurboModules, but you are still using the Old Architecture's Native Modules.