Skip to content

Commit a44e750

Browse files
authored
Store instance handles in an internal map behind flag (#35053)
We already append `randomKey` to each handle name to prevent external libraries from accessing and relying on these internals. But more libraries recently have been getting around this by simply iterating over the element properties and using a `startsWith` check. This flag allows us to experiment with moving these handles to an internal map. This PR starts with the two most common internals, the props object and the fiber. We can consider moving additional properties such as the container root and others depending on perf results.
1 parent 37b089a commit a44e750

File tree

8 files changed

+91
-11
lines changed

8 files changed

+91
-11
lines changed

packages/react-dom-bindings/src/client/ReactDOMComponentTree.js

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ import {getParentHydrationBoundary} from './ReactFiberConfigDOM';
3838

3939
import {enableScopeAPI} from 'shared/ReactFeatureFlags';
4040

41+
import {enableInternalInstanceMap} from 'shared/ReactFeatureFlags';
42+
4143
const randomKey = Math.random().toString(36).slice(2);
4244
const internalInstanceKey = '__reactFiber$' + randomKey;
4345
const internalPropsKey = '__reactProps$' + randomKey;
@@ -49,7 +51,32 @@ const internalRootNodeResourcesKey = '__reactResources$' + randomKey;
4951
const internalHoistableMarker = '__reactMarker$' + randomKey;
5052
const internalScrollTimer = '__reactScroll$' + randomKey;
5153

54+
type InstanceUnion =
55+
| Instance
56+
| TextInstance
57+
| SuspenseInstance
58+
| ActivityInstance
59+
| ReactScopeInstance
60+
| Container;
61+
62+
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
63+
const internalInstanceMap:
64+
| WeakMap<InstanceUnion, Fiber>
65+
| Map<InstanceUnion, Fiber> = new PossiblyWeakMap();
66+
const internalPropsMap:
67+
| WeakMap<InstanceUnion, Props>
68+
| Map<InstanceUnion, Props> = new PossiblyWeakMap();
69+
5270
export function detachDeletedInstance(node: Instance): void {
71+
if (enableInternalInstanceMap) {
72+
internalInstanceMap.delete(node);
73+
internalPropsMap.delete(node);
74+
delete (node: any)[internalEventHandlersKey];
75+
delete (node: any)[internalEventHandlerListenersKey];
76+
delete (node: any)[internalEventHandlesSetKey];
77+
delete (node: any)[internalRootNodeResourcesKey];
78+
return;
79+
}
5380
// TODO: This function is only called on host components. I don't think all of
5481
// these fields are relevant.
5582
delete (node: any)[internalInstanceKey];
@@ -68,6 +95,10 @@ export function precacheFiberNode(
6895
| ActivityInstance
6996
| ReactScopeInstance,
7097
): void {
98+
if (enableInternalInstanceMap) {
99+
internalInstanceMap.set(node, hostInst);
100+
return;
101+
}
71102
(node: any)[internalInstanceKey] = hostInst;
72103
}
73104

@@ -95,7 +126,12 @@ export function isContainerMarkedAsRoot(node: Container): boolean {
95126
// HostRoot back. To get to the HostRoot, you need to pass a child of it.
96127
// The same thing applies to Suspense and Activity boundaries.
97128
export function getClosestInstanceFromNode(targetNode: Node): null | Fiber {
98-
let targetInst = (targetNode: any)[internalInstanceKey];
129+
let targetInst: void | Fiber;
130+
if (enableInternalInstanceMap) {
131+
targetInst = internalInstanceMap.get(((targetNode: any): InstanceUnion));
132+
} else {
133+
targetInst = (targetNode: any)[internalInstanceKey];
134+
}
99135
if (targetInst) {
100136
// Don't return HostRoot, SuspenseComponent or ActivityComponent here.
101137
return targetInst;
@@ -112,9 +148,15 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber {
112148
// itself because the fibers are conceptually between the container
113149
// node and the first child. It isn't surrounding the container node.
114150
// If it's not a container, we check if it's an instance.
115-
targetInst =
116-
(parentNode: any)[internalContainerInstanceKey] ||
117-
(parentNode: any)[internalInstanceKey];
151+
if (enableInternalInstanceMap) {
152+
targetInst =
153+
(parentNode: any)[internalContainerInstanceKey] ||
154+
internalInstanceMap.get(((parentNode: any): InstanceUnion));
155+
} else {
156+
targetInst =
157+
(parentNode: any)[internalContainerInstanceKey] ||
158+
(parentNode: any)[internalInstanceKey];
159+
}
118160
if (targetInst) {
119161
// Since this wasn't the direct target of the event, we might have
120162
// stepped past dehydrated DOM nodes to get here. However they could
@@ -147,8 +189,10 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber {
147189
// have had an internalInstanceKey on it.
148190
// Let's get the fiber associated with the SuspenseComponent
149191
// as the deepest instance.
150-
// $FlowFixMe[prop-missing]
151-
const targetFiber = hydrationInstance[internalInstanceKey];
192+
const targetFiber = enableInternalInstanceMap
193+
? internalInstanceMap.get(hydrationInstance)
194+
: // $FlowFixMe[prop-missing]
195+
hydrationInstance[internalInstanceKey];
152196
if (targetFiber) {
153197
return targetFiber;
154198
}
@@ -175,9 +219,16 @@ export function getClosestInstanceFromNode(targetNode: Node): null | Fiber {
175219
* instance, or null if the node was not rendered by this React.
176220
*/
177221
export function getInstanceFromNode(node: Node): Fiber | null {
178-
const inst =
179-
(node: any)[internalInstanceKey] ||
180-
(node: any)[internalContainerInstanceKey];
222+
let inst: void | null | Fiber;
223+
if (enableInternalInstanceMap) {
224+
inst =
225+
internalInstanceMap.get(((node: any): InstanceUnion)) ||
226+
(node: any)[internalContainerInstanceKey];
227+
} else {
228+
inst =
229+
(node: any)[internalInstanceKey] ||
230+
(node: any)[internalContainerInstanceKey];
231+
}
181232
if (inst) {
182233
const tag = inst.tag;
183234
if (
@@ -226,16 +277,24 @@ export function getFiberCurrentPropsFromNode(
226277
| TextInstance
227278
| SuspenseInstance
228279
| ActivityInstance,
229-
): Props {
280+
): Props | null {
281+
if (enableInternalInstanceMap) {
282+
return internalPropsMap.get(node) || null;
283+
}
230284
return (node: any)[internalPropsKey] || null;
231285
}
232286

233287
export function updateFiberProps(node: Instance, props: Props): void {
288+
if (enableInternalInstanceMap) {
289+
internalPropsMap.set(node, props);
290+
return;
291+
}
234292
(node: any)[internalPropsKey] = props;
235293
}
236294

237295
export function getEventListenerSet(node: EventTarget): Set<string> {
238-
let elementListenerSet = (node: any)[internalEventHandlersKey];
296+
let elementListenerSet: Set<string> | void;
297+
elementListenerSet = (node: any)[internalEventHandlersKey];
239298
if (elementListenerSet === undefined) {
240299
elementListenerSet = (node: any)[internalEventHandlersKey] = new Set();
241300
}
@@ -246,6 +305,9 @@ export function getFiberFromScopeInstance(
246305
scope: ReactScopeInstance,
247306
): null | Fiber {
248307
if (enableScopeAPI) {
308+
if (enableInternalInstanceMap) {
309+
return internalInstanceMap.get(((scope: any): InstanceUnion)) || null;
310+
}
249311
return (scope: any)[internalInstanceKey] || null;
250312
}
251313
return null;
@@ -318,6 +380,12 @@ export function clearScrollEndTimer(node: EventTarget): void {
318380
}
319381

320382
export function isOwnedInstance(node: Node): boolean {
383+
if (enableInternalInstanceMap) {
384+
return !!(
385+
(node: any)[internalHoistableMarker] ||
386+
internalInstanceMap.has((node: any))
387+
);
388+
}
321389
return !!(
322390
(node: any)[internalHoistableMarker] || (node: any)[internalInstanceKey]
323391
);

packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ export const enableFragmentRefs: boolean = true;
147147
export const enableFragmentRefsScrollIntoView: boolean = true;
148148
export const enableFragmentRefsInstanceHandles: boolean = false;
149149

150+
export const enableInternalInstanceMap: boolean = false;
151+
150152
// -----------------------------------------------------------------------------
151153
// Ready for next major.
152154
//

packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export const enableComponentPerformanceTrack: boolean =
8484
__PROFILE__ && dynamicFlags.enableComponentPerformanceTrack;
8585
export const enablePerformanceIssueReporting: boolean =
8686
enableComponentPerformanceTrack;
87+
export const enableInternalInstanceMap: boolean = false;
8788

8889
// Flow magic to verify the exports of this file match the original version.
8990
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ export const enableFragmentRefs: boolean = true;
7676
export const enableFragmentRefsScrollIntoView: boolean = false;
7777
export const enableFragmentRefsInstanceHandles: boolean = false;
7878

79+
export const enableInternalInstanceMap: boolean = false;
80+
7981
// Profiling Only
8082
export const enableProfilerTimer: boolean = __PROFILE__;
8183
export const enableProfilerCommitHooks: boolean = __PROFILE__;

packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ export const enableFragmentRefs: boolean = true;
7777
export const enableFragmentRefsScrollIntoView: boolean = true;
7878
export const enableFragmentRefsInstanceHandles: boolean = false;
7979

80+
export const enableInternalInstanceMap: boolean = false;
81+
8082
// TODO: This must be in sync with the main ReactFeatureFlags file because
8183
// the Test Renderer's value must be the same as the one used by the
8284
// react package.

packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,7 @@ export const enableFragmentRefsScrollIntoView: boolean = false;
8585
export const enableFragmentRefsInstanceHandles: boolean = false;
8686
export const ownerStackLimit = 1e4;
8787

88+
export const enableInternalInstanceMap: boolean = false;
89+
8890
// Flow magic to verify the exports of this file match the original version.
8991
((((null: any): ExportsType): FeatureFlagsType): ExportsType);

packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export const enableFragmentRefs: boolean = __VARIANT__;
3838
export const enableFragmentRefsScrollIntoView: boolean = __VARIANT__;
3939
export const enableAsyncDebugInfo: boolean = __VARIANT__;
4040

41+
export const enableInternalInstanceMap: boolean = __VARIANT__;
42+
4143
// TODO: These flags are hard-coded to the default values used in open source.
4244
// Update the tests so that they pass in either mode, then set these
4345
// to __VARIANT__.

packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const {
3535
enableFragmentRefs,
3636
enableFragmentRefsScrollIntoView,
3737
enableAsyncDebugInfo,
38+
enableInternalInstanceMap,
3839
} = dynamicFeatureFlags;
3940

4041
// On WWW, __EXPERIMENTAL__ is used for a new modern build.

0 commit comments

Comments
 (0)