From 6fd2e72b5e06358ebe6ab719edbbde2a1845ce4a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 14 May 2019 11:01:41 -0700 Subject: [PATCH] Minor nits --- src/__tests__/setupTests.js | 54 +++++++++------------ src/__tests__/utils.js | 6 +-- src/backend/agent.js | 4 +- src/backend/renderer.js | 45 ++++++++--------- src/devtools/ProfilingCache.js | 88 ++++++++++++++++------------------ 5 files changed, 87 insertions(+), 110 deletions(-) diff --git a/src/__tests__/setupTests.js b/src/__tests__/setupTests.js index c259d8d434..2cd71ae731 100644 --- a/src/__tests__/setupTests.js +++ b/src/__tests__/setupTests.js @@ -31,43 +31,33 @@ env.beforeEach(() => { installHook(global); - function init() { - const bridgeListeners = []; - const bridge = new Bridge({ - listen(callback) { - bridgeListeners.push(callback); - return () => { - const index = bridgeListeners.indexOf(callback); - if (index >= 0) { - bridgeListeners.splice(index, 1); - } - }; - }, - send(event: string, payload: any, transferable?: Array) { - bridgeListeners.forEach(callback => callback({ event, payload })); - }, - }); + const bridgeListeners = []; + const bridge = new Bridge({ + listen(callback) { + bridgeListeners.push(callback); + return () => { + const index = bridgeListeners.indexOf(callback); + if (index >= 0) { + bridgeListeners.splice(index, 1); + } + }; + }, + send(event: string, payload: any, transferable?: Array) { + bridgeListeners.forEach(callback => callback({ event, payload })); + }, + }); - const agent = new Agent(bridge); + const agent = new Agent(bridge); - const hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__; + const hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__; - const destroyBackend = initBackend(hook, agent, global); + initBackend(hook, agent, global); - const store = new Store(bridge); + const store = new Store(bridge); - global.agent = agent; - global.bridge = bridge; - global.store = store; - - // Reinit may be used to reset the store and the bridge during a test. - global.reinit = () => { - destroyBackend(); - init(); - }; - } - - init(); + global.agent = agent; + global.bridge = bridge; + global.store = store; }); env.afterEach(() => { delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__; diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index 06b18d0598..20d7bb511c 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -139,7 +139,7 @@ export function exportImportHelper( rendererID: number, rootID: number ): void { - const utils = require('./utils'); + const { act } = require('./utils'); const { prepareExportedProfilingSummary, prepareImportedProfilingData, @@ -153,7 +153,7 @@ export function exportImportHelper( }; bridge.addListener('exportFile', onExportFile); - utils.act(() => { + act(() => { const exportProfilingSummary = prepareExportedProfilingSummary( store.profilingOperations, store.profilingSnapshots, @@ -183,7 +183,7 @@ export function exportImportHelper( // Snapshot the JSON-parsed object, rather than the raw string, because Jest formats the diff nicer. expect(importedProfilingData).toMatchSnapshot('imported data'); - utils.act(() => { + act(() => { store.importedProfilingData = importedProfilingData; }); } diff --git a/src/backend/agent.js b/src/backend/agent.js index 701d4dff79..097ad00827 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -175,8 +175,8 @@ export default class Agent extends EventEmitter { contents: JSON.stringify(exportedProfilingData, null, 2), filename: 'profile-data.json', }); - } catch (ex) { - console.warn(`Unable to export file: ${ex.stack}`); + } catch (error) { + console.warn(`Unable to export file: ${error.stack}`); } }; diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 69aa874d41..c45240e60d 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -784,14 +784,14 @@ export function attach( function recordMount(fiber: Fiber, parentFiber: Fiber | null) { const isRoot = fiber.tag === HostRoot; - const fiberID = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberID(getPrimaryFiber(fiber)); const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); if (isRoot) { pushOperation(TREE_OPERATION_ADD); - pushOperation(fiberID); + pushOperation(id); pushOperation(ElementTypeRoot); pushOperation(isProfilingSupported ? 1 : 0); pushOperation(hasOwnerMetadata ? 1 : 0); @@ -810,7 +810,7 @@ export function attach( let displayNameStringID = getStringID(displayName); let keyStringID = getStringID(key); pushOperation(TREE_OPERATION_ADD); - pushOperation(fiberID); + pushOperation(id); pushOperation(elementType); pushOperation(parentID); pushOperation(ownerID); @@ -819,7 +819,7 @@ export function attach( } if (isProfilingSupported) { - idToRootMap.set(fiberID, currentRootID); + idToRootMap.set(id, currentRootID); recordProfilingDurations(fiber); } @@ -972,10 +972,10 @@ export function attach( } function recordProfilingDurations(fiber: Fiber) { - const fiberID = getFiberID(getPrimaryFiber(fiber)); + const id = getFiberID(getPrimaryFiber(fiber)); const { actualDuration, treeBaseDuration } = fiber; - idToTreeBaseDurationMap.set(fiberID, fiber.treeBaseDuration || 0); + idToTreeBaseDurationMap.set(id, fiber.treeBaseDuration || 0); if (isProfiling) { const { alternate } = fiber; @@ -990,7 +990,7 @@ export function attach( (fiber.treeBaseDuration || 0) * 1000 ); pushOperation(TREE_OPERATION_UPDATE_TREE_BASE_DURATION); - pushOperation(fiberID); + pushOperation(id); pushOperation(treeBaseDuration); } @@ -1014,7 +1014,7 @@ export function attach( // In some cases actualDuration might be 0 for fibers we worked on (particularly if we're using Date.now) // In other cases (e.g. Memo) actualDuration might be greater than 0 even if we "bailed out". const metadata = ((currentCommitProfilingMetadata: any): CommitProfilingData); - metadata.durations.push(fiberID, actualDuration, selfDuration); + metadata.durations.push(id, actualDuration, selfDuration); metadata.maxActualDuration = Math.max( metadata.maxActualDuration, actualDuration @@ -2073,9 +2073,9 @@ export function attach( }; } - function getCommitDetailsForEachCommit( + function getExportedProfilingData( rootID: number - ): Array { + ): ExportedProfilingDataFromRenderer { const commitDetailsForEachCommit = []; const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get( rootID @@ -2085,16 +2085,11 @@ export function attach( commitDetailsForEachCommit.push(getCommitDetails(rootID, index)); } } - return commitDetailsForEachCommit; - } - function getExportedProfilingData( - rootID: number - ): ExportedProfilingDataFromRenderer { return { version: PROFILER_EXPORT_VERSION, profilingSummary: getProfilingSummary(rootID), - commitDetails: getCommitDetailsForEachCommit(rootID), + commitDetails: commitDetailsForEachCommit, interactions: getInteractions(rootID), }; } @@ -2119,14 +2114,14 @@ export function attach( const initialTreeBaseDurations = []; if (initialTreeBaseDurationsMap != null) { - initialTreeBaseDurationsMap.forEach((treeBaseDuration, fiberID) => { + initialTreeBaseDurationsMap.forEach((treeBaseDuration, id) => { if ( initialIDToRootMap != null && - initialIDToRootMap.get(fiberID) === rootID + initialIDToRootMap.get(id) === rootID ) { // We don't need to convert milliseconds to microseconds in this case, // because the profiling summary is JSON serialized. - initialTreeBaseDurations.push(fiberID, treeBaseDuration); + initialTreeBaseDurations.push(id, treeBaseDuration); } }); } @@ -2175,11 +2170,11 @@ export function attach( let forceFallbackForSuspenseIDs = new Set(); function shouldSuspendFiberAccordingToSet(fiber) { - const fiberID = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); - return forceFallbackForSuspenseIDs.has(fiberID); + const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); + return forceFallbackForSuspenseIDs.has(id); } - function overrideSuspense(fiberID, forceFallback) { + function overrideSuspense(id, forceFallback) { if ( typeof setSuspenseHandler !== 'function' || typeof scheduleUpdate !== 'function' @@ -2189,19 +2184,19 @@ export function attach( ); } if (forceFallback) { - forceFallbackForSuspenseIDs.add(fiberID); + forceFallbackForSuspenseIDs.add(id); if (forceFallbackForSuspenseIDs.size === 1) { // First override is added. Switch React to slower path. setSuspenseHandler(shouldSuspendFiberAccordingToSet); } } else { - forceFallbackForSuspenseIDs.delete(fiberID); + forceFallbackForSuspenseIDs.delete(id); if (forceFallbackForSuspenseIDs.size === 0) { // Last override is gone. Switch React back to fast path. setSuspenseHandler(shouldSuspendFiberAlwaysFalse); } } - const fiber = idToFiberMap.get(fiberID); + const fiber = idToFiberMap.get(id); scheduleUpdate(fiber); } diff --git a/src/devtools/ProfilingCache.js b/src/devtools/ProfilingCache.js index 61420b1bf7..fe40d7e0b3 100644 --- a/src/devtools/ProfilingCache.js +++ b/src/devtools/ProfilingCache.js @@ -123,8 +123,9 @@ export default class ProfilingCache { return; } - // If no profiling data was recorded for this root, skip the round trip. this._pendingCommitDetailsMap.delete(pendingKey); + + // If no profiling data was recorded for this root, skip the round trip. resolve({ rootID, commitIndex, @@ -156,13 +157,12 @@ export default class ProfilingCache { commitDurations.push(commitIndex, selfDuration); } }); - const fiberCommitsFrontend: FiberCommitsFrontend = { + this._pendingFiberCommitsMap.delete(pendingKey); + resolve({ commitDurations, fiberID, rootID, - }; - this._pendingFiberCommitsMap.delete(pendingKey); - resolve(fiberCommitsFrontend); + }); return; } else if (this._store.profilingOperations.has(rootID)) { this._pendingFiberCommitsMap.set(pendingKey, resolve); @@ -174,14 +174,14 @@ export default class ProfilingCache { return; } - // If no profiling data was recorded for this root, skip the round trip. this._pendingFiberCommitsMap.delete(pendingKey); - const fiberCommitsFrontend: FiberCommitsFrontend = { + + // If no profiling data was recorded for this root, skip the round trip. + resolve({ commitDurations: [], fiberID, rootID, - }; - resolve(fiberCommitsFrontend); + }); }); }, ({ fiberID, rendererID, rootID }: FiberCommitsParams) => @@ -214,13 +214,13 @@ export default class ProfilingCache { return; } - // If no profiling data was recorded for this root, skip the round trip. this._pendingInteractionsMap.delete(pendingKey); - const interactionsFrontend: InteractionsFrontend = { + + // If no profiling data was recorded for this root, skip the round trip. + resolve({ interactions: [], rootID, - }; - resolve(interactionsFrontend); + }); }); }, ({ rendererID, rootID }: ProfilingSummaryParams) => rootID @@ -249,16 +249,16 @@ export default class ProfilingCache { return; } - // If no profiling data was recorded for this root, skip the round trip. this._pendingProfileSummaryMap.delete(pendingKey); - const profilingSummaryFrontend: ProfilingSummaryFrontend = { + + // If no profiling data was recorded for this root, skip the round trip. + resolve({ rootID, commitDurations: [], commitTimes: [], initialTreeBaseDurations: new Map(), interactionCount: 0, - }; - resolve(profilingSummaryFrontend); + }); }); }, ({ rendererID, rootID }: ProfilingSummaryParams) => rootID @@ -361,15 +361,13 @@ export default class ProfilingCache { selfDurationsMap.set(fiberID, durations[i + 2]); } - resolve( - ({ - actualDurations: actualDurationsMap, - commitIndex, - interactions, - rootID, - selfDurations: selfDurationsMap, - }: CommitDetailsFrontend) - ); + resolve({ + actualDurations: actualDurationsMap, + commitIndex, + interactions, + rootID, + selfDurations: selfDurationsMap, + }); } }; @@ -383,13 +381,11 @@ export default class ProfilingCache { if (resolve != null) { this._pendingFiberCommitsMap.delete(key); - resolve( - ({ - commitDurations, - fiberID, - rootID, - }: FiberCommitsFrontend) - ); + resolve({ + commitDurations, + fiberID, + rootID, + }); } }; @@ -398,12 +394,10 @@ export default class ProfilingCache { if (resolve != null) { this._pendingInteractionsMap.delete(rootID); - resolve( - ({ - interactions, - rootID, - }: InteractionsFrontend) - ); + resolve({ + interactions, + rootID, + }); } }; @@ -425,15 +419,13 @@ export default class ProfilingCache { initialTreeBaseDurationsMap.set(fiberID, initialTreeBaseDuration); } - resolve( - ({ - commitDurations, - commitTimes, - initialTreeBaseDurations: initialTreeBaseDurationsMap, - interactionCount, - rootID, - }: ProfilingSummaryFrontend) - ); + resolve({ + commitDurations, + commitTimes, + initialTreeBaseDurations: initialTreeBaseDurationsMap, + interactionCount, + rootID, + }); } }; }