Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
friyiajr opened this issue Nov 26, 2022 · 15 comments
Closed

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

friyiajr opened this issue Nov 26, 2022 · 15 comments
Labels
Resolution: Answered When the issue is resolved with a simple answer Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@friyiajr
Copy link

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

@friyiajr friyiajr added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Nov 26, 2022
@cipolleschi
Copy link
Contributor

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
Copy link
Author

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.

@cortinico cortinico added Resolution: Answered When the issue is resolved with a simple answer and removed Needs: Triage 🔍 labels Nov 29, 2022
@cipolleschi
Copy link
Contributor

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
Copy link
Author

friyiajr commented Dec 2, 2022

@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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

cipolleschi commented Dec 20, 2022

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

  • Look at some Android module and see how it does that. I expect something similar: extend a Java EventEmitter base class and call the proper method
  • wait for @cortinico to come back! 😅

Luckily, the JS side should stay the same! 😉

@friyiajr
Copy link
Author

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
Copy link
Author

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

@cipolleschi
Copy link
Contributor

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

@antondrob
Copy link

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

@brunoro
Copy link

brunoro commented Jul 15, 2024

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

@cipolleschi
Copy link
Contributor

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!

@yaroslav-beletsky
Copy link

@cipolleschi Are there any examples of working with events for Android on the new architecture?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Answered When the issue is resolved with a simple answer Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

6 participants