Skip to content

Commit 3250725

Browse files
rubennortefacebook-github-bot
authored andcommitted
Expose instance handles from shadow nodes directly, instead of from event emitters/targets (#37553)
Summary: Pull Request resolved: #37553 ## Context I added some getters in EventTarget/EventEmitter to access the React instance handle for a given shadow node/shadow node family in 43864a1 (D44022477), so I could implement DOM traversal methods easily. I wanted to reuse that method to implement `MutationObserver` and `IntersectionObserver`, but unfortunately `EventEmitter`/`EventTarget` only allow access to the instance handle as long as the shadow node is mounted. That makes sense for events, but not for this use case, as with `MutationObserver` we intentionally want to have access to unmounted nodes when creating the list of `addedNodes` and `removedNodes` for the `MutationRecord` (depending on when exactly we generate this list, either `addedNodes` or `removedNodes` wouldn't be mounted). ## Changes This reverts my original change and adds a new field in `ShadowNodeFamily` to provide access to the `InstanceHandle` at any point (even if the node isn't mounted). This provides the same guarantees as the original method, keeping weak references to the handles to avoid memory leaks. Changelog: [internal] Reviewed By: sammy-SC Differential Revision: D46149084 fbshipit-source-id: f76abae50134a5d55a98cab42eebeb62084024f9
1 parent a88c0ed commit 3250725

27 files changed

+195
-148
lines changed

packages/react-native/ReactCommon/react/renderer/animations/tests/LayoutAnimationTest.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ static void testShadowNodeTreeLifeCycleLayoutAnimations(
5656
ViewComponentDescriptor(componentDescriptorParameters);
5757
auto rootComponentDescriptor =
5858
RootComponentDescriptor(componentDescriptorParameters);
59-
auto noopEventEmitter =
60-
std::make_shared<ViewEventEmitter const>(nullptr, -1, eventDispatcher);
6159

6260
PropsParserContext parserContext{-1, *contextContainer};
6361

@@ -95,7 +93,7 @@ static void testShadowNodeTreeLifeCycleLayoutAnimations(
9593
auto surfaceId = SurfaceId(surfaceIdInt);
9694

9795
auto family = rootComponentDescriptor.createFamily(
98-
{Tag(surfaceIdInt), surfaceId, nullptr}, nullptr);
96+
{Tag(surfaceIdInt), surfaceId, nullptr});
9997

10098
// Creating an initial root shadow node.
10199
auto emptyRootNode = std::const_pointer_cast<RootShadowNode>(

packages/react-native/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,13 @@ ShadowNode::Shared ComponentDescriptorRegistry::createNode(
126126
std::string const &viewName,
127127
SurfaceId surfaceId,
128128
folly::dynamic const &propsDynamic,
129-
SharedEventTarget const &eventTarget) const {
129+
InstanceHandle::Shared const &instanceHandle) const {
130130
auto unifiedComponentName = componentNameByReactViewName(viewName);
131131
auto const &componentDescriptor = this->at(unifiedComponentName);
132132

133-
auto const fragment = ShadowNodeFamilyFragment{tag, surfaceId, nullptr};
134-
auto family = componentDescriptor.createFamily(fragment, eventTarget);
133+
auto const fragment =
134+
ShadowNodeFamilyFragment{tag, surfaceId, instanceHandle};
135+
auto family = componentDescriptor.createFamily(fragment);
135136
auto const props = componentDescriptor.cloneProps(
136137
PropsParserContext{surfaceId, *contextContainer_.get()},
137138
nullptr,

packages/react-native/ReactCommon/react/renderer/componentregistry/ComponentDescriptorRegistry.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include <react/renderer/componentregistry/ComponentDescriptorProvider.h>
1616
#include <react/renderer/core/ComponentDescriptor.h>
17+
#include <react/renderer/core/InstanceHandle.h>
1718
#include <react/utils/ContextContainer.h>
1819

1920
namespace facebook::react {
@@ -59,7 +60,7 @@ class ComponentDescriptorRegistry {
5960
std::string const &viewName,
6061
SurfaceId surfaceId,
6162
folly::dynamic const &props,
62-
SharedEventTarget const &eventTarget) const;
63+
InstanceHandle::Shared const &instanceHandle) const;
6364

6465
void setFallbackComponentDescriptor(
6566
const SharedComponentDescriptor &descriptor);

packages/react-native/ReactCommon/react/renderer/core/ComponentDescriptor.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#pragma once
99

1010
#include <react/renderer/core/EventDispatcher.h>
11+
#include <react/renderer/core/EventEmitter.h>
12+
#include <react/renderer/core/InstanceHandle.h>
1113
#include <react/renderer/core/Props.h>
1214
#include <react/renderer/core/PropsParserContext.h>
1315
#include <react/renderer/core/RawPropsParser.h>
@@ -136,8 +138,13 @@ class ComponentDescriptor {
136138
* Creates a shadow node family for particular node.
137139
*/
138140
virtual ShadowNodeFamily::Shared createFamily(
139-
ShadowNodeFamilyFragment const &fragment,
140-
SharedEventTarget eventTarget) const = 0;
141+
ShadowNodeFamilyFragment const &fragment) const = 0;
142+
143+
/*
144+
* Creates an event emitter for particular node.
145+
*/
146+
virtual SharedEventEmitter createEventEmitter(
147+
InstanceHandle::Shared const &instanceHandle) const = 0;
141148

142149
protected:
143150
EventDispatcher::Weak eventDispatcher_;

packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -180,17 +180,20 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
180180
}
181181

182182
ShadowNodeFamily::Shared createFamily(
183-
ShadowNodeFamilyFragment const &fragment,
184-
SharedEventTarget eventTarget) const override {
185-
auto eventEmitter = std::make_shared<ConcreteEventEmitter const>(
186-
std::move(eventTarget), fragment.tag, eventDispatcher_);
183+
ShadowNodeFamilyFragment const &fragment) const override {
187184
return std::make_shared<ShadowNodeFamily>(
188185
ShadowNodeFamilyFragment{
189-
fragment.tag, fragment.surfaceId, eventEmitter},
186+
fragment.tag, fragment.surfaceId, fragment.instanceHandle},
190187
eventDispatcher_,
191188
*this);
192189
}
193190

191+
SharedEventEmitter createEventEmitter(
192+
InstanceHandle::Shared const &instanceHandle) const override {
193+
return std::make_shared<ConcreteEventEmitter const>(
194+
std::make_shared<EventTarget>(instanceHandle), eventDispatcher_);
195+
}
196+
194197
protected:
195198
/*
196199
* Called immediately after `ShadowNode` is created or cloned.

packages/react-native/ReactCommon/react/renderer/core/EventEmitter.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ ValueFactory EventEmitter::defaultPayloadFactory() {
4343

4444
EventEmitter::EventEmitter(
4545
SharedEventTarget eventTarget,
46-
Tag /*tag*/,
4746
EventDispatcher::Weak eventDispatcher)
4847
: eventTarget_(std::move(eventTarget)),
4948
eventDispatcher_(std::move(eventDispatcher)) {}
@@ -131,8 +130,4 @@ void EventEmitter::setEnabled(bool enabled) const {
131130
}
132131
}
133132

134-
const SharedEventTarget &EventEmitter::getEventTarget() const {
135-
return eventTarget_;
136-
}
137-
138133
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/core/EventEmitter.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ class EventEmitter {
3838

3939
EventEmitter(
4040
SharedEventTarget eventTarget,
41-
Tag tag,
4241
EventDispatcher::Weak eventDispatcher);
4342

4443
virtual ~EventEmitter() = default;
@@ -55,8 +54,6 @@ class EventEmitter {
5554
*/
5655
void setEnabled(bool enabled) const;
5756

58-
SharedEventTarget const &getEventTarget() const;
59-
6057
protected:
6158
#ifdef ANDROID
6259
// We need this temporarily due to lack of Java-counterparts for particular

packages/react-native/ReactCommon/react/renderer/core/EventTarget.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,9 @@ namespace facebook::react {
1313

1414
using Tag = EventTarget::Tag;
1515

16-
EventTarget::EventTarget(
17-
jsi::Runtime &runtime,
18-
jsi::Value const &instanceHandle,
19-
Tag tag)
20-
: weakInstanceHandle_(
21-
jsi::WeakObject(runtime, instanceHandle.asObject(runtime))),
22-
strongInstanceHandle_(jsi::Value::null()),
23-
tag_(tag) {}
16+
EventTarget::EventTarget(InstanceHandle::Shared instanceHandle)
17+
: instanceHandle_(std::move(instanceHandle)),
18+
strongInstanceHandle_(jsi::Value::null()) {}
2419

2520
void EventTarget::setEnabled(bool enabled) const {
2621
enabled_ = enabled;
@@ -31,7 +26,7 @@ void EventTarget::retain(jsi::Runtime &runtime) const {
3126
return;
3227
}
3328

34-
strongInstanceHandle_ = weakInstanceHandle_.lock(runtime);
29+
strongInstanceHandle_ = instanceHandle_->getInstanceHandle(runtime);
3530

3631
// Having a `null` or `undefined` object here indicates that
3732
// `weakInstanceHandle_` was already deallocated. This should *not* happen by
@@ -62,7 +57,7 @@ jsi::Value EventTarget::getInstanceHandle(jsi::Runtime &runtime) const {
6257
}
6358

6459
Tag EventTarget::getTag() const {
65-
return tag_;
60+
return instanceHandle_->getTag();
6661
}
6762

6863
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/core/EventTarget.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77

88
#pragma once
99

10-
#include <memory>
11-
1210
#include <jsi/jsi.h>
11+
#include <react/renderer/core/InstanceHandle.h>
12+
#include <memory>
1313

1414
namespace facebook::react {
1515

@@ -34,7 +34,7 @@ class EventTarget {
3434
/*
3535
* Constructs an EventTarget from a weak instance handler and a tag.
3636
*/
37-
EventTarget(jsi::Runtime &runtime, jsi::Value const &instanceHandle, Tag tag);
37+
explicit EventTarget(InstanceHandle::Shared instanceHandle);
3838

3939
/*
4040
* Sets the `enabled` flag that allows creating a strong instance handle from
@@ -65,10 +65,9 @@ class EventTarget {
6565
Tag getTag() const;
6666

6767
private:
68+
const InstanceHandle::Shared instanceHandle_;
6869
mutable bool enabled_{false}; // Protected by `EventEmitter::DispatchMutex()`.
69-
mutable jsi::WeakObject weakInstanceHandle_; // Protected by `jsi::Runtime &`.
7070
mutable jsi::Value strongInstanceHandle_; // Protected by `jsi::Runtime &`.
71-
Tag tag_;
7271
};
7372

7473
using SharedEventTarget = std::shared_ptr<const EventTarget>;
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include "InstanceHandle.h"
9+
10+
namespace facebook::react {
11+
12+
InstanceHandle::InstanceHandle(
13+
jsi::Runtime &runtime,
14+
jsi::Value const &instanceHandle,
15+
Tag tag)
16+
: weakInstanceHandle_(
17+
jsi::WeakObject(runtime, instanceHandle.asObject(runtime))),
18+
tag_(tag) {}
19+
20+
jsi::Value InstanceHandle::getInstanceHandle(jsi::Runtime &runtime) const {
21+
return weakInstanceHandle_.lock(runtime);
22+
}
23+
24+
Tag InstanceHandle::getTag() const {
25+
return tag_;
26+
}
27+
28+
} // namespace facebook::react

0 commit comments

Comments
 (0)