Don't use closures in DevTools injection (#18278)
* Don't use closures in DevTools injection Nested closures are tricky. They're not super efficient and when they share scope between multiple closures they're hard for a compiler to optimize. It's also unclear how many versions will be created. By hoisting things out an just make it simple calls the compiler can do a much better job. * Store injected hook to work around fast refresh
Sebastian Markbåge committed
Apr 8, 2020 at 17:57 UTC
b014e2d5e377a94bbf8d94d9a2602ce05c184e38
2 files changed
+77
-83
packages/react-reconciler/src/ReactFiberDevToolsHook.js
+55
-68
@@ -20,9 +20,8 @@ import {DidCapture} from './ReactSideEffectTags';
20
21
declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void;
22
23
-let onScheduleFiberRoot = null;
24
-let onCommitFiberRoot = null;
25
-let onCommitFiberUnmount = null;
23
+let rendererID = null;
24
+let injectedHook = null;
25
let hasLoggedError = false;
26
27
export const isDevToolsPresent =
@@ -52,66 +51,9 @@ export function injectInternals(internals: Object): boolean {
51
return true;
52
}
53
try {
55
- const rendererID = hook.inject(internals);
54
+ rendererID = hook.inject(internals);
55
// We have successfully injected, so now it is safe to set up hooks.
57
- if (__DEV__) {
58
- // Only used by Fast Refresh
59
- if (typeof hook.onScheduleFiberRoot === 'function') {
60
- onScheduleFiberRoot = (root, children) => {
61
- try {
62
- hook.onScheduleFiberRoot(rendererID, root, children);
63
- } catch (err) {
64
- if (__DEV__ && !hasLoggedError) {
65
- hasLoggedError = true;
66
- console.error(
67
- 'React instrumentation encountered an error: %s',
68
- err,
69
- );
70
- }
71
- }
72
- };
73
- }
74
- }
75
- onCommitFiberRoot = (root, expirationTime) => {
76
- try {
77
- const didError = (root.current.effectTag & DidCapture) === DidCapture;
78
- if (enableProfilerTimer) {
79
- const currentTime = getCurrentTime();
80
- const priorityLevel = inferPriorityFromExpirationTime(
81
- currentTime,
82
- expirationTime,
83
- );
84
- hook.onCommitFiberRoot(rendererID, root, priorityLevel, didError);
85
- } else {
86
- hook.onCommitFiberRoot(rendererID, root, undefined, didError);
87
- }
88
- } catch (err) {
89
- if (__DEV__) {
90
- if (!hasLoggedError) {
91
- hasLoggedError = true;
92
- console.error(
93
- 'React instrumentation encountered an error: %s',
94
- err,
95
- );
96
- }
97
- }
98
- }
99
- };
100
- onCommitFiberUnmount = fiber => {
101
- try {
102
- hook.onCommitFiberUnmount(rendererID, fiber);
103
- } catch (err) {
104
- if (__DEV__) {
105
- if (!hasLoggedError) {
106
- hasLoggedError = true;
107
- console.error(
108
- 'React instrumentation encountered an error: %s',
109
- err,
110
- );
111
- }
112
- }
113
- }
114
- };
56
+ injectedHook = hook;
57
} catch (err) {
58
// Catch all errors because it is unsafe to throw during initialization.
59
if (__DEV__) {
@@ -123,19 +65,64 @@ export function injectInternals(internals: Object): boolean {
65
}
66
67
export function onScheduleRoot(root: FiberRoot, children: ReactNodeList) {
126
- if (typeof onScheduleFiberRoot === 'function') {
127
- onScheduleFiberRoot(root, children);
68
+ if (__DEV__) {
69
+ if (
70
+ injectedHook &&
71
+ typeof injectedHook.onScheduleFiberRoot === 'function'
72
+ ) {
73
+ try {
74
+ injectedHook.onScheduleFiberRoot(rendererID, root, children);
75
+ } catch (err) {
76
+ if (__DEV__ && !hasLoggedError) {
77
+ hasLoggedError = true;
78
+ console.error('React instrumentation encountered an error: %s', err);
79
+ }
80
+ }
81
+ }
82
}
83
}
84
85
export function onCommitRoot(root: FiberRoot, expirationTime: ExpirationTime) {
132
- if (typeof onCommitFiberRoot === 'function') {
133
- onCommitFiberRoot(root, expirationTime);
86
+ if (injectedHook && typeof injectedHook.onCommitFiberRoot === 'function') {
87
+ try {
88
+ const didError = (root.current.effectTag & DidCapture) === DidCapture;
89
+ if (enableProfilerTimer) {
90
+ const currentTime = getCurrentTime();
91
+ const priorityLevel = inferPriorityFromExpirationTime(
92
+ currentTime,
93
+ expirationTime,
94
+ );
95
+ injectedHook.onCommitFiberRoot(
96
+ rendererID,
97
+ root,
98
+ priorityLevel,
99
+ didError,
100
+ );
101
+ } else {
102
+ injectedHook.onCommitFiberRoot(rendererID, root, undefined, didError);
103
+ }
104
+ } catch (err) {
105
+ if (__DEV__) {
106
+ if (!hasLoggedError) {
107
+ hasLoggedError = true;
108
+ console.error('React instrumentation encountered an error: %s', err);
109
+ }
110
+ }
111
+ }
112
}
113
}
114
115
export function onCommitUnmount(fiber: Fiber) {
138
- if (typeof onCommitFiberUnmount === 'function') {
139
- onCommitFiberUnmount(fiber);
116
+ if (injectedHook && typeof injectedHook.onCommitFiberUnmount === 'function') {
117
+ try {
118
+ injectedHook.onCommitFiberUnmount(rendererID, fiber);
119
+ } catch (err) {
120
+ if (__DEV__) {
121
+ if (!hasLoggedError) {
122
+ hasLoggedError = true;
123
+ console.error('React instrumentation encountered an error: %s', err);
124
+ }
125
+ }
126
+ }
127
}
128
}
packages/react-reconciler/src/ReactFiberReconciler.old.js
+22
-15
@@ -506,6 +506,24 @@ if (__DEV__) {
506
};
507
}
508
509
+function findHostInstanceByFiber(fiber: Fiber): Instance | TextInstance | null {
510
+ const hostFiber = findCurrentHostFiber(fiber);
511
+ if (hostFiber === null) {
512
+ return null;
513
+ }
514
+ return hostFiber.stateNode;
515
+}
516
+
517
+function emptyFindFiberByHostInstance(
518
+ instance: Instance | TextInstance,
519
+): Fiber | null {
520
+ return null;
521
+}
522
+
523
+function getCurrentFiberForDevTools() {
524
+ return ReactCurrentFiberCurrent;
525
+}
526
+
527
export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
528
const {findFiberByHostInstance} = devToolsConfig;
529
const {ReactCurrentDispatcher} = ReactSharedInternals;
@@ -520,27 +538,16 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
538
setSuspenseHandler,
539
scheduleUpdate,
540
currentDispatcherRef: ReactCurrentDispatcher,
523
- findHostInstanceByFiber(fiber: Fiber): Instance | TextInstance | null {
524
- const hostFiber = findCurrentHostFiber(fiber);
525
- if (hostFiber === null) {
526
- return null;
527
- }
528
- return hostFiber.stateNode;
529
- },
530
- findFiberByHostInstance(instance: Instance | TextInstance): Fiber | null {
531
- if (!findFiberByHostInstance) {
532
- // Might not be implemented by the renderer.
533
- return null;
534
- }
535
- return findFiberByHostInstance(instance);
536
- },
541
+ findHostInstanceByFiber,
542
+ findFiberByHostInstance:
543
+ findFiberByHostInstance || emptyFindFiberByHostInstance,
544
// React Refresh
545
findHostInstancesForRefresh: __DEV__ ? findHostInstancesForRefresh : null,
546
scheduleRefresh: __DEV__ ? scheduleRefresh : null,
547
scheduleRoot: __DEV__ ? scheduleRoot : null,
548
setRefreshHandler: __DEV__ ? setRefreshHandler : null,
549
// Enables DevTools to append owner stacks to error messages in DEV mode.
543
- getCurrentFiber: __DEV__ ? () => ReactCurrentFiberCurrent : null,
550
+ getCurrentFiber: __DEV__ ? getCurrentFiberForDevTools : null,
551
});
552
}
553