@samitouri / QOS-React / commits / 0aa7d2f800

Reverted optimization to avoid re-sending inspected fiber unless it committed

Brian Vaughn committed Apr 23, 2019 at 14:03 UTC 0aa7d2f8006888557e23b3e003eb3889747db19f
4 files changed +36 -83
src/backend/renderer.js
+9 -43
@@ -40,10 +40,7 @@ import type {
40 ReactRenderer,
41 RendererInterface,
42 } from './types';
43 -import type {
44 - InspectedElement,
45 - InspectedElementResponse,
46 -} from 'src/devtools/views/Components/types';
43 +import type { InspectedElement } from 'src/devtools/views/Components/types';
44
45 function getInternalReactConstants(version) {
46 const ReactSymbols = {
@@ -962,20 +959,6 @@ export function attach(
959 debug('updateFiberRecursively()', nextFiber, parentFiber);
960 }
961 const shouldIncludeInTree = !shouldFilterFiber(nextFiber);
965 -
966 - // If this is the most recently inspected Fiber, take note of whether it was part of the new commit.
967 - // If not, we can avoid re-serializing its props and state if asked again.
968 - // Note that we avoid even comparing IDs for fibers not in the tree,
969 - // so that we don't inadvertantly add them to the ID Map.
970 - if (
971 - shouldIncludeInTree &&
972 - inspectedElementID !== null &&
973 - inspectedElementID === getFiberID(getPrimaryFiber(nextFiber)) &&
974 - nextFiber.actualDuration > 0
975 - ) {
976 - hasInspectedElementChanged = true;
977 - }
978 -
962 const isSuspense = nextFiber.tag === SuspenseComponent;
963 let shouldResetChildren = false;
964 // The behavior of timed-out Suspense trees is unique.
@@ -1509,7 +1492,6 @@ export function attach(
1492 }
1493 }
1494
1512 - // TODO Send a no-op message if the specified Fiber hasn't been committed since it was last inspected.
1495 function inspectElementRaw(id: number): InspectedElement | null {
1496 let fiber = idToFiberMap.get(id);
1497
@@ -1649,33 +1631,17 @@ export function attach(
1631 };
1632 }
1633
1652 - let inspectedElementID: number | null = null;
1653 - let hasInspectedElementChanged: boolean = false;
1654 -
1655 - function inspectElement(id: number): InspectedElementResponse | null {
1656 - if (inspectedElementID === id && !hasInspectedElementChanged) {
1657 - // Optimization: Don't resend (and reserialize) unchanged props.
1658 - return {
1659 - id,
1660 - inspectedElement: null,
1661 - };
1662 - }
1663 -
1664 - inspectedElementID = id;
1665 - hasInspectedElementChanged = false;
1666 -
1667 - let inspectedElement = inspectElementRaw(id);
1668 - if (inspectedElement === null) {
1634 + function inspectElement(id: number): InspectedElement | null {
1635 + let result = inspectElementRaw(id);
1636 + if (result === null) {
1637 return null;
1638 }
1671 -
1639 // TODO Review sanitization approach for the below inspectable values.
1673 - inspectedElement.context = cleanForBridge(inspectedElement.context);
1674 - inspectedElement.hooks = cleanForBridge(inspectedElement.hooks);
1675 - inspectedElement.props = cleanForBridge(inspectedElement.props);
1676 - inspectedElement.state = cleanForBridge(inspectedElement.state);
1677 -
1678 - return { id, inspectedElement };
1640 + result.context = cleanForBridge(result.context);
1641 + result.hooks = cleanForBridge(result.hooks);
1642 + result.props = cleanForBridge(result.props);
1643 + result.state = cleanForBridge(result.state);
1644 + return result;
1645 }
1646
1647 function logElementToConsole(id) {
src/backend/types.js
+2 -2
@@ -1,7 +1,7 @@
1 // @flow
2
3 import type { ElementType } from 'src/devtools/types';
4 -import type { InspectedElementResponse } from 'src/devtools/views/Components/types';
4 +import type { InspectedElement } from 'src/devtools/views/Components/types';
5
6 type BundleType =
7 | 0 // PROD
@@ -105,7 +105,7 @@ export type RendererInterface = {
105 getProfilingSummary: (rootID: number) => ProfilingSummary,
106 handleCommitFiberRoot: (fiber: Object) => void,
107 handleCommitFiberUnmount: (fiber: Object) => void,
108 - inspectElement: (id: number) => InspectedElementResponse | null,
108 + inspectElement: (id: number) => InspectedElement | null,
109 logElementToConsole: (id: number) => void,
110 overrideSuspense: (id: number, forceFallback: boolean) => void,
111 prepareViewElementSource: (id: number) => void,
src/devtools/views/Components/InspectedElementContext.js
+25 -33
@@ -15,7 +15,6 @@ import { TreeStateContext } from './TreeContext';
15 import type {
16 DehydratedData,
17 InspectedElement,
18 - InspectedElementResponse,
18 } from 'src/devtools/views/Components/types';
19 import type { Resource, Thenable } from '../../cache';
20
@@ -70,33 +69,28 @@ function InspectedElementContextController({ children }: Props) {
69
70 // This effect handler invalidates the suspense cache and schedules rendering updates with React.
71 useEffect(() => {
73 - const onInspectedElement = (
74 - inspectedElementResponse: InspectedElementResponse | null
75 - ) => {
76 - if (inspectedElementResponse != null) {
77 - let { inspectedElement } = inspectedElementResponse;
78 - if (inspectedElement !== null) {
79 - const id = inspectedElement.id;
80 -
81 - inspectedElement = (({
82 - ...inspectedElement,
83 - context: hydrateHelper(inspectedElement.context),
84 - hooks: hydrateHelper(inspectedElement.hooks),
85 - props: hydrateHelper(inspectedElement.props),
86 - state: hydrateHelper(inspectedElement.state),
87 - }: any): InspectedElement);
88 -
89 - const request = inProgressRequests.get(id);
90 - if (request != null) {
91 - inProgressRequests.delete(id);
92 - request.resolveFn(inspectedElement);
93 - } else {
94 - resource.write(id, inspectedElement);
95 -
96 - // Schedule update with React if the curently-selected element has been invalidated.
97 - if (id === selectedElementID) {
98 - setCount(count => count + 1);
99 - }
72 + const onInspectedElement = (inspectedElement: InspectedElement | null) => {
73 + if (inspectedElement !== null) {
74 + const id = inspectedElement.id;
75 +
76 + inspectedElement = (({
77 + ...inspectedElement,
78 + context: hydrateHelper(inspectedElement.context),
79 + hooks: hydrateHelper(inspectedElement.hooks),
80 + props: hydrateHelper(inspectedElement.props),
81 + state: hydrateHelper(inspectedElement.state),
82 + }: any): InspectedElement);
83 +
84 + const request = inProgressRequests.get(id);
85 + if (request != null) {
86 + inProgressRequests.delete(id);
87 + request.resolveFn(inspectedElement);
88 + } else {
89 + resource.write(id, inspectedElement);
90 +
91 + // Schedule update with React if the curently-selected element has been invalidated.
92 + if (id === selectedElementID) {
93 + setCount(count => count + 1);
94 }
95 }
96 }
@@ -129,12 +123,10 @@ function InspectedElementContextController({ children }: Props) {
123 // Update the $r variable.
124 bridge.send('selectElement', { id: selectedElementID, rendererID });
125
132 - const onInspectedElement = (
133 - inspectedElementResponse: InspectedElementResponse | null
134 - ) => {
126 + const onInspectedElement = (inspectedElement: InspectedElement | null) => {
127 if (
136 - inspectedElementResponse !== null &&
137 - inspectedElementResponse.id === selectedElementID
128 + inspectedElement !== null &&
129 + inspectedElement.id === selectedElementID
130 ) {
131 // If this is the element we requested, wait a little bit and then ask for an update.
132 timeoutID = setTimeout(sendRequest, 1000);
src/devtools/views/Components/types.js
-5
@@ -65,11 +65,6 @@ export type InspectedElement = {|
65 source: Object | null,
66 |};
67
68 -export type InspectedElementResponse = {|
69 - id: number,
70 - inspectedElement: InspectedElement | null,
71 -|};
72 -
68 // TODO: Add profiling type
69
70 export type DehydratedData = {|