Skip to content

Commit ab06cf5

Browse files
committed
refactor: self code review
1 parent d15b6e9 commit ab06cf5

File tree

1 file changed

+10
-27
lines changed

1 file changed

+10
-27
lines changed

src/renderer/reconciler.ts

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,14 @@ export type TextInstance = {
3434
};
3535

3636
type HostContext = {
37-
elementType: string;
37+
type: string;
3838
isInsideText: boolean;
3939
};
4040

41-
const NO_CONTEXT = {};
4241
const UPDATE_SIGNAL = {};
4342
const nodeToInstanceMap = new WeakMap<object, Instance>();
4443

4544
if (__DEV__) {
46-
Object.freeze(NO_CONTEXT);
4745
Object.freeze(UPDATE_SIGNAL);
4846
}
4947

@@ -140,7 +138,7 @@ const hostConfig = {
140138
): TextInstance {
141139
if (!hostContext.isInsideText) {
142140
throw new Error(
143-
`Invariant Violation: Text strings must be rendered within a <Text> component. Detected attempt to render "${text}" string within a <${hostContext.elementType}> component.`,
141+
`Invariant Violation: Text strings must be rendered within a <Text> component. Detected attempt to render "${text}" string within a <${hostContext.type}> component.`,
144142
);
145143
}
146144

@@ -216,7 +214,6 @@ const hostConfig = {
216214
* This method happens **in the render phase**. Do not mutate the tree from it.
217215
*/
218216
shouldSetTextContent(_type: Type, _props: Props): boolean {
219-
// TODO: what should RN do here?
220217
return false;
221218
},
222219

@@ -228,7 +225,7 @@ const hostConfig = {
228225
* This method happens **in the render phase**. Do not mutate the tree from it.
229226
*/
230227
getRootHostContext(_rootContainer: Container): HostContext | null {
231-
return { elementType: 'ROOT', isInsideText: false };
228+
return { type: 'ROOT', isInsideText: false };
232229
},
233230

234231
/**
@@ -251,7 +248,7 @@ const hostConfig = {
251248
_rootContainer: Container,
252249
): HostContext {
253250
const isInsideText = type === 'Text';
254-
return { elementType: type, isInsideText };
251+
return { type: type, isInsideText };
255252
},
256253

257254
/**
@@ -263,7 +260,6 @@ const hostConfig = {
263260
getPublicInstance(instance: Instance | TextInstance): PublicInstance {
264261
switch (instance.tag) {
265262
case 'INSTANCE': {
266-
//console.log('🔷 getPublicInstance', instance);
267263
const createNodeMock = instance.rootContainer.createNodeMock;
268264
const mockNode = createNodeMock({
269265
type: instance.type,
@@ -332,7 +328,6 @@ const hostConfig = {
332328
/**
333329
* Optional. You can proxy this to `queueMicrotask` or its equivalent in your environment.
334330
*/
335-
// TODO: what should RN do here?
336331
scheduleMicrotask: queueMicrotask,
337332

338333
/**
@@ -345,7 +340,6 @@ const hostConfig = {
345340
/**
346341
* Whether the renderer shouldn't trigger missing `act()` warnings
347342
*/
348-
// TODO: what should RN do here?
349343
warnsIfNotActing: true,
350344

351345
/**
@@ -393,7 +387,7 @@ const hostConfig = {
393387
const instance = nodeToInstanceMap.get(node);
394388
if (instance !== undefined) {
395389
// TODO: why not just return the instance?
396-
return instance.internalHandle;
390+
return instance;
397391
}
398392

399393
return null;
@@ -411,8 +405,8 @@ const hostConfig = {
411405
nodeToInstanceMap.set(scopeInstance, instance);
412406
},
413407

414-
getInstanceFromScope(scopeInstance: any): null | Instance {
415-
return nodeToInstanceMap.get(scopeInstance) || null;
408+
getInstanceFromScope(scopeInstance: any): Instance | null {
409+
return nodeToInstanceMap.get(scopeInstance) ?? null;
416410
},
417411

418412
detachDeletedInstance(_node: Instance): void {
@@ -482,7 +476,7 @@ const hostConfig = {
482476
*
483477
* Here, `textInstance` is a node created by `createTextInstance`.
484478
*/
485-
commitTextUpdate(textInstance: TextInstance, oldText: string, newText: string): void {
479+
commitTextUpdate(textInstance: TextInstance, _oldText: string, newText: string): void {
486480
textInstance.text = newText;
487481
},
488482

@@ -583,7 +577,8 @@ const hostConfig = {
583577
// of creating it from scratch. For example, the DOM renderer uses this to attach to an HTML markup.
584578
//
585579
// To support hydration, you need to declare `supportsHydration: true` and then implement the methods in
586-
// the "Hydration" section [listed in this file](https://github.com/facebook/react/blob/master/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js). File an issue if you need help.
580+
// the "Hydration" section [listed in this file](https://github.com/facebook/react/blob/master/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js).
581+
// File an issue if you need help.
587582
// -------------------
588583
supportsHydration: false,
589584
};
@@ -599,18 +594,6 @@ export const TestReconciler = createReconciler(hostConfig);
599594
* look at `commitMount`.
600595
*/
601596
function appendChild(parentInstance: Container | Instance, child: Instance | TextInstance): void {
602-
if (__DEV__) {
603-
if (!Array.isArray(parentInstance.children)) {
604-
// eslint-disable-next-line no-console
605-
console.error(
606-
'An invalid container has been provided. ' +
607-
'This may indicate that another renderer is being used in addition to the test renderer. ' +
608-
'(For example, ReactDOM.createPortal inside of a ReactTestRenderer tree.) ' +
609-
'This is not supported.',
610-
);
611-
}
612-
}
613-
614597
const index = parentInstance.children.indexOf(child);
615598
if (index !== -1) {
616599
parentInstance.children.splice(index, 1);

0 commit comments

Comments
 (0)