facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.27k stars 24.35k forks source link

Codegen for TurboModules does not allow for Events to be Emitted #35488

Closed friyiajr closed 1 year ago

friyiajr commented 1 year ago

Description

I am working on an app that requires a native module to be able to emit random events. In my case I am building a pedometer and steps can come from the native part of the application at random times. I can't use simple method calls because of this behaviour. I also want to use a BLE heart rate monitor which again requires the ability to use random events because the sensor data comes in at random times.

I tried to use DirectEventHandler in my Codegen file and the codegen compiler just shows errors. My interface code looks like this:

import type { TurboModule } from 'react-native';
import { TurboModuleRegistry } from 'react-native';
import type {
  DirectEventHandler,
  Int32,
} from 'react-native/Libraries/Types/CodegenTypes';

export interface Spec extends TurboModule {
  multiply(a: number, b: number): number;
  add(a: number, b: number): number;
  randomEvent: DirectEventHandler<{ data: Int32 }>;
}

export default TurboModuleRegistry.getEnforcing<Spec>('RtmPedometer');

If I comment out the randomEvent code then Codegen works perfectly fine and I have no issues. This seems to be the only type in the documentation we have for event emitting unless I am missing something. This issue is especially frustrating because this was easy to do with the old arch and is really well documented. See: https://reactnative.dev/docs/native-modules-android#sending-events-to-javascript . This bug with DirectEventHandler is integral to how TurboModules should work and I feel like it should be a pretty high priority fix.

Version

0.70.5

Output of npx react-native info

System:
    OS: macOS 12.5
    CPU: (10) arm64 Apple M1 Pro
    Memory: 149.16 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.15.0 - /usr/local/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.11.3 - /Users/danielfriyia/.rvm/gems/ruby-3.0.0/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 16.0, macOS 12.3, tvOS 16.0, watchOS 9.0
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.2 AI-212.5712.43.2112.8815526
    Xcode: 14.0/14A309 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.16 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.1.0 => 18.1.0 
    react-native: 0.70.5 => 0.70.5 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

Clone the Reproducer Repo: https://github.com/friyiajr/EventBugReproducer

Run the command yarn bootstrap

At this point you will see it fail. If you want it to succeed comment out the randomEvent line along with its import statements.

Snack, code example, screenshot, or link to a repository

https://github.com/friyiajr/EventBugReproducer

cipolleschi commented 1 year ago

Hi @friyiajr, thank you for asking the question.

What you are trying to achieve is valid, but DirectEventHandler is not the right tool. That type must only be used with Fabric Native Components and not with Native TurboModules (TM).

The Codegen fails because DirectEventHandler is not a valid type for TMs.

The right way to implement what you are trying to do is by using NativeEventEmitter.

The implementation should go roughly right that:

  1. You make your TM extends to RCTEventEmitter
  2. You invoke the sendEventWithName in your TM logic
  3. In JS, you register to the event emitter to handle the events.

As an example you can look at the StatusBarManager

We know that the documentation is lacking in this respect. We are working hard to cover this use cases. Thank you for reporting the problem!

friyiajr commented 1 year ago

Hey @cipolleschi thanks for your response 🙂. Your listening example doesn't show me how to handle the events that come in. Could you possibly highlight the line you are referring too. All I see here is the ability to add a listener using the string. I don't see how you actually listen for the frame coordinates you are emitting.

cipolleschi commented 1 year ago

Uh, sorry, I haven't realized a missed the last step. I think this is the missing piece: EventEmitter.js.

Basically, you instantiate the EventEmitter.js which is a global object, and you subscribe to the event calling emitter.on('your_event_name', (event) => {<your code>}).

I haven't tried this out, let me know if it works. PS: i'd be on PTO from tomorrow till next Thursday. Happy to continue on Friday if there are other issues.

friyiajr commented 1 year ago

@cipolleschi I updated the reproducer. I can compile and get the event to work, the problem is, the normal method calls stopped working. If you console log the exports my add method gets overwritten by addListeners and removeListeners. Can you let me know why this is happening and how I can fix this?

cipolleschi commented 1 year ago

@friyiajr, sorry for the late response. I don't have an answer right now, I think we have to dig a little deeper into this.

cipolleschi commented 1 year ago

@friyiajr, I prepared a guide for how to use the EventEmitter and Swift - if you don't need Swift, it should be fairly easy to remove the Swift specific code and to implement it in Objective-C++.

You can find it here, I hope it helps!

friyiajr commented 1 year ago

@cipolleschi thanks this is great! Does anything like this exist for Android? I need to be able to get this working on both platforms

cipolleschi commented 1 year ago

Not yet. I'm not an Android expert, I can try to hack something together. Other things we can do:

Luckily, the JS side should stay the same! 😉

friyiajr commented 1 year ago

Thanks for letting me know. I'll see how far I can get on my own. Hopefully @cortinico can work on something for us when they get back

friyiajr commented 1 year ago

Yeah, no luck 😅. Please publish a guide when you get back if you can @cortinico

cipolleschi commented 1 year ago

@friyiajr a small heads up that there is typo in the podspec of the guide I prepared: https://github.com/react-native-community/RNNewArchitectureLibraries/pull/10. Thanks to @TheInkedEngineer for finding it.

antondrob commented 1 year ago

Hi, are there any plans to publish Android sample for this?

brunoro commented 4 months ago

Android samples would be amazing! Any updates? @cipolleschi @cortinico

cipolleschi commented 4 months ago

Hi there, we are working heads down to fix the last issues of the New Architecture. Our plan is to revisit the documentation towards the end of August, beginning of September. We will create more examples around that time!