Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

prevent spamming warnings related to performance measurement code #7299

Merged
merged 4 commits into from
Jul 19, 2016
Merged

prevent spamming warnings related to performance measurement code #7299

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2662a2d
prevent spamming warnings related to performance measurement code
sassanh Jul 18, 2016
5bfb08b
minor changes in names and such
sassanh Jul 18, 2016
2d51f65
-
sassanh Jul 18, 2016
c5d98dd
-
sassanh Jul 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/renderers/shared/ReactDebugTool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ var currentTimerStartTime = null;
var currentTimerNestedFlushDuration = null;
var currentTimerType = null;

var lifeCycleTimerHasWarned = false;

function clearHistory() {
ReactComponentTreeDevtool.purgeUnmountedComponents();
ReactHostOperationHistoryDevtool.clearHistory();
Expand Down Expand Up @@ -110,14 +112,15 @@ function beginLifeCycleTimer(debugID, timerType) {
return;
}
warning(
!currentTimerType,
!currentTimerType || lifeCycleTimerHasWarned,
'There is an internal error in the React performance measurement code. ' +
'Did not expect %s timer to start while %s timer is still in ' +
'progress for %s instance.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
lifeCycleTimerHasWarned = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets it regardless of whether the warning has occurred. Sorry I didn't realize this at first.

It seems like it would indeed be easier to add an if (currentTimetType) here. The warning would just get lifeCycleTimerHasWarned as an argument.

Same below.

Sorry I didn't see this at first!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Np happens, I'm glad you found it. Just fixed it in new commit.

currentTimerStartTime = performanceNow();
currentTimerNestedFlushDuration = 0;
currentTimerDebugID = debugID;
Expand All @@ -129,14 +132,15 @@ function endLifeCycleTimer(debugID, timerType) {
return;
}
warning(
currentTimerType === timerType,
currentTimerType === timerType || lifeCycleTimerHasWarned,
'There is an internal error in the React performance measurement code. ' +
'We did not expect %s timer to stop while %s timer is still in ' +
'progress for %s instance. Please report this as a bug in React.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
lifeCycleTimerHasWarned = true;
if (isProfiling) {
currentFlushMeasurements.push({
timerType,
Expand Down