{"body":"Eagerly inject renderer (before document) to support reload-and-profile","commit":{"authoredAt":"2019-04-01T14:47:51-07:00","revision":"b8ada905ff42ab46c162ed241b04c92a8e2bdafb","shortRevision":"b8ada905ff","subject":"Merge pull request #35 from bvaughn\/reload-and-profile","authorEmail":"brian.david.vaughn@gmail.com","authorName":"Brian Vaughn"},"parents":["37ed5a7175fe6f7350f4a9a847e50730e34e1bf3","2a80f8ca9ca0be44cca9539d83ede9a5d62b62d0"],"patch":"diff --git a\/shells\/browser\/chrome\/manifest.json b\/shells\/browser\/chrome\/manifest.json\nindex 2458a4a0f5..6a26c7b21a 100644\n--- a\/shells\/browser\/chrome\/manifest.json\n+++ b\/shells\/browser\/chrome\/manifest.json\n@@ -27,7 +27,12 @@\n \"devtools_page\": \"main.html\",\n \n \"content_security_policy\": \"script-src 'self' 'unsafe-eval'; object-src 'self'\",\n- \"web_accessible_resources\": [\"main.html\", \"panel.html\", \"build\/backend.js\"],\n+ \"web_accessible_resources\": [\n+ \"main.html\",\n+ \"panel.html\",\n+ \"build\/backend.js\",\n+ \"build\/renderer.js\"\n+ ],\n \n \"background\": {\n \"scripts\": [\"build\/background.js\"],\ndiff --git a\/shells\/browser\/firefox\/manifest.json b\/shells\/browser\/firefox\/manifest.json\nindex 0cff52c594..7f6a1281b5 100644\n--- a\/shells\/browser\/firefox\/manifest.json\n+++ b\/shells\/browser\/firefox\/manifest.json\n@@ -33,7 +33,12 @@\n \"devtools_page\": \"main.html\",\n \n \"content_security_policy\": \"script-src 'self' 'unsafe-eval'; object-src 'self'\",\n- \"web_accessible_resources\": [\"main.html\", \"panel.html\", \"build\/backend.js\"],\n+ \"web_accessible_resources\": [\n+ \"main.html\",\n+ \"panel.html\",\n+ \"build\/backend.js\",\n+ \"build\/renderer.js\"\n+ ],\n \n \"background\": {\n \"scripts\": [\"build\/background.js\"],\ndiff --git a\/shells\/browser\/shared\/src\/GlobalHook.js b\/shells\/browser\/shared\/src\/GlobalHook.js\nindex 9ccab3bb8a..074b289db6 100644\n--- a\/shells\/browser\/shared\/src\/GlobalHook.js\n+++ b\/shells\/browser\/shared\/src\/GlobalHook.js\n@@ -2,13 +2,24 @@\n \n import nullthrows from 'nullthrows';\n import { installHook } from 'src\/hook';\n+import { RELOAD_AND_PROFILE_KEY } from 'src\/constants';\n+\n+function injectCode(code) {\n+ const script = document.createElement('script');\n+ script.textContent = code;\n+\n+ \/\/ This script runs before the element is created,\n+ \/\/ so we add the script to instead.\n+ nullthrows(document.documentElement).appendChild(script);\n+ nullthrows(script.parentNode).removeChild(script);\n+}\n \n let lastDetectionResult;\n \n-\/\/ We want to detect when a renderer attaches, and notify the \"background\n-\/\/ page\" (which is shared between tabs and can highlight the React icon).\n-\/\/ Currently we are in \"content script\" context, so we can't listen\n-\/\/ to the hook directly (it will be injected directly into the page).\n+\/\/ We want to detect when a renderer attaches, and notify the \"background page\"\n+\/\/ (which is shared between tabs and can highlight the React icon).\n+\/\/ Currently we are in \"content script\" context, so we can't listen to the hook directly\n+\/\/ (it will be injected directly into the page).\n \/\/ So instead, the hook will use postMessage() to pass message to us here.\n \/\/ And when this happens, we'll send a message to the \"background page\".\n window.addEventListener('message', function(evt) {\n@@ -51,14 +62,27 @@ window.__REACT_DEVTOOLS_GLOBAL_HOOK__.nativeWeakMap = WeakMap;\n window.__REACT_DEVTOOLS_GLOBAL_HOOK__.nativeSet = Set;\n `;\n \n+\/\/ If we have just reloaded to profile, we need to inject the renderer interface before the app loads.\n+if (localStorage.getItem(RELOAD_AND_PROFILE_KEY) === 'true') {\n+ const rendererURL = chrome.runtime.getURL('build\/renderer.js');\n+ let rendererCode;\n+\n+ \/\/ We need to inject in time to catch the initial mount.\n+ \/\/ This means we need to synchronously read the renderer code itself,\n+ \/\/ and synchronously inject it into the page.\n+ \/\/ There are very few ways to actually do this.\n+ \/\/ This seems to be the best approach.\n+ const request = new XMLHttpRequest();\n+ request.addEventListener('load', function() {\n+ rendererCode = this.responseText;\n+ });\n+ request.open('GET', rendererURL, false);\n+ request.send();\n+ injectCode(rendererCode);\n+}\n+\n \/\/ Inject a `__REACT_DEVTOOLS_GLOBAL_HOOK__` global so that React can detect that the\n \/\/ devtools are installed (and skip its suggestion to install the devtools).\n-const js =\n- ';(' + installHook.toString() + '(window))' + saveNativeValues + detectReact;\n-\n-\/\/ This script runs before the element is created, so we add the script\n-\/\/ to instead.\n-const script = document.createElement('script');\n-script.textContent = js;\n-nullthrows(document.documentElement).appendChild(script);\n-nullthrows(script.parentNode).removeChild(script);\n+injectCode(\n+ ';(' + installHook.toString() + '(window))' + saveNativeValues + detectReact\n+);\ndiff --git a\/shells\/browser\/shared\/src\/main.js b\/shells\/browser\/shared\/src\/main.js\nindex 08befa651b..d1d4eb1024 100644\n--- a\/shells\/browser\/shared\/src\/main.js\n+++ b\/shells\/browser\/shared\/src\/main.js\n@@ -78,15 +78,18 @@ function createPanelIfReactLoaded() {\n \/\/ This flag lets us tip the Store off early that we expect to be profiling.\n \/\/ This avoids flashing a temporary \"Profiling not supported\" message in the Profiler tab,\n \/\/ after a user has clicked the \"reload and profile\" button.\n+ let isProfiling = false;\n let supportsProfiling = false;\n if (localStorage.getItem(SUPPORTS_PROFILING_KEY) === 'true') {\n supportsProfiling = true;\n+ isProfiling = true;\n localStorage.removeItem(SUPPORTS_PROFILING_KEY);\n }\n \n const browserName = getBrowserName();\n \n store = new Store(bridge, {\n+ isProfiling,\n supportsFileDownloads: browserName === 'Chrome',\n supportsReloadAndProfile: true,\n supportsProfiling,\ndiff --git a\/shells\/browser\/shared\/src\/renderer.js b\/shells\/browser\/shared\/src\/renderer.js\nnew file mode 100644\nindex 0000000000..c4cc4c6f55\n--- \/dev\/null\n+++ b\/shells\/browser\/shared\/src\/renderer.js\n@@ -0,0 +1,22 @@\n+\/**\n+ * In order to support reload-and-profile functionality, the renderer needs to be injected before any other scripts.\n+ * Since it is a complex file (with imports) we can't just toString() it like we do with the hook itself,\n+ * So this entry point (one of the web_accessible_resources) provcides a way to eagerly inject it.\n+ * The hook will look for the presence of a global __REACT_DEVTOOLS_ATTACH__ and attach an injected renderer early.\n+ * The normal case (not a reload-and-profile) will not make use of this entry point though.\n+ *\n+ * @flow\n+ *\/\n+\n+import { attach } from 'src\/backend\/renderer';\n+\n+Object.defineProperty(\n+ window,\n+ '__REACT_DEVTOOLS_ATTACH__',\n+ ({\n+ enumerable: false,\n+ get() {\n+ return attach;\n+ },\n+ }: Object)\n+);\ndiff --git a\/shells\/browser\/shared\/webpack.config.js b\/shells\/browser\/shared\/webpack.config.js\nindex 5c11458ac7..b33c9248dd 100644\n--- a\/shells\/browser\/shared\/webpack.config.js\n+++ b\/shells\/browser\/shared\/webpack.config.js\n@@ -18,6 +18,7 @@ module.exports = {\n inject: '.\/src\/GlobalHook.js',\n main: '.\/src\/main.js',\n panel: '.\/src\/panel.js',\n+ renderer: '.\/src\/renderer.js',\n },\n output: {\n path: __dirname + '\/build',\ndiff --git a\/src\/backend\/agent.js b\/src\/backend\/agent.js\nindex ef363dbaae..dab48def13 100644\n--- a\/src\/backend\/agent.js\n+++ b\/src\/backend\/agent.js\n@@ -1,7 +1,7 @@\n \/\/ @flow\n \n import EventEmitter from 'events';\n-import { __DEBUG__ } from '..\/constants';\n+import { RELOAD_AND_PROFILE_KEY, __DEBUG__ } from '..\/constants';\n import { hideOverlay, showOverlay } from '.\/views\/Highlighter';\n \n import type { RendererID, RendererInterface } from '.\/types';\n@@ -38,8 +38,6 @@ type SetInParams = {|\n value: any,\n |};\n \n-const RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile';\n-\n export default class Agent extends EventEmitter {\n _bridge: Bridge = ((null: any): Bridge);\n _isProfiling: boolean = false;\ndiff --git a\/src\/backend\/index.js b\/src\/backend\/index.js\nindex 4b08474056..298d06a121 100644\n--- a\/src\/backend\/index.js\n+++ b\/src\/backend\/index.js\n@@ -23,7 +23,10 @@ export function initBackend(\n rendererInterface: RendererInterface,\n }) => {\n agent.setRendererInterface(id, rendererInterface);\n- rendererInterface.walkTree();\n+\n+ \/\/ Now that the Store and the renderer interface are connected,\n+ \/\/ it's time to flush the pending operation codes to the frontend.\n+ rendererInterface.flushInitialOperations();\n }\n ),\n \n@@ -33,8 +36,17 @@ export function initBackend(\n ];\n \n const attachRenderer = (id: number, renderer: ReactRenderer) => {\n- const rendererInterface = attach(hook, id, renderer, global);\n- hook.rendererInterfaces.set(id, rendererInterface);\n+ let rendererInterface = hook.rendererInterfaces.get(id);\n+\n+ \/\/ Inject any not-yet-injected renderers (if we didn't reload-and-profile)\n+ if (!rendererInterface) {\n+ rendererInterface = attach(hook, id, renderer, global);\n+\n+ hook.rendererInterfaces.set(id, rendererInterface);\n+ }\n+\n+ \/\/ Notify the DevTools frontend about new renderers.\n+ \/\/ This includes any that were attached early (via __REACT_DEVTOOLS_ATTACH__).\n hook.emit('renderer-attached', {\n id,\n renderer,\ndiff --git a\/src\/backend\/renderer.js b\/src\/backend\/renderer.js\nindex 6a43c969ef..821060fc78 100644\n--- a\/src\/backend\/renderer.js\n+++ b\/src\/backend\/renderer.js\n@@ -18,6 +18,7 @@ import { getDisplayName, utfEncodeString } from '..\/utils';\n import { cleanForBridge, copyWithSet, setInObject } from '.\/utils';\n import {\n __DEBUG__,\n+ RELOAD_AND_PROFILE_KEY,\n TREE_OPERATION_ADD,\n TREE_OPERATION_REMOVE,\n TREE_OPERATION_RESET_CHILDREN,\n@@ -566,6 +567,7 @@ export function attach(\n }\n \n let pendingOperations: Uint32Array = new Uint32Array(0);\n+ let pendingOperationsQueue: Array | null = [];\n \n function addOperation(\n newAction: Uint32Array,\n@@ -603,7 +605,16 @@ export function attach(\n \/\/ Let the frontend know about tree operations.\n \/\/ The first value in this array will identify which root it corresponds to,\n \/\/ so we do no longer need to dispatch a separate root-committed event.\n- hook.emit('operations', pendingOperations);\n+ if (pendingOperationsQueue !== null) {\n+ \/\/ Until the frontend has been connected, store the tree operations.\n+ \/\/ This will let us avoid walking the tree later when the frontend connects,\n+ \/\/ and it enables the Profiler's reload-and-profile functionality to work as well.\n+ pendingOperationsQueue.push(pendingOperations);\n+ } else {\n+ \/\/ If we've already connected to the frontend, just pass the operations through.\n+ hook.emit('operations', pendingOperations);\n+ }\n+\n pendingOperations = new Uint32Array(0);\n }\n \n@@ -913,31 +924,46 @@ export function attach(\n \/\/ We don't patch any methods so there is no cleanup.\n }\n \n- function walkTree() {\n- \/\/ Hydrate all the roots for the first time.\n- hook.getFiberRoots(rendererID).forEach(root => {\n- currentRootID = getFiberID(getPrimaryFiber(root.current));\n+ function flushInitialOperations() {\n+ const localPendingOperationsQueue = pendingOperationsQueue;\n \n- if (isProfiling) {\n- \/\/ If profiling is active, store commit time and duration, and the current interactions.\n- \/\/ The frontend may request this information after profiling has stopped.\n- currentCommitProfilingMetadata = {\n- actualDurations: [],\n- commitTime: performance.now() - profilingStartTime,\n- interactions: Array.from(root.memoizedInteractions).map(\n- (interaction: Interaction) => ({\n- ...interaction,\n- timestamp: interaction.timestamp - profilingStartTime,\n- })\n- ),\n- maxActualDuration: 0,\n- };\n- }\n+ pendingOperationsQueue = null;\n \n- mountFiber(root.current, null);\n- flushPendingEvents(root);\n- currentRootID = -1;\n- });\n+ if (\n+ localPendingOperationsQueue !== null &&\n+ localPendingOperationsQueue.length > 0\n+ ) {\n+ \/\/ We may have already queued up some operations before the frontend connected\n+ \/\/ If so, let the frontend know about them.\n+ localPendingOperationsQueue.forEach(pendingOperations => {\n+ hook.emit('operations', pendingOperations);\n+ });\n+ } else {\n+ \/\/ If we have not been profiling, then we can just walk the tree and build up its current state as-is.\n+ hook.getFiberRoots(rendererID).forEach(root => {\n+ currentRootID = getFiberID(getPrimaryFiber(root.current));\n+\n+ if (isProfiling) {\n+ \/\/ If profiling is active, store commit time and duration, and the current interactions.\n+ \/\/ The frontend may request this information after profiling has stopped.\n+ currentCommitProfilingMetadata = {\n+ actualDurations: [],\n+ commitTime: performance.now() - profilingStartTime,\n+ interactions: Array.from(root.memoizedInteractions).map(\n+ (interaction: Interaction) => ({\n+ ...interaction,\n+ timestamp: interaction.timestamp - profilingStartTime,\n+ })\n+ ),\n+ maxActualDuration: 0,\n+ };\n+ }\n+\n+ mountFiber(root.current, null);\n+ flushPendingEvents(root);\n+ currentRootID = -1;\n+ });\n+ }\n }\n \n function handleCommitFiberUnmount(fiber) {\n@@ -1595,6 +1621,10 @@ export function attach(\n }\n \n function startProfiling() {\n+ if (isProfiling) {\n+ return;\n+ }\n+\n \/\/ Capture initial values as of the time profiling starts.\n \/\/ It's important we snapshot both the durations and the id-to-root map,\n \/\/ since either of these may change during the profiling session\n@@ -1611,8 +1641,14 @@ export function attach(\n isProfiling = false;\n }\n \n+ \/\/ Automatically start profiling so that we don't miss timing info from initial \"mount\".\n+ if (localStorage.getItem(RELOAD_AND_PROFILE_KEY) === 'true') {\n+ startProfiling();\n+ }\n+\n return {\n cleanup,\n+ flushInitialOperations,\n getCommitDetails,\n getFiberIDFromNative,\n getInteractions,\n@@ -1631,6 +1667,5 @@ export function attach(\n setInState,\n startProfiling,\n stopProfiling,\n- walkTree,\n };\n }\ndiff --git a\/src\/backend\/types.js b\/src\/backend\/types.js\nindex 9e85dac6d2..8768715d41 100644\n--- a\/src\/backend\/types.js\n+++ b\/src\/backend\/types.js\n@@ -82,6 +82,7 @@ export type ProfilingSummary = {|\n \n export type RendererInterface = {\n cleanup: () => void,\n+ flushInitialOperations: () => void,\n getCommitDetails: (rootID: number, commitIndex: number) => CommitDetails,\n getNativeFromReactElement?: ?(component: Fiber) => ?NativeType,\n getFiberIDFromNative: (\n@@ -108,7 +109,6 @@ export type RendererInterface = {\n setInState: (id: number, path: Array, value: any) => void,\n startProfiling: () => void,\n stopProfiling: () => void,\n- walkTree: () => void,\n };\n \n export type Handler = (data: any) => void;\ndiff --git a\/src\/constants.js b\/src\/constants.js\nindex 1800fa8001..3523aa5214 100644\n--- a\/src\/constants.js\n+++ b\/src\/constants.js\n@@ -5,4 +5,6 @@ export const TREE_OPERATION_REMOVE = 2;\n export const TREE_OPERATION_RESET_CHILDREN = 3;\n export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4;\n \n+export const RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile';\n+\n export const __DEBUG__ = false;\ndiff --git a\/src\/devtools\/store.js b\/src\/devtools\/store.js\nindex 708f3ee50b..345dee9b0c 100644\n--- a\/src\/devtools\/store.js\n+++ b\/src\/devtools\/store.js\n@@ -103,10 +103,14 @@ export default class Store extends EventEmitter {\n \n if (config != null) {\n const {\n+ isProfiling,\n supportsFileDownloads,\n supportsProfiling,\n supportsReloadAndProfile,\n } = config;\n+ if (isProfiling) {\n+ this._isProfiling = true;\n+ }\n if (supportsFileDownloads) {\n this._supportsFileDownloads = true;\n }\ndiff --git a\/src\/devtools\/views\/Profiler\/CommitTreeBuilder.js b\/src\/devtools\/views\/Profiler\/CommitTreeBuilder.js\nindex 92e85c62ad..2f714aaa9d 100644\n--- a\/src\/devtools\/views\/Profiler\/CommitTreeBuilder.js\n+++ b\/src\/devtools\/views\/Profiler\/CommitTreeBuilder.js\n@@ -112,14 +112,9 @@ export function getCommitTree({\n }\n }\n \n- console.error(\n+ throw Error(\n `getCommitTree(): Unable to reconstruct tree for root \"${rootID}\" and commit ${commitIndex}`\n );\n-\n- return {\n- nodes: new Map(),\n- rootID,\n- };\n }\n \n function recursivelyIniitliazeTree(\ndiff --git a\/src\/devtools\/views\/Profiler\/FlamegraphChartBuilder.js b\/src\/devtools\/views\/Profiler\/FlamegraphChartBuilder.js\nindex df183d63d9..9a7339cd01 100644\n--- a\/src\/devtools\/views\/Profiler\/FlamegraphChartBuilder.js\n+++ b\/src\/devtools\/views\/Profiler\/FlamegraphChartBuilder.js\n@@ -56,6 +56,11 @@ export function getChartData({\n idToDepthMap.set(id, currentDepth);\n \n const node = ((nodes.get(id): any): Node);\n+\n+ if (node == null) {\n+ throw Error(`Could not find node with id \"${id}\" in commit tree`);\n+ }\n+\n const name = node.displayName || 'Unknown';\n \n const selfDuration = calculateSelfDuration(id, commitTree, commitDetails);\ndiff --git a\/src\/devtools\/views\/Profiler\/ProfilerContext.js b\/src\/devtools\/views\/Profiler\/ProfilerContext.js\nindex 123386e156..b50cd12f98 100644\n--- a\/src\/devtools\/views\/Profiler\/ProfilerContext.js\n+++ b\/src\/devtools\/views\/Profiler\/ProfilerContext.js\n@@ -130,7 +130,7 @@ function ProfilerContextController({ children }: Props) {\n const [\n isCommitFilterEnabled,\n setIsCommitFilterEnabled,\n- ] = useLocalStorage('isCommitFilterEnabled', false);\n+ ] = useLocalStorage('React::DevTools::isCommitFilterEnabled', false);\n const [minCommitDuration, setMinCommitDuration] = useLocalStorage(\n 'minCommitDuration',\n 0\ndiff --git a\/src\/devtools\/views\/Profiler\/RankedChartBuilder.js b\/src\/devtools\/views\/Profiler\/RankedChartBuilder.js\nindex f900a6d3f0..b514cac3bc 100644\n--- a\/src\/devtools\/views\/Profiler\/RankedChartBuilder.js\n+++ b\/src\/devtools\/views\/Profiler\/RankedChartBuilder.js\n@@ -41,6 +41,10 @@ export function getChartData({\n actualDurations.forEach((actualDuration, id) => {\n const node = ((nodes.get(id): any): Node);\n \n+ if (node == null) {\n+ throw Error(`Could not find node with id \"${id}\" in commit tree`);\n+ }\n+\n \/\/ Don't show the root node in this chart.\n if (node.parentID === 0) {\n return;\ndiff --git a\/src\/devtools\/views\/Settings\/SettingsContext.js b\/src\/devtools\/views\/Settings\/SettingsContext.js\nindex 4e51b5554e..c47fc224b4 100644\n--- a\/src\/devtools\/views\/Settings\/SettingsContext.js\n+++ b\/src\/devtools\/views\/Settings\/SettingsContext.js\n@@ -41,10 +41,13 @@ function SettingsContextController({\n settingsPortalContainer,\n }: Props) {\n const [displayDensity, setDisplayDensity] = useLocalStorage(\n- 'displayDensity',\n+ 'React::DevTools::displayDensity',\n 'compact'\n );\n- const [theme, setTheme] = useLocalStorage('theme', 'auto');\n+ const [theme, setTheme] = useLocalStorage(\n+ 'React::DevTools::theme',\n+ 'auto'\n+ );\n \n const documentElements = useMemo(() => {\n const array: Array = [\ndiff --git a\/src\/hook.js b\/src\/hook.js\nindex e5d66c40ac..fa960d73be 100644\n--- a\/src\/hook.js\n+++ b\/src\/hook.js\n@@ -77,6 +77,14 @@ export function installHook(target: any): DevToolsHook | null {\n ? 'deadcode'\n : detectReactBuildType(renderer);\n \n+ \/\/ If we have just reloaded to profile, we need to inject the renderer interface before the app loads.\n+ \/\/ Otherwise the renderer won't yet exist and we can skip this step.\n+ const attach = target.__REACT_DEVTOOLS_ATTACH__;\n+ if (typeof attach === 'function') {\n+ const rendererInterface = attach(hook, id, renderer, target);\n+ hook.rendererInterfaces.set(id, rendererInterface);\n+ }\n+\n hook.emit('renderer', { id, renderer, reactBuildType });\n \n return id;\n","files":[{"path":"shells\/browser\/chrome\/manifest.json","status":"M"},{"path":"shells\/browser\/firefox\/manifest.json","status":"M"},{"path":"shells\/browser\/shared\/src\/GlobalHook.js","status":"M"},{"path":"shells\/browser\/shared\/src\/main.js","status":"M"},{"path":"shells\/browser\/shared\/src\/renderer.js","status":"A"},{"path":"shells\/browser\/shared\/webpack.config.js","status":"M"},{"path":"src\/backend\/agent.js","status":"M"},{"path":"src\/backend\/index.js","status":"M"},{"path":"src\/backend\/renderer.js","status":"M"},{"path":"src\/backend\/types.js","status":"M"},{"path":"src\/constants.js","status":"M"},{"path":"src\/devtools\/store.js","status":"M"},{"path":"src\/devtools\/views\/Profiler\/CommitTreeBuilder.js","status":"M"},{"path":"src\/devtools\/views\/Profiler\/FlamegraphChartBuilder.js","status":"M"},{"path":"src\/devtools\/views\/Profiler\/ProfilerContext.js","status":"M"},{"path":"src\/devtools\/views\/Profiler\/RankedChartBuilder.js","status":"M"},{"path":"src\/devtools\/views\/Settings\/SettingsContext.js","status":"M"},{"path":"src\/hook.js","status":"M"}]}

dillon1000/react

Files changed