From e4ab223d9b0295dff5693e79924e80da9b5ff1a0 Mon Sep 17 00:00:00 2001 From: Vincent Riemer <1398555+vincentriemer@users.noreply.github.com> Date: Mon, 10 Jul 2023 16:34:44 -0700 Subject: [PATCH] Refactor event pipeline to accept typed event payloads (#38276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/38276 Changelog: [Internal] Refactor Fabric event pipeline to accept typed event payloads As a first step towards my project of managing the Pointer Capture API by intercepting events in UIManagerBinding I need infrastructure to be able to safely & efficently read the properties of the event payload which is what this diff lays the ground work of addressing. Currently the events are passed from the EventEmitter all the way to UIManagerBinding with only a ValueFactory (std::function lambda) which is called to produce the jsi::Value. My diff introduces a new virtual base class, EventPayload, which provides the same functionality but in a member function, asJSIValue. To ease the transition to this new paradigm I also introduce the first concrete subclass of EventPayload: ValueFactoryEventPayload — which simply stores and calls a ValueFactory so that we can incrementally migrate to typed events (or frankly, continue to be used for events that we don't *need* to be typed, as the only real use-case in the beginning will be for Pointer Events). This diff notably does not change any behavior and should behave the exact same way it did before — it is in later diffs where I will begin applying this to the pointer events. Differential Revision: D47299631 fbshipit-source-id: 91c1fbf8af18bf217754f6560a00764712c1d5b6 --- .../react/renderer/core/EventEmitter.cpp | 4 +-- .../react/renderer/core/EventEmitter.h | 2 ++ .../react/renderer/core/EventPayload.h | 29 +++++++++++++++++++ .../react/renderer/core/EventPipe.h | 3 +- .../renderer/core/EventQueueProcessor.cpp | 9 +++++- .../react/renderer/core/RawEvent.cpp | 4 +-- .../react/renderer/core/RawEvent.h | 6 ++-- .../core/ValueFactoryEventPayload.cpp | 19 ++++++++++++ .../renderer/core/ValueFactoryEventPayload.h | 24 +++++++++++++++ .../core/tests/EventQueueProcessorTest.cpp | 17 ++++++----- .../react/renderer/scheduler/Scheduler.cpp | 4 +-- .../renderer/uimanager/UIManagerBinding.cpp | 4 +-- .../renderer/uimanager/UIManagerBinding.h | 2 +- 13 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 packages/react-native/ReactCommon/react/renderer/core/EventPayload.h create mode 100644 packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.cpp create mode 100644 packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.h diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp index 22a12a2ffb43fe..ecc23247180051 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp @@ -84,7 +84,7 @@ void EventEmitter::dispatchEvent( eventDispatcher->dispatchEvent( RawEvent( normalizeEventType(std::move(type)), - payloadFactory, + std::make_shared(payloadFactory), eventTarget_, category), priority); @@ -102,7 +102,7 @@ void EventEmitter::dispatchUniqueEvent( eventDispatcher->dispatchUniqueEvent(RawEvent( normalizeEventType(std::move(type)), - payloadFactory, + std::make_shared(payloadFactory), eventTarget_, RawEvent::Category::Continuous)); } diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.h b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.h index c02d605b43b77d..df0c12c93a4f76 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventEmitter.h @@ -12,9 +12,11 @@ #include #include +#include #include #include #include +#include namespace facebook::react { diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventPayload.h b/packages/react-native/ReactCommon/react/renderer/core/EventPayload.h new file mode 100644 index 00000000000000..4d30cf5ec5f952 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/core/EventPayload.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include + +namespace facebook::react { + +/** + * Abstract base class for all event payload types. + */ +struct EventPayload { + virtual ~EventPayload() = default; + + EventPayload() = default; + EventPayload(const EventPayload &) = default; + EventPayload &operator=(const EventPayload &) = default; + EventPayload(EventPayload &&) = default; + EventPayload &operator=(EventPayload &&) = default; + + virtual jsi::Value asJSIValue(jsi::Runtime &runtime) const = 0; +}; + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h b/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h index 53d41488418d3e..a31bc54d5ad5c5 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h +++ b/packages/react-native/ReactCommon/react/renderer/core/EventPipe.h @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -22,6 +23,6 @@ using EventPipe = std::function; + const EventPayload &payload)>; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp index 4ff9406f60bbe1..9ba86af056e8c4 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/EventQueueProcessor.cpp @@ -6,6 +6,7 @@ */ #include +#include #include "EventEmitter.h" #include "EventLogger.h" #include "EventQueue.h" @@ -53,12 +54,18 @@ void EventQueueProcessor::flushEvents( eventLogger->onEventDispatch(event.loggingTag); } + if (event.eventPayload == nullptr) { + react_native_log_error( + "EventQueueProcessor: Unexpected null event payload"); + continue; + } + eventPipe_( runtime, event.eventTarget.get(), event.type, reactPriority, - event.payloadFactory); + *event.eventPayload); if (eventLogger != nullptr) { eventLogger->onEventEnd(event.loggingTag); diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawEvent.cpp b/packages/react-native/ReactCommon/react/renderer/core/RawEvent.cpp index 646ce4aea3ad86..a636ebfe143fe2 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawEvent.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/RawEvent.cpp @@ -11,11 +11,11 @@ namespace facebook::react { RawEvent::RawEvent( std::string type, - ValueFactory payloadFactory, + std::shared_ptr eventPayload, SharedEventTarget eventTarget, Category category) : type(std::move(type)), - payloadFactory(std::move(payloadFactory)), + eventPayload(std::move(eventPayload)), eventTarget(std::move(eventTarget)), category(category) {} diff --git a/packages/react-native/ReactCommon/react/renderer/core/RawEvent.h b/packages/react-native/ReactCommon/react/renderer/core/RawEvent.h index d9ec02c796e79a..472f2d92c37936 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/RawEvent.h +++ b/packages/react-native/ReactCommon/react/renderer/core/RawEvent.h @@ -11,8 +11,8 @@ #include #include +#include #include -#include namespace facebook::react { @@ -60,12 +60,12 @@ struct RawEvent { RawEvent( std::string type, - ValueFactory payloadFactory, + std::shared_ptr eventPayload, SharedEventTarget eventTarget, Category category = Category::Unspecified); std::string type; - ValueFactory payloadFactory; + std::shared_ptr eventPayload; SharedEventTarget eventTarget; Category category; EventTag loggingTag{0}; diff --git a/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.cpp b/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.cpp new file mode 100644 index 00000000000000..fc5e81d3646f76 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.cpp @@ -0,0 +1,19 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "ValueFactoryEventPayload.h" + +namespace facebook::react { + +ValueFactoryEventPayload::ValueFactoryEventPayload(ValueFactory factory) + : valueFactory_(std::move(factory)) {} + +jsi::Value ValueFactoryEventPayload::asJSIValue(jsi::Runtime &runtime) const { + return valueFactory_(runtime); +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.h b/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.h new file mode 100644 index 00000000000000..08486966dd5905 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/core/ValueFactoryEventPayload.h @@ -0,0 +1,24 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook::react { + +class ValueFactoryEventPayload : public EventPayload { + public: + explicit ValueFactoryEventPayload(ValueFactory factory); + jsi::Value asJSIValue(jsi::Runtime &runtime) const override; + + private: + ValueFactory valueFactory_; +}; + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp index e88013b9e555b6..3bc03bf1a39974 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/EventQueueProcessorTest.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include @@ -26,7 +27,7 @@ class EventQueueProcessorTest : public testing::Test { const EventTarget * /*eventTarget*/, const std::string &type, ReactEventPriority priority, - const ValueFactory & /*payloadFactory*/) { + const EventPayload & /*payload*/) { eventTypes_.push_back(type); eventPriorities_.push_back(priority); }; @@ -49,7 +50,7 @@ TEST_F(EventQueueProcessorTest, singleUnspecifiedEvent) { *runtime_, {RawEvent( "my type", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::Unspecified)}); @@ -63,22 +64,22 @@ TEST_F(EventQueueProcessorTest, continuousEvent) { *runtime_, {RawEvent( "touchStart", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::ContinuousStart), RawEvent( "touchMove", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::Unspecified), RawEvent( "touchEnd", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::ContinuousEnd), RawEvent( "custom event", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::Unspecified)}); @@ -103,7 +104,7 @@ TEST_F(EventQueueProcessorTest, alwaysContinuousEvent) { { RawEvent( "onScroll", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::Continuous), }); @@ -120,7 +121,7 @@ TEST_F(EventQueueProcessorTest, alwaysDiscreteEvent) { { RawEvent( "onChange", - dummyValueFactory_, + std::make_shared(dummyValueFactory_), nullptr, RawEvent::Category::Discrete), }); diff --git a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp index 1d812fcccf845a..990912be9cbe77 100644 --- a/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp @@ -60,11 +60,11 @@ Scheduler::Scheduler( const EventTarget *eventTarget, const std::string &type, ReactEventPriority priority, - const ValueFactory &payloadFactory) { + const EventPayload &payload) { uiManager->visitBinding( [&](UIManagerBinding const &uiManagerBinding) { uiManagerBinding.dispatchEvent( - runtime, eventTarget, type, priority, payloadFactory); + runtime, eventTarget, type, priority, payload); }, runtime); if (runtimeScheduler != nullptr) { diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index ef4100ef6c4df9..e56132e02c226f 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -93,10 +93,10 @@ void UIManagerBinding::dispatchEvent( EventTarget const *eventTarget, std::string const &type, ReactEventPriority priority, - ValueFactory const &payloadFactory) const { + const EventPayload &eventPayload) const { SystraceSection s("UIManagerBinding::dispatchEvent", "type", type); - auto payload = payloadFactory(runtime); + auto payload = eventPayload.asJSIValue(runtime); // If a payload is null, the factory has decided to cancel the event if (payload.isNull()) { diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.h b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.h index b2180ea4ede328..7f5adc03f4c122 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.h +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.h @@ -52,7 +52,7 @@ class UIManagerBinding : public jsi::HostObject { EventTarget const *eventTarget, std::string const &type, ReactEventPriority priority, - ValueFactory const &payloadFactory) const; + const EventPayload &payload) const; /* * Invalidates the binding and underlying UIManager.