Implement REACT_EVENT_EMITTER and add to the SampleTurboModule module #13533
Conversation
microsoft-github-policy-service
bot
added
Area: Turbo Modules
New Architecture
This PR provides a proper implementation of the `SampleTurboModule` module, and removes the proxy code in `TurboModuleManager` which instead substituted the old `SampleTurboCxxModule` module. Closes microsoft#13531
jonthysell
force-pushed
the
sampleturbomodule
branch
from
5d3056b to
a1b543b
Compare
jonthysell
changed the title
| #define REACT_EVENT(/* field, [opt] eventName, [opt] eventEmitterName */...) \ | ||
| INTERNAL_REACT_MEMBER(__VA_ARGS__)(EventField, __VA_ARGS__) | ||
|
|
||
| // REACT_EVENT_EMITTER(field, [opt] eventName, [opt] eventEmitterName) |
| auto eventEmitterInitializer = ModuleEventEmitterFieldInfo<TField>::GetEventEmitterInitializer(m_module, field); | ||
|
|
||
| m_moduleBuilder.AddInitializer(eventHandlerEmptyInitializer); | ||
| m_moduleBuilder.AddEventEmitter(eventName, eventEmitterInitializer); |
| m_methods.push_back(std::move(cxxMethod)); | ||
| } | ||
|
|
||
| void ReactModuleBuilder::AddEventEmitter(hstring const &name, EventEmitterInitializerDelegate const &emitter) noexcept { |
There was a problem hiding this comment.
Event emitters are only supported on TurboModules. The ReactModuleBuilder is used when the module is running as a native module. We could choose to throw and not allow the module to run at all if it's being run as a NativeModule. I was leaning towards allowing at least the rest of the module to continue to work if its run in that mode. But I'd be ok making this throw.
vmoroz
left a comment
vmoroz
left a comment
There was a problem hiding this comment.
Looking through the code it is not clear how the new code works.
Can we simply reuse the existing events?
If not then why the new EventEmitters cannot be implemented the same way as the Events.
Currently I see that the EventEmitter registration calls AddInitializer and the new AddEventEmitter. Both of them are no-op.
microsoft-github-policy-service
bot
added
the
Needs: Author Feedback
No, the new code works differently than the old events. To listen to the old events you would do something like: let subscription = RCTDeviceEventEmitter.addListener(<EVENTNAME>, handler);where has to be unique across all native modules. In the new world you do: import MyModule from 'NativeMyModule'
let subscription = MyModule.<EVENTNAME>(handler);The AddInitializer call sets the native event std::function to a no-op function. Since if no-one has registered for the event, the std::function can be a no-op. |
microsoft-github-policy-service
bot
added
Needs: Attention 👋
jonthysell
removed
the
Needs: Attention 👋
3 participants
Description
This PR provides a proper implementation of the
SampleTurboModulemodule, with the new EventEmitter events (coming in #13508) and removes the proxy code inTurboModuleManagerwhich instead substituted the oldSampleTurboCxxModulemodule.Type of Change
Why
The APIs of
SampleTurboModuleare starting to deviate from the olderSampleTurboCxxModule, specifically the addtion of newEventEmittermembers. So it's time we had a implementation for both.Closes #13531
Partial implementation of #13532
What
What changes were made to the codebase to solve the bug, add the functionality, etc. that you specified above.
Screenshots
Add any relevant screen captures here from before or after your changes.
Testing
Verified tests still pass and the new module is being called.
Changelog
Should this change be included in the release notes: yes
Implement the SampleTurboModule module
Microsoft Reviewers: Open in CodeFlow