From 195b03e196df53ea52c063b0c7846873b1ed6953 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Wed, 7 Sep 2016 20:08:09 -0700 Subject: [PATCH] Type ReactHostOperationHistoryHook (#7672) In order to properly type an `Operation`, we need to change the call site from having two arguments: one for `type` and one for `payload` into an object that contains both. This isn't a perf regression because we were already constructing this object in the first place and doesn't change the emitted event so shouldn't affect the dev tools. None of the call sites are actually flow-ified so it isn't technically used but once we will, it'll make sure that we don't send random strings and payload through those very generic methods. (cherry picked from commit eaefd9052ae41d3318a2add286828a75da362193) --- src/renderers/dom/client/ReactMount.js | 10 +-- .../dom/client/utils/DOMChildrenOperations.js | 80 +++++++++---------- .../dom/shared/CSSPropertyOperations.js | 10 +-- .../dom/shared/DOMPropertyOperations.js | 40 +++++----- src/renderers/shared/ReactDebugTool.js | 23 +----- .../hooks/ReactHostOperationHistoryHook.js | 29 ++++--- 6 files changed, 94 insertions(+), 98 deletions(-) diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index d7f24aaf38..c0e3c9ec00 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -729,11 +729,11 @@ var ReactMount = { if (__DEV__) { var hostNode = ReactDOMComponentTree.getInstanceFromNode(container.firstChild); if (hostNode._debugID !== 0) { - ReactInstrumentation.debugTool.onHostOperation( - hostNode._debugID, - 'mount', - markup.toString() - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: hostNode._debugID, + type: 'mount', + payload: markup.toString(), + }); } } }, diff --git a/src/renderers/dom/client/utils/DOMChildrenOperations.js b/src/renderers/dom/client/utils/DOMChildrenOperations.js index f13b16b3f6..2200f2529a 100644 --- a/src/renderers/dom/client/utils/DOMChildrenOperations.js +++ b/src/renderers/dom/client/utils/DOMChildrenOperations.js @@ -122,11 +122,11 @@ function replaceDelimitedText(openingComment, closingComment, stringText) { } if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - ReactDOMComponentTree.getInstanceFromNode(openingComment)._debugID, - 'replace text', - stringText - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: ReactDOMComponentTree.getInstanceFromNode(openingComment)._debugID, + type: 'replace text', + payload: stringText, + }); } } @@ -135,19 +135,19 @@ if (__DEV__) { dangerouslyReplaceNodeWithMarkup = function(oldChild, markup, prevInstance) { Danger.dangerouslyReplaceNodeWithMarkup(oldChild, markup); if (prevInstance._debugID !== 0) { - ReactInstrumentation.debugTool.onHostOperation( - prevInstance._debugID, - 'replace with', - markup.toString() - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: prevInstance._debugID, + type: 'replace with', + payload: markup.toString(), + }); } else { var nextInstance = ReactDOMComponentTree.getInstanceFromNode(markup.node); if (nextInstance._debugID !== 0) { - ReactInstrumentation.debugTool.onHostOperation( - nextInstance._debugID, - 'mount', - markup.toString() - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: nextInstance._debugID, + type: 'mount', + payload: markup.toString(), + }); } } }; @@ -185,11 +185,11 @@ var DOMChildrenOperations = { getNodeAfter(parentNode, update.afterNode) ); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - parentNodeDebugID, - 'insert child', - {toIndex: update.toIndex, content: update.content.toString()} - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: parentNodeDebugID, + type: 'insert child', + payload: {toIndex: update.toIndex, content: update.content.toString()}, + }); } break; case 'MOVE_EXISTING': @@ -199,11 +199,11 @@ var DOMChildrenOperations = { getNodeAfter(parentNode, update.afterNode) ); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - parentNodeDebugID, - 'move child', - {fromIndex: update.fromIndex, toIndex: update.toIndex} - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: parentNodeDebugID, + type: 'move child', + payload: {fromIndex: update.fromIndex, toIndex: update.toIndex}, + }); } break; case 'SET_MARKUP': @@ -212,11 +212,11 @@ var DOMChildrenOperations = { update.content ); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - parentNodeDebugID, - 'replace children', - update.content.toString() - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: parentNodeDebugID, + type: 'replace children', + payload: update.content.toString(), + }); } break; case 'TEXT_CONTENT': @@ -225,21 +225,21 @@ var DOMChildrenOperations = { update.content ); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - parentNodeDebugID, - 'replace text', - update.content.toString() - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: parentNodeDebugID, + type: 'replace text', + payload: update.content.toString(), + }); } break; case 'REMOVE_NODE': removeChild(parentNode, update.fromNode); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - parentNodeDebugID, - 'remove child', - {fromIndex: update.fromIndex} - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: parentNodeDebugID, + type: 'remove child', + payload: {fromIndex: update.fromIndex}, + }); } break; } diff --git a/src/renderers/dom/shared/CSSPropertyOperations.js b/src/renderers/dom/shared/CSSPropertyOperations.js index c3222c202f..81c5319959 100644 --- a/src/renderers/dom/shared/CSSPropertyOperations.js +++ b/src/renderers/dom/shared/CSSPropertyOperations.js @@ -193,11 +193,11 @@ var CSSPropertyOperations = { */ setValueForStyles: function(node, styles, component) { if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - component._debugID, - 'update styles', - styles - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: component._debugID, + type: 'update styles', + payload: styles, + }); } var style = node.style; diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 3d6cb7f59c..895a7ec71e 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -166,11 +166,11 @@ var DOMPropertyOperations = { if (__DEV__) { var payload = {}; payload[name] = value; - ReactInstrumentation.debugTool.onHostOperation( - ReactDOMComponentTree.getInstanceFromNode(node)._debugID, - 'update attribute', - payload - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: ReactDOMComponentTree.getInstanceFromNode(node)._debugID, + type: 'update attribute', + payload: payload, + }); } }, @@ -187,11 +187,11 @@ var DOMPropertyOperations = { if (__DEV__) { var payload = {}; payload[name] = value; - ReactInstrumentation.debugTool.onHostOperation( - ReactDOMComponentTree.getInstanceFromNode(node)._debugID, - 'update attribute', - payload - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: ReactDOMComponentTree.getInstanceFromNode(node)._debugID, + type: 'update attribute', + payload: payload, + }); } }, @@ -204,11 +204,11 @@ var DOMPropertyOperations = { deleteValueForAttribute: function(node, name) { node.removeAttribute(name); if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - ReactDOMComponentTree.getInstanceFromNode(node)._debugID, - 'remove attribute', - name - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: ReactDOMComponentTree.getInstanceFromNode(node)._debugID, + type: 'remove attribute', + payload: name, + }); } }, @@ -240,11 +240,11 @@ var DOMPropertyOperations = { } if (__DEV__) { - ReactInstrumentation.debugTool.onHostOperation( - ReactDOMComponentTree.getInstanceFromNode(node)._debugID, - 'remove attribute', - name - ); + ReactInstrumentation.debugTool.onHostOperation({ + instanceID: ReactDOMComponentTree.getInstanceFromNode(node)._debugID, + type: 'remove attribute', + payload: name, + }); } }, diff --git a/src/renderers/shared/ReactDebugTool.js b/src/renderers/shared/ReactDebugTool.js index 31ef8c7073..06581e5c74 100644 --- a/src/renderers/shared/ReactDebugTool.js +++ b/src/renderers/shared/ReactDebugTool.js @@ -23,9 +23,9 @@ var warning = require('warning'); import type { ReactElement } from 'ReactElementType'; import type { DebugID } from 'ReactInstanceType'; +import type { Operation } from 'ReactHostOperationHistoryHook'; type Hook = any; -type Payload = mixed; type TimerType = 'ctor' | @@ -38,21 +38,6 @@ type TimerType = 'componentDidUpdate' | 'componentDidMount'; -type HostOperationType = - 'mount' | - 'insert child' | - 'move child' | - 'remove child' | - 'replace children' | - 'replace text' | - 'replace with'; - -type Operation = { - instanceID: DebugID, - type: string, - payload: Payload, -}; - type Measurement = { timerType: TimerType, instanceID: DebugID, @@ -378,9 +363,9 @@ var ReactDebugTool = { onEndProcessingChildContext(): void { emitEvent('onEndProcessingChildContext'); }, - onHostOperation(debugID: DebugID, type: HostOperationType, payload: Payload): void { - checkDebugID(debugID); - emitEvent('onHostOperation', debugID, type, payload); + onHostOperation(operation: Operation) { + checkDebugID(operation.instanceID); + emitEvent('onHostOperation', operation); }, onSetState(): void { emitEvent('onSetState'); diff --git a/src/renderers/shared/hooks/ReactHostOperationHistoryHook.js b/src/renderers/shared/hooks/ReactHostOperationHistoryHook.js index 48456caea9..675a00e732 100644 --- a/src/renderers/shared/hooks/ReactHostOperationHistoryHook.js +++ b/src/renderers/shared/hooks/ReactHostOperationHistoryHook.js @@ -7,22 +7,33 @@ * of patent rights can be found in the PATENTS file in the same directory. * * @providesModule ReactHostOperationHistoryHook + * @flow */ 'use strict'; -var history = []; +import type { DebugID } from 'ReactInstanceType'; + +export type Operation = {instanceID: DebugID} & ( + {type: 'mount', payload: string} | + {type: 'insert child', payload: {toIndex: number, content: string}} | + {type: 'move child', payload: {fromIndex: number, toIndex: number}} | + {type: 'replace children', payload: string} | + {type: 'replace text', payload: string} | + {type: 'replace with', payload: string} | + {type: 'update styles', payload: mixed /* Style Object */} | + {type: 'update attribute', payload: {[name: string]: string}} | + {type: 'remove attribute', payload: string} +); + +var history: Array = []; var ReactHostOperationHistoryHook = { - onHostOperation(debugID, type, payload) { - history.push({ - instanceID: debugID, - type, - payload, - }); + onHostOperation(operation: Operation) { + history.push(operation); }, - clearHistory() { + clearHistory(): void { if (ReactHostOperationHistoryHook._preventClearing) { // Should only be used for tests. return; @@ -31,7 +42,7 @@ var ReactHostOperationHistoryHook = { history = []; }, - getHistory() { + getHistory(): Array { return history; }, };