Skip to content

Commit f6c63b9

Browse files
rubennortekassens
authored andcommitted
Implement public instances for text nodes in Fabric (facebook#26516)
## Summary This adds the ability to create public instances for text nodes in Fabric. The implementation for the public instances lives in React Native (as it does for host components after facebook#26437). The logic here just handles their lazy instantiation when requested via `getPublicInstanceFromInternalInstanceHandle`, which is called by Fabric with information coming from the shadow tree. It's important that the creation of public instances for text nodes is done lazily to avoid regressing memory usage when unused. Instances for text nodes are left intact if the public instance is never accessed. This is necessary to implement access to text nodes in React Native as explained in react-native-community/discussions-and-proposals#607 ## How did you test this change? Added unit tests (also fixed a test that was only testing the logic in a mock :S).
1 parent 85bea51 commit f6c63b9

File tree

6 files changed

+151
-25
lines changed

6 files changed

+151
-25
lines changed

packages/react-native-renderer/src/ReactFabricHostConfig.js

+42-10
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ import {
1414
DefaultEventPriority,
1515
DiscreteEventPriority,
1616
} from 'react-reconciler/src/ReactEventPriorities';
17+
import {HostText} from 'react-reconciler/src/ReactWorkTags';
1718

1819
// Modules provided by RN:
1920
import {
2021
ReactNativeViewConfigRegistry,
2122
deepFreezeAndThrowOnMutationInDev,
2223
createPublicInstance,
24+
createPublicTextInstance,
2325
type PublicInstance as ReactNativePublicInstance,
26+
type PublicTextInstance,
2427
} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
2528

2629
const {
@@ -47,23 +50,33 @@ const {get: getViewConfigForType} = ReactNativeViewConfigRegistry;
4750
// This means that they never overlap.
4851
let nextReactTag = 2;
4952

53+
type InternalInstanceHandle = Object;
5054
type Node = Object;
5155
export type Type = string;
5256
export type Props = Object;
5357
export type Instance = {
5458
// Reference to the shadow node.
5559
node: Node,
60+
// This object is shared by all the clones of the instance.
61+
// We use it to access their shared public instance (exposed through refs)
62+
// and to access its committed state for events, etc.
5663
canonical: {
5764
nativeTag: number,
5865
viewConfig: ViewConfig,
5966
currentProps: Props,
6067
// Reference to the React handle (the fiber)
61-
internalInstanceHandle: Object,
68+
internalInstanceHandle: InternalInstanceHandle,
6269
// Exposed through refs.
6370
publicInstance: PublicInstance,
6471
},
6572
};
66-
export type TextInstance = {node: Node, ...};
73+
export type TextInstance = {
74+
// Reference to the shadow node.
75+
node: Node,
76+
// Text instances are never cloned, so we don't need to keep a "canonical"
77+
// reference to make sure all clones of the instance point to the same values.
78+
publicInstance?: PublicTextInstance,
79+
};
6780
export type HydratableInstance = Instance | TextInstance;
6881
export type PublicInstance = ReactNativePublicInstance;
6982
export type Container = number;
@@ -115,7 +128,7 @@ export function createInstance(
115128
props: Props,
116129
rootContainerInstance: Container,
117130
hostContext: HostContext,
118-
internalInstanceHandle: Object,
131+
internalInstanceHandle: InternalInstanceHandle,
119132
): Instance {
120133
const tag = nextReactTag;
121134
nextReactTag += 2;
@@ -162,7 +175,7 @@ export function createTextInstance(
162175
text: string,
163176
rootContainerInstance: Container,
164177
hostContext: HostContext,
165-
internalInstanceHandle: Object,
178+
internalInstanceHandle: InternalInstanceHandle,
166179
): TextInstance {
167180
if (__DEV__) {
168181
if (!hostContext.isInAParentText) {
@@ -239,9 +252,26 @@ export function getPublicInstance(instance: Instance): null | PublicInstance {
239252
return null;
240253
}
241254

255+
function getPublicTextInstance(
256+
textInstance: TextInstance,
257+
internalInstanceHandle: InternalInstanceHandle,
258+
): PublicTextInstance {
259+
if (textInstance.publicInstance == null) {
260+
textInstance.publicInstance = createPublicTextInstance(
261+
internalInstanceHandle,
262+
);
263+
}
264+
return textInstance.publicInstance;
265+
}
266+
242267
export function getPublicInstanceFromInternalInstanceHandle(
243-
internalInstanceHandle: Object,
244-
): null | PublicInstance {
268+
internalInstanceHandle: InternalInstanceHandle,
269+
): null | PublicInstance | PublicTextInstance {
270+
if (internalInstanceHandle.tag === HostText) {
271+
const textInstance: TextInstance = internalInstanceHandle.stateNode;
272+
return getPublicTextInstance(textInstance, internalInstanceHandle);
273+
}
274+
245275
const instance: Instance = internalInstanceHandle.stateNode;
246276
return getPublicInstance(instance);
247277
}
@@ -321,7 +351,7 @@ export function cloneInstance(
321351
type: string,
322352
oldProps: Props,
323353
newProps: Props,
324-
internalInstanceHandle: Object,
354+
internalInstanceHandle: InternalInstanceHandle,
325355
keepChildren: boolean,
326356
recyclableInstance: null | Instance,
327357
): Instance {
@@ -350,7 +380,7 @@ export function cloneHiddenInstance(
350380
instance: Instance,
351381
type: string,
352382
props: Props,
353-
internalInstanceHandle: Object,
383+
internalInstanceHandle: InternalInstanceHandle,
354384
): Instance {
355385
const viewConfig = instance.canonical.viewConfig;
356386
const node = instance.node;
@@ -367,7 +397,7 @@ export function cloneHiddenInstance(
367397
export function cloneHiddenTextInstance(
368398
instance: Instance,
369399
text: string,
370-
internalInstanceHandle: Object,
400+
internalInstanceHandle: InternalInstanceHandle,
371401
): TextInstance {
372402
throw new Error('Not yet implemented.');
373403
}
@@ -399,7 +429,9 @@ export function getInstanceFromNode(node: any): empty {
399429
throw new Error('Not yet implemented.');
400430
}
401431

402-
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
432+
export function beforeActiveInstanceBlur(
433+
internalInstanceHandle: InternalInstanceHandle,
434+
) {
403435
// noop
404436
}
405437

packages/react-native-renderer/src/ReactNativeTypes.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ export type ReactNativeType = {
215215
export opaque type Node = mixed;
216216
export opaque type InternalInstanceHandle = mixed;
217217
type PublicInstance = mixed;
218+
type PublicTextInstance = mixed;
218219

219220
export type ReactFabricType = {
220221
findHostInstance_DEPRECATED<TElementType: ElementType>(
@@ -244,7 +245,7 @@ export type ReactFabricType = {
244245
): ?Node,
245246
getPublicInstanceFromInternalInstanceHandle(
246247
internalInstanceHandle: InternalInstanceHandle,
247-
): PublicInstance,
248+
): PublicInstance | PublicTextInstance,
248249
...
249250
};
250251

packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/ReactNativePrivateInterface.js

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
export opaque type PublicInstance = mixed;
11+
export opaque type PublicTextInstance = mixed;
1112

1213
module.exports = {
1314
get BatchedBridge() {
@@ -55,4 +56,7 @@ module.exports = {
5556
get createPublicInstance() {
5657
return require('./createPublicInstance').default;
5758
},
59+
get createPublicTextInstance() {
60+
return require('./createPublicTextInstance').default;
61+
},
5862
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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+
* @flow strict
8+
*/
9+
10+
import type {PublicInstance} from './ReactNativePrivateInterface';
11+
12+
export default function createPublicTextInstance(
13+
internalInstanceHandle: mixed,
14+
): PublicInstance {
15+
return {
16+
__internalInstanceHandle: internalInstanceHandle,
17+
};
18+
}

packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js

+81-14
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,10 @@
1212

1313
let React;
1414
let ReactFabric;
15+
let ReactNativePrivateInterface;
1516
let createReactNativeComponentClass;
1617
let StrictMode;
1718
let act;
18-
let getNativeTagFromPublicInstance;
19-
let getNodeFromPublicInstance;
2019

2120
const DISPATCH_COMMAND_REQUIRES_HOST_COMPONENT =
2221
"Warning: dispatchCommand was called with a ref that isn't a " +
@@ -39,14 +38,10 @@ describe('ReactFabric', () => {
3938
React = require('react');
4039
StrictMode = React.StrictMode;
4140
ReactFabric = require('react-native-renderer/fabric');
41+
ReactNativePrivateInterface = require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface');
4242
createReactNativeComponentClass =
4343
require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface')
4444
.ReactNativeViewConfigRegistry.register;
45-
getNativeTagFromPublicInstance =
46-
require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface').getNativeTagFromPublicInstance;
47-
getNodeFromPublicInstance =
48-
require('react-native/Libraries/ReactPrivate/ReactNativePrivateInterface').getNodeFromPublicInstance;
49-
5045
act = require('internal-test-utils').act;
5146
});
5247

@@ -937,7 +932,9 @@ describe('ReactFabric', () => {
937932
'\n in RCTView (at **)' +
938933
'\n in ContainsStrictModeChild (at **)',
939934
]);
940-
expect(match).toBe(getNativeTagFromPublicInstance(child));
935+
expect(match).toBe(
936+
ReactNativePrivateInterface.getNativeTagFromPublicInstance(child),
937+
);
941938
});
942939

943940
it('findNodeHandle should warn if passed a component that is inside StrictMode', async () => {
@@ -974,7 +971,9 @@ describe('ReactFabric', () => {
974971
'\n in RCTView (at **)' +
975972
'\n in IsInStrictMode (at **)',
976973
]);
977-
expect(match).toBe(getNativeTagFromPublicInstance(child));
974+
expect(match).toBe(
975+
ReactNativePrivateInterface.getNativeTagFromPublicInstance(child),
976+
);
978977
});
979978

980979
it('should no-op if calling sendAccessibilityEvent on unmounted refs', async () => {
@@ -1015,6 +1014,30 @@ describe('ReactFabric', () => {
10151014
uiViewClassName: 'RCTView',
10161015
}));
10171016

1017+
await act(() => {
1018+
ReactFabric.render(<View foo="test" />, 1);
1019+
});
1020+
1021+
const internalInstanceHandle =
1022+
nativeFabricUIManager.createNode.mock.calls[0][4];
1023+
expect(internalInstanceHandle).toEqual(expect.any(Object));
1024+
1025+
const expectedShadowNode =
1026+
nativeFabricUIManager.createNode.mock.results[0].value;
1027+
expect(expectedShadowNode).toEqual(expect.any(Object));
1028+
1029+
const node = ReactFabric.getNodeFromInternalInstanceHandle(
1030+
internalInstanceHandle,
1031+
);
1032+
expect(node).toBe(expectedShadowNode);
1033+
});
1034+
1035+
it('getPublicInstanceFromInternalInstanceHandle should provide public instances for HostComponent', async () => {
1036+
const View = createReactNativeComponentClass('RCTView', () => ({
1037+
validAttributes: {foo: true},
1038+
uiViewClassName: 'RCTView',
1039+
}));
1040+
10181041
let viewRef;
10191042
await act(() => {
10201043
ReactFabric.render(
@@ -1028,11 +1051,55 @@ describe('ReactFabric', () => {
10281051
);
10291052
});
10301053

1031-
const expectedShadowNode =
1032-
nativeFabricUIManager.createNode.mock.results[0].value;
1033-
expect(expectedShadowNode).toEqual(expect.any(Object));
1054+
const internalInstanceHandle =
1055+
nativeFabricUIManager.createNode.mock.calls[0][4];
1056+
expect(internalInstanceHandle).toEqual(expect.any(Object));
10341057

1035-
const node = getNodeFromPublicInstance(viewRef);
1036-
expect(node).toBe(expectedShadowNode);
1058+
const publicInstance =
1059+
ReactFabric.getPublicInstanceFromInternalInstanceHandle(
1060+
internalInstanceHandle,
1061+
);
1062+
expect(publicInstance).toBe(viewRef);
1063+
});
1064+
1065+
it('getPublicInstanceFromInternalInstanceHandle should provide public instances for HostText', async () => {
1066+
jest.spyOn(ReactNativePrivateInterface, 'createPublicTextInstance');
1067+
1068+
const RCTText = createReactNativeComponentClass('RCTText', () => ({
1069+
validAttributes: {},
1070+
uiViewClassName: 'RCTText',
1071+
}));
1072+
1073+
await act(() => {
1074+
ReactFabric.render(<RCTText>Text content</RCTText>, 1);
1075+
});
1076+
1077+
// Access the internal instance handle used to create the text node.
1078+
const internalInstanceHandle =
1079+
nativeFabricUIManager.createNode.mock.calls[0][4];
1080+
expect(internalInstanceHandle).toEqual(expect.any(Object));
1081+
1082+
// Text public instances should be created lazily.
1083+
expect(
1084+
ReactNativePrivateInterface.createPublicTextInstance,
1085+
).not.toHaveBeenCalled();
1086+
1087+
const publicInstance =
1088+
ReactFabric.getPublicInstanceFromInternalInstanceHandle(
1089+
internalInstanceHandle,
1090+
);
1091+
1092+
// We just requested the text public instance, so it should have been created at this point.
1093+
expect(
1094+
ReactNativePrivateInterface.createPublicTextInstance,
1095+
).toHaveBeenCalledTimes(1);
1096+
expect(
1097+
ReactNativePrivateInterface.createPublicTextInstance,
1098+
).toHaveBeenCalledWith(internalInstanceHandle);
1099+
1100+
const expectedPublicInstance =
1101+
ReactNativePrivateInterface.createPublicTextInstance.mock.results[0]
1102+
.value;
1103+
expect(publicInstance).toBe(expectedPublicInstance);
10371104
});
10381105
});

scripts/flow/react-native-host-hooks.js

+4
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'
145145
...
146146
};
147147
declare export opaque type PublicInstance;
148+
declare export opaque type PublicTextInstance;
148149
declare export function getNodeFromPublicInstance(
149150
publicInstance: PublicInstance,
150151
): Object;
@@ -156,6 +157,9 @@ declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'
156157
viewConfig: __ViewConfig,
157158
internalInstanceHandle: mixed,
158159
): PublicInstance;
160+
declare export function createPublicTextInstance(
161+
internalInstanceHandle: mixed,
162+
): PublicTextInstance;
159163
}
160164

161165
declare module 'react-native/Libraries/ReactPrivate/ReactNativePrivateInitializeCore' {

0 commit comments

Comments
 (0)