From 64412a0f24e485938b8ed7daac99531e251f8be9 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Thu, 26 May 2016 01:24:11 +0300 Subject: [PATCH 01/10] add not dev warning for ReactPerf & remove dev checking for ReactDebugTool.GetFlushHistory --- src/renderers/shared/ReactDebugTool.js | 4 +--- src/renderers/shared/ReactPerf.js | 32 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/ReactDebugTool.js b/src/renderers/shared/ReactDebugTool.js index cd6ac3bb80ed..6deb84bc85be 100644 --- a/src/renderers/shared/ReactDebugTool.js +++ b/src/renderers/shared/ReactDebugTool.js @@ -208,9 +208,7 @@ var ReactDebugTool = { } }, getFlushHistory() { - if (__DEV__) { - return flushHistory; - } + return flushHistory; }, onBeginFlush() { if (__DEV__) { diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 209651e21693..9284209bcc0f 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -13,17 +13,31 @@ var ReactDebugTool = require('ReactDebugTool'); var warning = require('warning'); +var alreadyWarned = false; function roundFloat(val, base = 2) { var n = Math.pow(10, base); return Math.floor(val * n) / n; } +function returnWarnIfDevFalse(returningValue = 0) { + if (!alreadyWarned) { + alreadyWarned = true; + warning(__DEV__, + 'ReactPerf is not supported in the production builds of React.' + + 'To collect measurements, please use the development build of React instead.'); + } + return returningValue; +} + function getFlushHistory() { + returnWarnIfDevFalse([]); return ReactDebugTool.getFlushHistory(); } function getExclusive(flushHistory = getFlushHistory()) { + returnWarnIfDevFalse([]); + var aggregatedStats = {}; var affectedIDs = {}; @@ -74,6 +88,8 @@ function getExclusive(flushHistory = getFlushHistory()) { } function getInclusive(flushHistory = getFlushHistory()) { + returnWarnIfDevFalse([]); + var aggregatedStats = {}; var affectedIDs = {}; @@ -142,6 +158,8 @@ function getInclusive(flushHistory = getFlushHistory()) { } function getWasted(flushHistory = getFlushHistory()) { + returnWarnIfDevFalse([]); + var aggregatedStats = {}; var affectedIDs = {}; @@ -235,6 +253,8 @@ function getWasted(flushHistory = getFlushHistory()) { } function getOperations(flushHistory = getFlushHistory()) { + returnWarnIfDevFalse([]); + var stats = []; flushHistory.forEach((flush, flushIndex) => { var {operations, treeSnapshot} = flush; @@ -258,6 +278,8 @@ function getOperations(flushHistory = getFlushHistory()) { } function printExclusive(flushHistory) { + returnWarnIfDevFalse(''); + var stats = getExclusive(flushHistory); var table = stats.map(item => { var {key, instanceCount, totalDuration} = item; @@ -279,6 +301,8 @@ function printExclusive(flushHistory) { } function printInclusive(flushHistory) { + returnWarnIfDevFalse(''); + var stats = getInclusive(flushHistory); var table = stats.map(item => { var {key, instanceCount, inclusiveRenderDuration, renderCount} = item; @@ -293,6 +317,8 @@ function printInclusive(flushHistory) { } function printWasted(flushHistory) { + returnWarnIfDevFalse(''); + var stats = getWasted(flushHistory); var table = stats.map(item => { var {key, instanceCount, inclusiveRenderDuration, renderCount} = item; @@ -307,6 +333,8 @@ function printWasted(flushHistory) { } function printOperations(flushHistory) { + returnWarnIfDevFalse(''); + var stats = getOperations(flushHistory); var table = stats.map(stat => ({ 'Owner > Node': stat.key, @@ -344,14 +372,18 @@ function getMeasurementsSummaryMap(measurements) { } function start() { + returnWarnIfDevFalse(); ReactDebugTool.beginProfiling(); } function stop() { + returnWarnIfDevFalse(); + alreadyWarned = false; ReactDebugTool.endProfiling(); } function isRunning() { + returnWarnIfDevFalse(); return ReactDebugTool.isProfiling(); } From 878a99f27c5a8291d52de6d2e689f6e75469cf10 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Thu, 26 May 2016 22:18:22 +0300 Subject: [PATCH 02/10] move from warn to console.error & add explicitly returns --- src/renderers/shared/ReactPerf.js | 70 ++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 9284209bcc0f..42f9f8a7e616 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -20,23 +20,27 @@ function roundFloat(val, base = 2) { return Math.floor(val * n) / n; } -function returnWarnIfDevFalse(returningValue = 0) { - if (!alreadyWarned) { - alreadyWarned = true; - warning(__DEV__, - 'ReactPerf is not supported in the production builds of React.' + - 'To collect measurements, please use the development build of React instead.'); - } - return returningValue; +function warnInProduction() { + if (typeof console !== 'undefined') { + console.error('ReactPerf is not supported in the production builds of React.' + + 'To collect measurements, please use the development build of React instead.'); + } } function getFlushHistory() { - returnWarnIfDevFalse([]); + if (!__DEV__) { + warnInProduction(); + return []; + } + return ReactDebugTool.getFlushHistory(); } function getExclusive(flushHistory = getFlushHistory()) { - returnWarnIfDevFalse([]); + if (!__DEV__) { + warnInProduction(); + return []; + } var aggregatedStats = {}; var affectedIDs = {}; @@ -88,7 +92,10 @@ function getExclusive(flushHistory = getFlushHistory()) { } function getInclusive(flushHistory = getFlushHistory()) { - returnWarnIfDevFalse([]); + if (!__DEV__) { + warnInProduction(); + return []; + } var aggregatedStats = {}; var affectedIDs = {}; @@ -158,7 +165,10 @@ function getInclusive(flushHistory = getFlushHistory()) { } function getWasted(flushHistory = getFlushHistory()) { - returnWarnIfDevFalse([]); + if (!__DEV__) { + warnInProduction(); + return []; + } var aggregatedStats = {}; var affectedIDs = {}; @@ -253,7 +263,10 @@ function getWasted(flushHistory = getFlushHistory()) { } function getOperations(flushHistory = getFlushHistory()) { - returnWarnIfDevFalse([]); + if (!__DEV__) { + warnInProduction(); + return []; + } var stats = []; flushHistory.forEach((flush, flushIndex) => { @@ -278,7 +291,10 @@ function getOperations(flushHistory = getFlushHistory()) { } function printExclusive(flushHistory) { - returnWarnIfDevFalse(''); + if (!__DEV__) { + warnInProduction(); + return ''; + } var stats = getExclusive(flushHistory); var table = stats.map(item => { @@ -317,7 +333,10 @@ function printInclusive(flushHistory) { } function printWasted(flushHistory) { - returnWarnIfDevFalse(''); + if (!__DEV__) { + warnInProduction(); + return ''; + } var stats = getWasted(flushHistory); var table = stats.map(item => { @@ -333,7 +352,10 @@ function printWasted(flushHistory) { } function printOperations(flushHistory) { - returnWarnIfDevFalse(''); + if (!__DEV__) { + warnInProduction(); + return ''; + } var stats = getOperations(flushHistory); var table = stats.map(stat => ({ @@ -372,18 +394,28 @@ function getMeasurementsSummaryMap(measurements) { } function start() { - returnWarnIfDevFalse(); + if (!__DEV__) { + warnInProduction(); + return; + } ReactDebugTool.beginProfiling(); } function stop() { - returnWarnIfDevFalse(); + if (!__DEV__) { + warnInProduction(); + return; + } + alreadyWarned = false; ReactDebugTool.endProfiling(); } function isRunning() { - returnWarnIfDevFalse(); + if (!__DEV__) { + warnInProduction(); + return; + } return ReactDebugTool.isProfiling(); } From 2840d5ea5142a08660400ae82d63c3080ae720ef Mon Sep 17 00:00:00 2001 From: sashashakun Date: Thu, 26 May 2016 22:23:52 +0300 Subject: [PATCH 03/10] fix linter errors --- src/renderers/shared/ReactPerf.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 42f9f8a7e616..06b14ac1337f 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -13,7 +13,6 @@ var ReactDebugTool = require('ReactDebugTool'); var warning = require('warning'); -var alreadyWarned = false; function roundFloat(val, base = 2) { var n = Math.pow(10, base); @@ -22,9 +21,9 @@ function roundFloat(val, base = 2) { function warnInProduction() { if (typeof console !== 'undefined') { - console.error('ReactPerf is not supported in the production builds of React.' + - 'To collect measurements, please use the development build of React instead.'); - } + console.error('ReactPerf is not supported in the production builds of React.' + + 'To collect measurements, please use the development build of React instead.'); + } } function getFlushHistory() { @@ -317,7 +316,10 @@ function printExclusive(flushHistory) { } function printInclusive(flushHistory) { - returnWarnIfDevFalse(''); + if (!__DEV__) { + warnInProduction(); + return ''; + } var stats = getInclusive(flushHistory); var table = stats.map(item => { @@ -407,14 +409,13 @@ function stop() { return; } - alreadyWarned = false; ReactDebugTool.endProfiling(); } function isRunning() { if (!__DEV__) { warnInProduction(); - return; + return false; } return ReactDebugTool.isProfiling(); } From 33ae3f613b13ae07763fded327ee726b4886d747 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Thu, 26 May 2016 22:55:57 +0300 Subject: [PATCH 04/10] more fixes after review --- src/renderers/shared/ReactPerf.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 06b14ac1337f..76f056e31e3f 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -21,8 +21,9 @@ function roundFloat(val, base = 2) { function warnInProduction() { if (typeof console !== 'undefined') { - console.error('ReactPerf is not supported in the production builds of React.' + - 'To collect measurements, please use the development build of React instead.'); + console.error( + 'ReactPerf is not supported in the production builds of React.' + + 'To collect measurements, please use the development build of React instead.'); } } @@ -292,7 +293,7 @@ function getOperations(flushHistory = getFlushHistory()) { function printExclusive(flushHistory) { if (!__DEV__) { warnInProduction(); - return ''; + return; } var stats = getExclusive(flushHistory); @@ -318,7 +319,7 @@ function printExclusive(flushHistory) { function printInclusive(flushHistory) { if (!__DEV__) { warnInProduction(); - return ''; + return; } var stats = getInclusive(flushHistory); @@ -337,7 +338,7 @@ function printInclusive(flushHistory) { function printWasted(flushHistory) { if (!__DEV__) { warnInProduction(); - return ''; + return; } var stats = getWasted(flushHistory); @@ -356,7 +357,7 @@ function printWasted(flushHistory) { function printOperations(flushHistory) { if (!__DEV__) { warnInProduction(); - return ''; + return; } var stats = getOperations(flushHistory); From 25b08100feb5f11b93a3fa1d14579e2df32112b9 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Thu, 26 May 2016 23:06:30 +0300 Subject: [PATCH 05/10] roll back alreadyWarned variable and its changes --- src/renderers/shared/ReactPerf.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 76f056e31e3f..1a861e02decd 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -13,6 +13,7 @@ var ReactDebugTool = require('ReactDebugTool'); var warning = require('warning'); +var alreadyWarned = false; function roundFloat(val, base = 2) { var n = Math.pow(10, base); @@ -20,7 +21,8 @@ function roundFloat(val, base = 2) { } function warnInProduction() { - if (typeof console !== 'undefined') { + if (typeof console !== 'undefined' && !alreadyWarned) { + alreadyWarned = true; console.error( 'ReactPerf is not supported in the production builds of React.' + 'To collect measurements, please use the development build of React instead.'); From ec4f40c2884febb10308571a92724cde925e0634 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Fri, 27 May 2016 00:05:25 +0300 Subject: [PATCH 06/10] more fixes after review --- src/renderers/shared/ReactPerf.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 1a861e02decd..3aafb04eb247 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -25,7 +25,8 @@ function warnInProduction() { alreadyWarned = true; console.error( 'ReactPerf is not supported in the production builds of React.' + - 'To collect measurements, please use the development build of React instead.'); + 'To collect measurements, please use the development build of React instead.' + ); } } From d8683349c6019ff7bcaa4a14ad6e1d98ac906957 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Fri, 27 May 2016 00:23:42 +0300 Subject: [PATCH 07/10] move from one if to two --- src/renderers/shared/ReactPerf.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 3aafb04eb247..8c146619e153 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -21,7 +21,11 @@ function roundFloat(val, base = 2) { } function warnInProduction() { - if (typeof console !== 'undefined' && !alreadyWarned) { + if (!alreadyWarned) { + return; + } + + if (typeof console !== 'undefined') { alreadyWarned = true; console.error( 'ReactPerf is not supported in the production builds of React.' + From a8b46fb8d04d4ee139c4b4df01c8da815701385f Mon Sep 17 00:00:00 2001 From: sashashakun Date: Sat, 28 May 2016 09:54:47 +0300 Subject: [PATCH 08/10] a little fix after review --- src/renderers/shared/ReactPerf.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 8c146619e153..57e9aa3dfce4 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -25,8 +25,9 @@ function warnInProduction() { return; } - if (typeof console !== 'undefined') { - alreadyWarned = true; + alreadyWarned = true; + + if (typeof console !== 'undefined') { console.error( 'ReactPerf is not supported in the production builds of React.' + 'To collect measurements, please use the development build of React instead.' From 497d94b5946b0f992789437459c87354fc5d1788 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Sat, 4 Jun 2016 17:10:44 +0300 Subject: [PATCH 09/10] add tests and fix bug in solution --- src/renderers/shared/ReactPerf.js | 4 +-- .../shared/__tests__/ReactPerf-test.js | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/renderers/shared/ReactPerf.js b/src/renderers/shared/ReactPerf.js index 57e9aa3dfce4..939bcdba0db2 100644 --- a/src/renderers/shared/ReactPerf.js +++ b/src/renderers/shared/ReactPerf.js @@ -21,13 +21,13 @@ function roundFloat(val, base = 2) { } function warnInProduction() { - if (!alreadyWarned) { + if (alreadyWarned) { return; } alreadyWarned = true; - if (typeof console !== 'undefined') { + if (typeof console !== 'undefined') { console.error( 'ReactPerf is not supported in the production builds of React.' + 'To collect measurements, please use the development build of React instead.' diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 5fa4e3685c31..375523549cc3 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/src/renderers/shared/__tests__/ReactPerf-test.js @@ -445,4 +445,38 @@ describe('ReactPerf', function() { ReactPerf.stop(); expect(ReactPerf.isRunning()).toBe(false); }); + + it('should print console error only once', () => { + __DEV__ = false; + + spyOn(console, 'error'); + + expect(ReactPerf.getLastMeasurements()).toEqual([]); + + expect(ReactPerf.getExclusive()).toEqual([]); + + expect(ReactPerf.getInclusive()).toEqual([]); + + expect(ReactPerf.getWasted()).toEqual([]); + + expect(ReactPerf.getOperations()).toEqual([]); + + expect(ReactPerf.printExclusive()).toEqual(undefined); + + expect(ReactPerf.printInclusive()).toEqual(undefined); + + expect(ReactPerf.printWasted()).toEqual(undefined); + + expect(ReactPerf.printOperations()).toEqual(undefined); + + expect(ReactPerf.start()).toBe(undefined); + + expect(ReactPerf.stop()).toBe(undefined); + + expect(ReactPerf.isRunning()).toBe(false); + + expect(console.error.calls.count()).toBe(1); + + __DEV__ = true; + }) }); From 2116205a79b66adade68322e3e220d29a3770ce3 Mon Sep 17 00:00:00 2001 From: sashashakun Date: Sat, 4 Jun 2016 19:57:48 +0300 Subject: [PATCH 10/10] after review fixes --- src/renderers/shared/__tests__/ReactPerf-test.js | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/renderers/shared/__tests__/ReactPerf-test.js b/src/renderers/shared/__tests__/ReactPerf-test.js index 375523549cc3..babd6a03a512 100644 --- a/src/renderers/shared/__tests__/ReactPerf-test.js +++ b/src/renderers/shared/__tests__/ReactPerf-test.js @@ -450,29 +450,18 @@ describe('ReactPerf', function() { __DEV__ = false; spyOn(console, 'error'); - - expect(ReactPerf.getLastMeasurements()).toEqual([]); + expect(ReactPerf.getLastMeasurements()).toEqual([]); expect(ReactPerf.getExclusive()).toEqual([]); - expect(ReactPerf.getInclusive()).toEqual([]); - expect(ReactPerf.getWasted()).toEqual([]); - expect(ReactPerf.getOperations()).toEqual([]); - expect(ReactPerf.printExclusive()).toEqual(undefined); - expect(ReactPerf.printInclusive()).toEqual(undefined); - expect(ReactPerf.printWasted()).toEqual(undefined); - expect(ReactPerf.printOperations()).toEqual(undefined); - expect(ReactPerf.start()).toBe(undefined); - expect(ReactPerf.stop()).toBe(undefined); - expect(ReactPerf.isRunning()).toBe(false); expect(console.error.calls.count()).toBe(1);